[GlobalISel] Add G_SUB for computeNumSignBits by ningxinr · Pull Request #158384 · llvm/llvm-project

ningxinr · 2025-09-12T23:31:02Z

This patch ports the ISD::SUB handling from SelectionDAG’s ComputeNumSignBits to GlobalISel.

Related to #150515.

llvmbot · 2025-09-15T22:51:21Z

@llvm/pr-subscribers-llvm-selectiondag
@llvm/pr-subscribers-llvm-globalisel
@llvm/pr-subscribers-backend-aarch64

@llvm/pr-subscribers-backend-arm

Author: Yatao Wang (ningxinr)

Changes

This patch ports the ISD::SUB handling from SelectionDAG’s ComputeNumSignBits to GlobalISel.

Related to #150515.

Full diff: https://github.com/llvm/llvm-project/pull/158384.diff

6 Files Affected:

(modified) llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp (+36)
(modified) llvm/test/CodeGen/AArch64/GlobalISel/knownbits-ashr.mir (+2-2)
(modified) llvm/test/CodeGen/AArch64/GlobalISel/knownbits-shl.mir (+2-2)
(added) llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir (+234)
(modified) llvm/test/CodeGen/ARM/GlobalISel/arm-legalize-bitcounts.mir (+4-3)
(modified) llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp (+45)

diff --git a/llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp b/llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp
index 0cf44e02254de..0630d643f92a3 100644
--- a/llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp
@@ -1976,6 +1976,42 @@ unsigned GISelValueTracking::computeNumSignBits(Register R,
 
     break;
   }
