[AArch64] AArch64TargetLowering::computeKnownBitsForTargetNode - add support for AArch64ISD::MOV/MVN constants #154039
Conversation
|
@llvm/pr-subscribers-backend-aarch64 Author: Yatao Wang (ningxinr) ChangesAdd support for the following constant nodes in Also add Issue: #153159 Full diff: https://github.com/llvm/llvm-project/pull/154039.diff 3 Files Affected:
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index aefbbe2534be2..958410588996c 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -2619,6 +2619,32 @@ void AArch64TargetLowering::computeKnownBitsForTargetNode(
<< Op->getConstantOperandVal(1)));
break;
}
+ case AArch64ISD::MOVImsl: {
+ Known = KnownBits::makeConstant(
+ APInt(Known.getBitWidth(), ~(~Op->getConstantOperandVal(0)
+ << Op->getConstantOperandVal(1))));
+ break;
+ }
+ case AArch64ISD::MOVIedit: {
+ Known = KnownBits::makeConstant(APInt(
+ Known.getBitWidth(),
+ AArch64_AM::decodeAdvSIMDModImmType10(Op->getConstantOperandVal(0))));
+ break;
+ }
+ case AArch64ISD::MVNIshift: {
+ Known = KnownBits::makeConstant(
+ APInt(Known.getBitWidth(),
+ (~Op->getConstantOperandVal(0) << Op->getConstantOperandVal(1)),
+ false, true));
+ break;
+ }
+ case AArch64ISD::MVNImsl: {
+ Known = KnownBits::makeConstant(
+ APInt(Known.getBitWidth(),
+ ~(Op->getConstantOperandVal(0) << Op->getConstantOperandVal(1)),
+ false, true));
+ break;
+ }
case AArch64ISD::LOADgot:
case AArch64ISD::ADDlow: {
if (!Subtarget->isTargetILP32())
@@ -30624,6 +30650,10 @@ bool AArch64TargetLowering::isTargetCanonicalConstantNode(SDValue Op) const {
return Op.getOpcode() == AArch64ISD::DUP ||
Op.getOpcode() == AArch64ISD::MOVI ||
Op.getOpcode() == AArch64ISD::MOVIshift ||
+ Op.getOpcode() == AArch64ISD::MOVImsl ||
+ Op.getOpcode() == AArch64ISD::MOVIedit ||
+ Op.getOpcode() == AArch64ISD::MVNIshift ||
+ Op.getOpcode() == AArch64ISD::MVNImsl ||
(Op.getOpcode() == ISD::EXTRACT_SUBVECTOR &&
Op.getOperand(0).getOpcode() == AArch64ISD::DUP) ||
TargetLowering::isTargetCanonicalConstantNode(Op);
diff --git a/llvm/test/CodeGen/AArch64/urem-vector-lkk.ll b/llvm/test/CodeGen/AArch64/urem-vector-lkk.ll
index 468a33ce5bfcf..4dd86769c1dd5 100644
--- a/llvm/test/CodeGen/AArch64/urem-vector-lkk.ll
+++ b/llvm/test/CodeGen/AArch64/urem-vector-lkk.ll
@@ -8,14 +8,14 @@ define <4 x i16> @fold_urem_vec_1(<4 x i16> %x) {
; CHECK-NEXT: ldr d1, [x8, :lo12:.LCPI0_0]
; CHECK-NEXT: adrp x8, .LCPI0_1
; CHECK-NEXT: ldr d2, [x8, :lo12:.LCPI0_1]
-; CHECK-NEXT: adrp x8, .LCPI0_2
-; CHECK-NEXT: ushl v1.4h, v0.4h, v1.4h
-; CHECK-NEXT: umull v1.4s, v1.4h, v2.4h
-; CHECK-NEXT: movi d2, #0000000000000000
-; CHECK-NEXT: shrn v1.4h, v1.4s, #16
-; CHECK-NEXT: fneg d2, d2
-; CHECK-NEXT: sub v3.4h, v0.4h, v1.4h
-; CHECK-NEXT: umull v2.4s, v3.4h, v2.4h
+; CHECK-NEXT: mov x8, #-9223372036854775808 // =0x8000000000000000
+; CHECK-NEXT: ushl v1.4h, v0.4h, v1.4h
+; CHECK-NEXT: fmov d3, x8
+; CHECK-NEXT: adrp x8, .LCPI0_2
+; CHECK-NEXT: umull v1.4s, v1.4h, v2.4h
+; CHECK-NEXT: shrn v1.4h, v1.4s, #16
+; CHECK-NEXT: sub v2.4h, v0.4h, v1.4h
+; CHECK-NEXT: umull v2.4s, v2.4h, v3.4h
; CHECK-NEXT: shrn v2.4h, v2.4s, #16
; CHECK-NEXT: add v1.4h, v2.4h, v1.4h
; CHECK-NEXT: ldr d2, [x8, :lo12:.LCPI0_2]
diff --git a/llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp b/llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp
index f06f03bb35a5d..131b7eca942d0 100644
--- a/llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp
+++ b/llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp
@@ -318,6 +318,64 @@ TEST_F(AArch64SelectionDAGTest, ComputeKnownBits_UADDO_CARRY) {
EXPECT_EQ(Known.One, APInt(8, 0x86));
}
+// Piggy-backing on the AArch64 tests to verify SelectionDAG::computeKnownBits.
+TEST_F(AArch64SelectionDAGTest, ComputeKnownBits_MOVI) {
+ SDLoc Loc;
+ auto Int8VT = EVT::getIntegerVT(Context, 8);
+ auto Int16VT = EVT::getIntegerVT(Context, 16);
+ auto Int32VT = EVT::getIntegerVT(Context, 32);
+ auto Int64VT = EVT::getIntegerVT(Context, 64);
+ auto N0 = DAG->getConstant(0xA5, Loc, Int8VT);
+ KnownBits Known;
+
+ auto OpMOVIedit = DAG->getNode(AArch64ISD::MOVIedit, Loc, Int64VT, N0);
+ Known = DAG->computeKnownBits(OpMOVIedit);
+ EXPECT_EQ(Known.Zero, APInt(64, 0x00FF00FFFF00FF00));
+ EXPECT_EQ(Known.One, APInt(64, 0xFF00FF0000FF00FF));
+
+ auto N1 = DAG->getConstant(16, Loc, Int8VT);
+ auto OpMOVImsl = DAG->getNode(AArch64ISD::MOVImsl, Loc, Int32VT, N0, N1);
+ Known = DAG->computeKnownBits(OpMOVImsl);
+ EXPECT_EQ(Known.Zero, APInt(32, 0xFF5A0000));
+ EXPECT_EQ(Known.One, APInt(32, 0x00A5FFFF));
+
+ auto OpMVNImsl = DAG->getNode(AArch64ISD::MVNImsl, Loc, Int32VT, N0, N1);
+ Known = DAG->computeKnownBits(OpMVNImsl);
+ EXPECT_EQ(Known.Zero, APInt(32, 0x00A50000));
+ EXPECT_EQ(Known.One, APInt(32, 0xFF5AFFFF));
+
+ auto N2 = DAG->getConstant(16, Loc, Int8VT);
+ auto OpMOVIshift32 =
+ DAG->getNode(AArch64ISD::MOVIshift, Loc, Int32VT, N0, N2);
+ Known = DAG->computeKnownBits(OpMOVIshift32);
+ EXPECT_EQ(Known.Zero, APInt(32, 0xFF5AFFFF));
+ EXPECT_EQ(Known.One, APInt(32, 0x00A50000));
+
+ auto OpMVNIshift32 =
+ DAG->getNode(AArch64ISD::MVNIshift, Loc, Int32VT, N0, N2);
+ Known = DAG->computeKnownBits(OpMVNIshift32);
+ EXPECT_EQ(Known.Zero, APInt(32, 0x00A5FFFF));
+ EXPECT_EQ(Known.One, APInt(32, 0xFF5A0000));
+
+ auto N3 = DAG->getConstant(8, Loc, Int8VT);
+ auto OpMOVIshift16 =
+ DAG->getNode(AArch64ISD::MOVIshift, Loc, Int16VT, N0, N3);
+ Known = DAG->computeKnownBits(OpMOVIshift16);
+ EXPECT_EQ(Known.One, APInt(16, 0xA500));
+ EXPECT_EQ(Known.Zero, APInt(16, 0x5AFF));
+
+ auto OpMVNIshift16 =
+ DAG->getNode(AArch64ISD::MVNIshift, Loc, Int16VT, N0, N3);
+ Known = DAG->computeKnownBits(OpMVNIshift16);
+ EXPECT_EQ(Known.Zero, APInt(16, 0xA5FF));
+ EXPECT_EQ(Known.One, APInt(16, 0x5A00));
+
+ auto OpMOVI = DAG->getNode(AArch64ISD::MOVI, Loc, Int8VT, N0);
+ Known = DAG->computeKnownBits(OpMOVI);
+ EXPECT_EQ(Known.Zero, APInt(8, 0x5A));
+ EXPECT_EQ(Known.One, APInt(8, 0xA5));
+}
+
// Piggy-backing on the AArch64 tests to verify SelectionDAG::computeKnownBits.
TEST_F(AArch64SelectionDAGTest, ComputeKnownBits_SUB) {
SDLoc Loc;
|
|
Hi Simon, I still don't have the permission to add reviewers. Would you please take a look when you get a chance? Thank you thank you! |
|
CC @aabhinavg1 |
…support for AArch64ISD::MOV/MVN constants
| ; CHECK-NEXT: movi d2, #0000000000000000 | ||
| ; CHECK-NEXT: shrn v1.4h, v1.4s, #16 | ||
| ; CHECK-NEXT: fneg d2, d2 |
There was a problem hiding this comment.
Suppose I add fneg(zero) as a canonical constant pattern on a different branch than this and this file stays unchanged, should I expect the move and fneg gets folded into say one single fmov -0.0 instead?
I tried a few things including the one proposed above in file llvm/lib/Target/AArch64/AArch64ISelLowering.cpp, I also tried to add ISD::FNEG as a case in AArch64TargetLowering::computeKnownBitsForTargetNode directly, but nothing seems to change the test result of this file at all.
Should the change affect this fneg at all? Or shall I create my own test instead?
Thanks for your help! :)
There was a problem hiding this comment.
Hello. AArch64 doesn't have way to generate splat(0x8000000000000000) in a single instruction. We can either generate fmov(mov i64 0x8000000000000000) or use fneg to do fneg(movi 0x0). There is not a lot in it, but the fmov from gpr->fpr is quite expensive so we prefer the fneg version. See #80641 and TryWithFNeg, which can apply to any constant that can be materialized with a fneg, although it looks like most of the other cases are OK.
It will usually look like fneg(nvcast(movi)), but in this particular case the nvcast is removed as both types are f64.
Ah, thanks for the explanation! So it's because both fmov and movi have restrictions, that neither can handle all the cases for So the change in That makes a lot of sense. Thank you thank you! |
|
Gentle ping for review, thanks! :) |
|
Hi @RKSimon, no rush but would you help me merge this PR when you have a minute? Many thanks for the help! (I am still working towards that commit access. :D ) |
|
This patch may have caused bot failures, e.g. sanitizer-aarch64-linux-bootstrap-ubsan. I am investigating. |
|
Oh - we probably need to mask the shift amount to make sure we don't shift by more than the bitwidth. Using getShiftValue(Op1) maybe. |
…orTargetNode - add support for AArch64ISD::MOV/MVN constants" (#155503) Reverts llvm/llvm-project#154039, as it breaks bots.
Add support for the following constant nodes in
AArch64TargetLowering::computeKnownBitsForTargetNode:Also add
AArch64TargetLowering::computeKnownBitsForTargetNodetests for all the MOVI constant nodes inllvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cppFixes: #153159