[GlobalISel] Add G_SUB for computeNumSignBits #158384

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);

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp

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

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) {


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

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

@@ -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.

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp

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

Gentle ping for review, thanks! :)

llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp

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

llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir

jayfoad · 2025-10-02T11:00:46Z

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp

+        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.


Suggested change

// the output of the NEG has the same number of sign bits as the input.

// the output of the NEG has at least as many sign bits as the input.

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 arsenm, davemgreen, aemerson and jayfoad and removed request for arsenm September 16, 2025 13:16

arsenm reviewed Sep 16, 2025

View reviewed changes

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp Outdated Show resolved Hide resolved

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp Show resolved Hide resolved

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

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp Outdated Show resolved Hide resolved

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 added 5 commits September 22, 2025 11:11

Add tests for ISel and GlobalISel Sub

8819469

Fix typo

16dd818

Initial attemp for sub

ab98b8c

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

294002a

Handle Non Neg

12970b3

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

llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir Outdated Show resolved Hide resolved

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp Show resolved Hide resolved

llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp Show resolved Hide resolved

ningxinr and others added 2 commits September 29, 2025 15:02

Apply suggestions from code review

73f8bbd

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

Added Cst Zero test and Cst Neg One test for vector

0fb6987

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

RKSimon reviewed Sep 30, 2025

View reviewed changes

llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir Outdated Show resolved Hide resolved

Addressed feedback

18e7984

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

jayfoad reviewed Oct 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[GlobalISel] Add G_SUB for computeNumSignBits #158384

[GlobalISel] Add G_SUB for computeNumSignBits #158384

ningxinr commented Sep 12, 2025 •

edited

Loading

Uh oh!

llvmbot commented Sep 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

davemgreen Sep 16, 2025

Uh oh!

ningxinr Sep 16, 2025 •

edited

Loading

Uh oh!

ningxinr Oct 1, 2025

Uh oh!

davemgreen Oct 2, 2025

Uh oh!

davemgreen Sep 17, 2025

Uh oh!

RKSimon Sep 17, 2025

Uh oh!

Uh oh!

ningxinr commented Sep 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jayfoad Oct 2, 2025

Uh oh!

Uh oh!

	// the output of the NEG has the same number of sign bits as the input.
	// the output of the NEG has at least as many sign bits as the input.

[GlobalISel] Add G_SUB for computeNumSignBits #158384

Are you sure you want to change the base?

[GlobalISel] Add G_SUB for computeNumSignBits #158384

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!

jayfoad Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ningxinr commented Sep 12, 2025 •

edited

Loading

llvmbot commented Sep 15, 2025 •

edited

Loading

ningxinr Sep 16, 2025 •

edited

Loading