+  case TargetOpcode::G_SUB: {
+    Register Src1 = MI.getOperand(1).getReg();
+    unsigned Src1NumSignBits =
+        computeNumSignBits(Src1, DemandedElts, Depth + 1);
+    if (Src1NumSignBits == 1)
+      return 1; // Early Out.
+
+    Register Src2 = MI.getOperand(2).getReg();
+    unsigned Src2NumSignBits =
+        computeNumSignBits(Src2, DemandedElts, Depth + 1);
+
+    // Handle NEG.
+    KnownBits Known1 = getKnownBits(Src1, DemandedElts, Depth);
+    if (Known1.Zero.isAllOnes()) {
+      KnownBits Known2 = getKnownBits(Src2, DemandedElts, Depth);
+      // If the input is known to be 0 or 1, the output is 0/-1, which is all
+      // sign bits set.
+      if ((Known2.Zero | 1).isAllOnes())
+        return TyBits;
+
+      // If the input is known to be positive (the sign bit is known clear),
+      // the output of the NEG has the same number of sign bits as the input.
+      if (Known2.isNonNegative())
+        return Src2NumSignBits;
+
+      // Otherwise, we treat this like a SUB.
+    }
+
+    if (Src2NumSignBits == 1)
+      return 1; // Early out.
+
+    // Sub can have at most one carry bit.  Thus we know that the output
+    // is, at worst, one more bit than the inputs.
+    FirstAnswer = std::min(Src1NumSignBits, Src2NumSignBits) - 1;
+    break;
+  }
   case TargetOpcode::G_FCMP:
   case TargetOpcode::G_ICMP: {
     bool IsFP = Opcode == TargetOpcode::G_FCMP;
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-ashr.mir b/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-ashr.mir
index 8552931c1f4c0..ee354479d603c 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-ashr.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-ashr.mir
@@ -102,8 +102,8 @@ body:             |
   ; CHECK-NEXT: %3:_ KnownBits:???????????????? SignBits:1
   ; CHECK-NEXT: %4:_ KnownBits:???????????????? SignBits:1
     %0:_(<4 x s16>) = COPY $d0
-    %2:_(s16) = COPY $h0
-    %1:_(s16) = G_CONSTANT i16 3
+    %1:_(s16) = COPY $h0
+    %2:_(s16) = G_CONSTANT i16 3
     %3:_(<4 x s16>) = G_BUILD_VECTOR %1, %2, %2, %1
     %4:_(<4 x s16>) = G_ASHR %0, %3
 ...
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-shl.mir b/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-shl.mir
index 61d1c4375bd6e..97bcb80503d8d 100644
--- a/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-shl.mir
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-shl.mir
@@ -135,8 +135,8 @@ body:             |
   ; CHECK-NEXT: %3:_ KnownBits:???????????????? SignBits:1
   ; CHECK-NEXT: %4:_ KnownBits:???????????????? SignBits:1
     %0:_(<4 x s16>) = COPY $d0
-    %2:_(s16) = COPY $h0
-    %1:_(s16) = G_CONSTANT i16 3
+    %1:_(s16) = COPY $h0
+    %2:_(s16) = G_CONSTANT i16 3
     %3:_(<4 x s16>) = G_BUILD_VECTOR %1, %2, %2, %1
     %4:_(<4 x s16>) = G_SHL %0, %3
 ...
diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir b/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir
new file mode 100644
index 0000000000000..6d3acdd85834b
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir
@@ -0,0 +1,234 @@
+# NOTE: Assertions have been autogenerated by utils/update_givaluetracking_test_checks.py UTC_ARGS: --version 5
+# RUN: llc -mtriple aarch64 -passes="print<gisel-value-tracking>" %s -o - 2>&1 | FileCheck %s
+
+---
+name:            Cst
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @Cst
+  ; CHECK-NEXT: %0:_ KnownBits:00000010 SignBits:6
+  ; CHECK-NEXT: %1:_ KnownBits:11100000 SignBits:3
+  ; CHECK-NEXT: %2:_ KnownBits:00100010 SignBits:2
+    %0:_(s8) = G_CONSTANT i8 2
+    %1:_(s8) = G_CONSTANT i8 224
+    %2:_(s8) = G_SUB %0, %1
+...
+---
+name:            CstZero
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @CstZero
+  ; CHECK-NEXT: %0:_ KnownBits:00000000 SignBits:8
+  ; CHECK-NEXT: %1:_ KnownBits:00000000 SignBits:8
+  ; CHECK-NEXT: %2:_ KnownBits:00000000 SignBits:8
+    %0:_(s8) = G_CONSTANT i8 0
+    %1:_(s8) = G_CONSTANT i8 0
+    %2:_(s8) = G_SUB %0, %1
+...
+---
+name:            CstNegOne
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @CstNegOne
+  ; CHECK-NEXT: %0:_ KnownBits:00000000 SignBits:8
+  ; CHECK-NEXT: %1:_ KnownBits:00000001 SignBits:7
+  ; CHECK-NEXT: %2:_ KnownBits:11111111 SignBits:8
+    %0:_(s8) = G_CONSTANT i8 0
+    %1:_(s8) = G_CONSTANT i8 1
+    %2:_(s8) = G_SUB %0, %1
+...
+---
+name:            CstNeg
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @CstNeg
+  ; CHECK-NEXT: %0:_ KnownBits:11100000 SignBits:3
+  ; CHECK-NEXT: %1:_ KnownBits:00000010 SignBits:6
+  ; CHECK-NEXT: %2:_ KnownBits:11011110 SignBits:2
+    %0:_(s8) = G_CONSTANT i8 224
+    %1:_(s8) = G_CONSTANT i8 2
+    %2:_(s8) = G_SUB %0, %1
+...
+---
+name:            ScalarVar
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @ScalarVar
+  ; CHECK-NEXT: %0:_ KnownBits:???????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:???????? SignBits:1
+  ; CHECK-NEXT: %2:_ KnownBits:???????? SignBits:1
+    %0:_(s8) = COPY $b0
+    %1:_(s8) = COPY $b1
+    %2:_(s8) = G_SUB %0, %1
+...
+---
+name:            ScalarRhsEarlyOut
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @ScalarRhsEarlyOut
+  ; CHECK-NEXT: %0:_ KnownBits:???????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:00000011 SignBits:6
+  ; CHECK-NEXT: %2:_ KnownBits:???????? SignBits:1
+    %0:_(s8) = COPY $b0
+    %1:_(s8) = G_CONSTANT i8 3
+    %2:_(s8) = G_SUB %0, %1
+...
+---
+name:            ScalarNonNegative
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @ScalarNonNegative
+  ; CHECK-NEXT: %0:_ KnownBits:???????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:00001111 SignBits:4
+  ; CHECK-NEXT: %2:_ KnownBits:0000???? SignBits:4
+  ; CHECK-NEXT: %3:_ KnownBits:00000000 SignBits:8
+  ; CHECK-NEXT: %4:_ KnownBits:???????? SignBits:4
+    %0:_(s8) = COPY $b0
+    %1:_(s8) = G_CONSTANT i8 15
+    %2:_(s8) = G_AND %0, %1
+    %3:_(s8) = G_CONSTANT i8 0
+    %4:_(s8) = G_SUB %3, %2
+...
+---
+name:            ScalarLhsEarlyOut
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @ScalarLhsEarlyOut
+  ; CHECK-NEXT: %0:_ KnownBits:???????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:00000011 SignBits:6
+  ; CHECK-NEXT: %2:_ KnownBits:???????? SignBits:1
+    %0:_(s8) = COPY $b0
+    %1:_(s8) = G_CONSTANT i8 3
+    %2:_(s8) = G_SUB %1, %0
+...
+---
+name:            ScalarPartKnown
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @ScalarPartKnown
+  ; CHECK-NEXT: %0:_ KnownBits:???????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:00001111 SignBits:4
+  ; CHECK-NEXT: %2:_ KnownBits:0000???? SignBits:4
+  ; CHECK-NEXT: %3:_ KnownBits:00000101 SignBits:5
+  ; CHECK-NEXT: %4:_ KnownBits:???????? SignBits:3
+    %0:_(s8) = COPY $b0
+    %1:_(s8) = G_CONSTANT i8 15
+    %2:_(s8) = G_AND %0, %1
+    %3:_(s8) = G_CONSTANT i8 5
+    %4:_(s8) = G_SUB %2, %3
+...
+---
+name:            VectorVar
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @VectorVar
+  ; CHECK-NEXT: %0:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %2:_ KnownBits:???????????????? SignBits:1
+    %0:_(<4 x s16>) = COPY $d0
+    %1:_(<4 x s16>) = COPY $d1
+    %2:_(<4 x s16>) = G_SUB %0, %1
+...
+---
+name:            VectorRhsEarlyOut
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @VectorRhsEarlyOut
+  ; CHECK-NEXT: %0:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:0000000000000011 SignBits:14
+  ; CHECK-NEXT: %2:_ KnownBits:0000000000000011 SignBits:14
+  ; CHECK-NEXT: %3:_ KnownBits:???????????????? SignBits:1
+    %0:_(<4 x s16>) = COPY $d0
+    %1:_(s16) = G_CONSTANT i16 3
+    %2:_(<4 x s16>) = G_BUILD_VECTOR %1, %1, %1, %1
+    %3:_(<4 x s16>) = G_SUB %2, %0
+...
+---
+name:            VectorNonNegative
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @VectorNonNegative
+  ; CHECK-NEXT: %0:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:0000000011111111 SignBits:8
+  ; CHECK-NEXT: %2:_ KnownBits:0000000011111111 SignBits:8
+  ; CHECK-NEXT: %3:_ KnownBits:00000000???????? SignBits:8
+  ; CHECK-NEXT: %4:_ KnownBits:0000000000000000 SignBits:16
+  ; CHECK-NEXT: %5:_ KnownBits:0000000000000000 SignBits:16
+  ; CHECK-NEXT: %6:_ KnownBits:???????????????? SignBits:8
+    %0:_(<4 x s16>) = COPY $d0
+    %1:_(s16) = G_CONSTANT i16 255
+    %2:_(<4 x s16>) = G_BUILD_VECTOR %1, %1, %1, %1
+    %3:_(<4 x s16>) = G_AND %0, %2
+    %4:_(s16) = G_CONSTANT i16 0
+    %5:_(<4 x s16>) = G_BUILD_VECTOR %4, %4, %4, %4
+    %6:_(<4 x s16>) = G_SUB %5, %3
+...
+---
+name:            VectorLhsEarlyOut
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @VectorLhsEarlyOut
+  ; CHECK-NEXT: %0:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:0000000000000011 SignBits:14
+  ; CHECK-NEXT: %2:_ KnownBits:0000000000000011 SignBits:14
+  ; CHECK-NEXT: %3:_ KnownBits:???????????????? SignBits:1
+    %0:_(<4 x s16>) = COPY $d0
+    %1:_(s16) = G_CONSTANT i16 3
+    %2:_(<4 x s16>) = G_BUILD_VECTOR %1, %1, %1, %1
+    %3:_(<4 x s16>) = G_SUB %0, %2
+...
+---
+name:            VectorPartKnown
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @VectorPartKnown
+  ; CHECK-NEXT: %0:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:0000000011111111 SignBits:8
+  ; CHECK-NEXT: %2:_ KnownBits:0000000011111111 SignBits:8
+  ; CHECK-NEXT: %3:_ KnownBits:00000000???????? SignBits:8
+  ; CHECK-NEXT: %4:_ KnownBits:0000000000101010 SignBits:10
+  ; CHECK-NEXT: %5:_ KnownBits:0000000001001010 SignBits:9
+  ; CHECK-NEXT: %6:_ KnownBits:000000000??01010 SignBits:9
+  ; CHECK-NEXT: %7:_ KnownBits:???????????????? SignBits:7
+    %0:_(<4 x s16>) = COPY $d0
+    %1:_(s16) = G_CONSTANT i16 255
+    %2:_(<4 x s16>) = G_BUILD_VECTOR %1, %1, %1, %1
+    %3:_(<4 x s16>) = G_AND %0, %2
+    %4:_(s16) = G_CONSTANT i16 42
+    %5:_(s16) = G_CONSTANT i16 74
+    %6:_(<4 x s16>) = G_BUILD_VECTOR %4, %5, %5, %4
+    %7:_(<4 x s16>) = G_SUB %6, %3
+...
+---
+name:            VectorCst36
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @VectorCst36
+  ; CHECK-NEXT: %0:_ KnownBits:0000000000000011 SignBits:14
+  ; CHECK-NEXT: %1:_ KnownBits:0000000000000110 SignBits:13
+  ; CHECK-NEXT: %2:_ KnownBits:0000000000000?1? SignBits:13
+  ; CHECK-NEXT: %3:_ KnownBits:0000000000000?1? SignBits:13
+  ; CHECK-NEXT: %4:_ KnownBits:???????????????? SignBits:12
+    %0:_(s16) = G_CONSTANT i16 3
+    %1:_(s16) = G_CONSTANT i16 6
+    %2:_(<4 x s16>) = G_BUILD_VECTOR %0, %1, %1, %0
+    %3:_(<4 x s16>) = G_BUILD_VECTOR %0, %1, %1, %0
+    %4:_(<4 x s16>) = G_SUB %2, %3
+...
+
+---
+name:            VectorCst3unknown
+body:             |
+  bb.1:
+  ; CHECK-LABEL: name: @VectorCst3unknown
+  ; CHECK-NEXT: %0:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %1:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %2:_ KnownBits:0000000000000011 SignBits:14
+  ; CHECK-NEXT: %3:_ KnownBits:???????????????? SignBits:1
+  ; CHECK-NEXT: %4:_ KnownBits:???????????????? SignBits:1
+    %0:_(<4 x s16>) = COPY $d0
+    %2:_(s16) = COPY $h0
+    %1:_(s16) = G_CONSTANT i16 3
+    %3:_(<4 x s16>) = G_BUILD_VECTOR %1, %2, %2, %1
+    %4:_(<4 x s16>) = G_SUB %0, %3
+...
diff --git a/llvm/test/CodeGen/ARM/GlobalISel/arm-legalize-bitcounts.mir b/llvm/test/CodeGen/ARM/GlobalISel/arm-legalize-bitcounts.mir
index c8fee5d334429..7cbe5de22debc 100644
--- a/llvm/test/CodeGen/ARM/GlobalISel/arm-legalize-bitcounts.mir
+++ b/llvm/test/CodeGen/ARM/GlobalISel/arm-legalize-bitcounts.mir
@@ -119,9 +119,10 @@ body:             |
     ; CHECK: [[R32:%[0-9]+]]:_(s32) = G_SUB [[COUNT]], [[BITDIFF]]
     %2(s16) = G_CTLZ %1
 
-    ; CHECK: [[SHIFTEDR:%[0-9]+]]:_(s32) = G_SHL [[R32]], [[BITDIFF]]
-    ; CHECK: [[R:%[0-9]+]]:_(s32) = G_ASHR [[SHIFTEDR]], [[BITDIFF]]
-    ; CHECK: $r0 = COPY [[R]]
+    ; LIBCALLS: [[SHIFTEDR:%[0-9]+]]:_(s32) = G_SHL [[R32]], [[BITDIFF]]
+    ; LIBCALLS: [[R:%[0-9]+]]:_(s32) = G_ASHR [[SHIFTEDR]], [[BITDIFF]]
+    ; LIBCALLS: $r0 = COPY [[R]]
+    ; CLZ: $r0 = COPY [[R32]]
     %3(s32) = G_SEXT %2(s16)
     $r0 = COPY %3(s32)
     BX_RET 14, $noreg, implicit $r0
diff --git a/llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp b/llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp
index c74d15782398a..b56e3ec53c18c 100644
--- a/llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp
+++ b/llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp
@@ -177,6 +177,51 @@ TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_VASHR) {
   EXPECT_EQ(DAG->ComputeNumSignBits(Fr2), 5u);
 }
 
+TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_SUB) {
+  SDLoc Loc;
+  auto IntVT = EVT::getIntegerVT(Context, 8);
+  auto N0 = DAG->getConstant(0x00, Loc, IntVT);
+  auto N1 = DAG->getConstant(0x01, Loc, IntVT);
+  auto N5 = DAG->getConstant(0x05, Loc, IntVT);
+  auto Nsign1 = DAG->getConstant(0x55, Loc, IntVT);
+  auto UnknownOp = DAG->getRegister(0, IntVT);
+  auto Mask = DAG->getConstant(0x1e, Loc, IntVT);
+  auto Nsign3 = DAG->getNode(ISD::AND, Loc, IntVT, Mask, UnknownOp);
+  // RHS early out
+  // Nsign1 = 01010101
+  // Nsign3 = 000????0
+  auto OpRhsEo = DAG->getNode(ISD::SUB, Loc, IntVT, Nsign3, Nsign1);
+  EXPECT_EQ(DAG->ComputeNumSignBits(OpRhsEo), 1u);
+
+  // Neg 0
+  // N0 = 00000000
+  auto OpNegZero = DAG->getNode(ISD::SUB, Loc, IntVT, N0, N0);
+  EXPECT_EQ(DAG->ComputeNumSignBits(OpNegZero), 8u);
+
+  // Neg 1
+  // N0 = 00000000
+  // N1 = 00000001
+  auto OpNegOne = DAG->getNode(ISD::SUB, Loc, IntVT, N0, N1);
+  EXPECT_EQ(DAG->ComputeNumSignBits(OpNegOne), 8u);
+
+  // Non negative
+  // N0     = 00000000
+  // Nsign3 = 000????0
+  auto OpNonNeg = DAG->getNode(ISD::SUB, Loc, IntVT, N0, Nsign3);
+  EXPECT_EQ(DAG->ComputeNumSignBits(OpNonNeg), 3u);
+
+  // LHS early out
+  // Nsign1 = 01010101
+  // Nsign3 = 000????0
+  auto OpLhsEo = DAG->getNode(ISD::SUB, Loc, IntVT, Nsign1, Nsign3);
+  EXPECT_EQ(DAG->ComputeNumSignBits(OpLhsEo), 1u);
+
+  // Nsign3 = 000????0
+  // N5     = 00000101
+  auto Op = DAG->getNode(ISD::SUB, Loc, IntVT, Nsign3, N5);
+  EXPECT_EQ(DAG->ComputeNumSignBits(Op), 2u);
+}
+
 TEST_F(AArch64SelectionDAGTest, SimplifyDemandedVectorElts_EXTRACT_SUBVECTOR) {
   TargetLowering TL(*TM);

davemgreen · 2025-09-16T13:32:51Z

@@ -177,6 +177,51 @@ TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_VASHR) {
  EXPECT_EQ(DAG->ComputeNumSignBits(Fr2), 5u);
 }

+TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_SUB) {


This will be testing selection dag, not the new gisel code that has been added.

I don't know if we have tests like this already but if not this could be a separate patch.

I don't know if we have tests like this already but if not this could be a separate patch.

If you meant ComputeNumSignBits unit tests for other opcodes, then yes, there are a couple for ZERO/SIGN_EXTEND_VECTOR* and others. For example, right above the added test for SUB, there is the ComputeNumSignBits_VASHR.

I only added selection dag test because 1) I haven't found any existing unit test for ComputeNumSignBits and SUB, 2) I want to have some sort of verification that both processes produce similar results.

Edit: Now that I am changing SelectionDAG as well, it would make sense to keep the test. :)

Hi David, would you still prefer that I remove the SelectionDAG test from this change?

Hi - I think it is fine to keep them here, either way is fine. You might find that in general the more focussed the patches are, the easier they are to review and get committed. But in this case it is fine to keep it here as the testing is related.

davemgreen · 2025-09-17T09:30:28Z

@@ -5066,8 +5066,13 @@ unsigned SelectionDAG::ComputeNumSignBits(SDValue Op, const APInt &DemandedElts,
    if (Tmp2 == 1) return 1; // Early out.
    return std::min(Tmp, Tmp2) - 1;
  case ISD::SUB:
+    Tmp = ComputeNumSignBits(Op.getOperand(0), DemandedElts, Depth + 1);


If we are making SelectionDAG changes that should probably be in it's own (NFC) commit.

This calculates ComputeNumSignBits on the first operand now even if it doesn't get used by the NEG if block. It might be faster if it kept the same structure as before.

Ideally we'd check https://llvm-compile-time-tracker.com for this one - (SUB X, C) is likely to fold to (ADD, -C) so the reasoning behind checking Op1 first for early-out is weaker, but it might still have an effect.

ningxinr · 2025-09-22T19:55:09Z

Gentle ping for review, thanks! :)

Co-authored-by: Matt Arsenault <arsenm2@gmail.com>

ningxinr · 2025-10-09T16:33:10Z

Gentle ping for review, thanks! :)

RKSimon

LGTM

This patch ports the ISD::SUB handling from SelectionDAG’s ComputeNumSignBits to GlobalISel. Related to llvm#150515. --------- Co-authored-by: Matt Arsenault <arsenm2@gmail.com> Co-authored-by: Simon Pilgrim <llvm-dev@redking.me.uk>

ningxinr marked this pull request as ready for review September 15, 2025 22:50

llvmbot added backend:ARM backend:AArch64 llvm:globalisel labels Sep 15, 2025

ningxinr mentioned this pull request Sep 15, 2025

[GlobalISel] Port known-bit and known-sign-bits for all operations #150515

Open

davemgreen requested review from aemerson, arsenm, davemgreen and jayfoad and removed request for arsenm September 16, 2025 13:16

arsenm reviewed Sep 16, 2025

View reviewed changes

Comment thread llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp Outdated

Comment thread llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp

davemgreen reviewed Sep 16, 2025

View reviewed changes

ningxinr force-pushed the issue-150515 branch from 3a7bc10 to 4c4500d Compare September 16, 2025 18:25

llvmbot added the llvm:SelectionDAG SelectionDAGISel as well label Sep 16, 2025

ningxinr requested review from arsenm and davemgreen September 16, 2025 18:42

davemgreen reviewed Sep 17, 2025

View reviewed changes

ningxinr force-pushed the issue-150515 branch from 4c4500d to 25d6ed1 Compare September 17, 2025 20:43

ningxinr requested review from RKSimon and davemgreen September 17, 2025 20:59

RKSimon reviewed Sep 19, 2025

View reviewed changes

Comment thread llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp Outdated

ningxinr force-pushed the issue-150515 branch from 25d6ed1 to acfa00a Compare September 19, 2025 17:53

ningxinr requested a review from RKSimon September 19, 2025 17:53

ningxinr force-pushed the issue-150515 branch from acfa00a to 12970b3 Compare September 22, 2025 18:26

arsenm reviewed Sep 25, 2025

View reviewed changes

Comment thread llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir Outdated

Comment thread llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp

Comment thread llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp

ningxinr requested a review from arsenm September 29, 2025 22:38

RKSimon reviewed Sep 30, 2025

View reviewed changes

Comment thread llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir Outdated

ningxinr requested a review from RKSimon October 1, 2025 16:03

jayfoad reviewed Oct 2, 2025

View reviewed changes

Comment thread llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp Outdated

ningxinr and others added 9 commits October 7, 2025 10:43

Add tests for ISel and GlobalISel Sub

9bf8974

Fix typo

6f1eaec

Initial attemp for sub

2fa01bd

Fix lit test CodeGen/ARM/GlobalISel/arm-legalize-bitcounts.mir

1c29e66

Handle Non Neg

df9752a

Apply suggestions from code review

9163f55

Co-authored-by: Matt Arsenault <arsenm2@gmail.com>

Added Cst Zero test and Cst Neg One test for vector

9f0a553

Addressed feedback

252b865

Improved result when RHS is constant

1c9c087

ningxinr force-pushed the issue-150515 branch from 18e7984 to 1c9c087 Compare October 7, 2025 22:03

ningxinr requested a review from jayfoad October 7, 2025 22:18

arsenm approved these changes Oct 10, 2025

View reviewed changes

RKSimon approved these changes Oct 13, 2025

View reviewed changes

Merge branch 'main' into issue-150515

5e01b75

RKSimon enabled auto-merge (squash) October 13, 2025 10:15

RKSimon merged commit c4bcbf0 into llvm:main Oct 13, 2025
9 of 10 checks passed

ningxinr deleted the issue-150515 branch October 13, 2025 16:14

Uh oh!

Conversation

ningxinr commented Sep 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davemgreen Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

ningxinr Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ningxinr Oct 1, 2025

Choose a reason for hiding this comment

Uh oh!

davemgreen Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

davemgreen Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

RKSimon Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ningxinr commented Sep 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ningxinr commented Oct 9, 2025

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ningxinr commented Sep 12, 2025 •

edited

Loading

llvmbot commented Sep 15, 2025 •

edited

Loading

ningxinr Sep 16, 2025 •

edited

Loading