Skip to content

[GlobalISel] Add G_SUB for computeNumSignBits #158384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+352 −7
Open

[GlobalISel] Add G_SUB for computeNumSignBits #158384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8819469
Add tests for ISel and GlobalISel Sub
ningxinr Sep 12, 2025
16dd818
Fix typo
ningxinr Sep 12, 2025
ab98b8c
Initial attemp for sub
ningxinr Sep 12, 2025
294002a
Fix lit test CodeGen/ARM/GlobalISel/arm-legalize-bitcounts.mir
ningxinr Sep 15, 2025
12970b3
Handle Non Neg
ningxinr Sep 15, 2025
73f8bbd
Apply suggestions from code review
ningxinr Sep 29, 2025
0fb6987
Added Cst Zero test and Cst Neg One test for vector
ningxinr Sep 29, 2025
18e7984
Addressed feedback
ningxinr Oct 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions llvm/lib/CodeGen/GlobalISel/GISelValueTracking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1959,6 +1959,41 @@ unsigned GISelValueTracking::computeNumSignBits(Register R,

break;
}
case TargetOpcode::G_SUB: {
Register Src2 = MI.getOperand(2).getReg();
unsigned Src2NumSignBits =
computeNumSignBits(Src2, DemandedElts, Depth + 1);
if (Src2NumSignBits == 1)
return 1; // Early out.

// Handle NEG.
Register Src1 = MI.getOperand(1).getReg();
KnownBits Known1 = getKnownBits(Src1, DemandedElts, Depth);
if (Known1.isZero()) {
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (Known2.isNonNegative())
return Src2NumSignBits;

// Otherwise, we treat this like a SUB.
}

unsigned Src1NumSignBits =
computeNumSignBits(Src1, DemandedElts, Depth + 1);
if (Src1NumSignBits == 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;
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/GlobalISel/knownbits-ashr.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
...
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/GlobalISel/knownbits-shl.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
...
Expand Down
264 changes: 264 additions & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/knownbits-sub.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
# 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>" -filetype=null %s 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: VectorCstZero
body: |
bb.1:
; CHECK-LABEL: name: @VectorCstZero
; CHECK-NEXT: %0:_ KnownBits:0000000000000000 SignBits:16
; CHECK-NEXT: %1:_ KnownBits:0000000000000000 SignBits:16
; CHECK-NEXT: %2:_ KnownBits:0000000000000000 SignBits:16
; CHECK-NEXT: %3:_ KnownBits:0000000000000000 SignBits:16
%0:_(s16) = G_CONSTANT i16 0
%1:_(<4 x s16>) = G_BUILD_VECTOR %0, %0, %0, %0
%2:_(<4 x s16>) = G_BUILD_VECTOR %0, %0, %0, %0
%3:_(<4 x s16>) = G_SUB %1, %2
...
---
name: VectorCstNegOne
body: |
bb.1:
; CHECK-LABEL: name: @VectorCstNegOne
; CHECK-NEXT: %0:_ KnownBits:0000000000000000 SignBits:16
; CHECK-NEXT: %1:_ KnownBits:0000000000000001 SignBits:15
; CHECK-NEXT: %2:_ KnownBits:0000000000000000 SignBits:16
; CHECK-NEXT: %3:_ KnownBits:0000000000000001 SignBits:15
; CHECK-NEXT: %4:_ KnownBits:1111111111111111 SignBits:16
%0:_(s16) = G_CONSTANT i16 0
%1:_(s16) = G_CONSTANT i16 1
%2:_(<4 x s16>) = G_BUILD_VECTOR %0, %0, %0, %0
%3:_(<4 x s16>) = G_BUILD_VECTOR %1, %1, %1, %1
%4:_(<4 x s16>) = 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
%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_SUB %0, %3
...
7 changes: 4 additions & 3 deletions llvm/test/CodeGen/ARM/GlobalISel/arm-legalize-bitcounts.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions llvm/unittests/Target/AArch64/AArch64SelectionDAGTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,51 @@ TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_VASHR) {
EXPECT_EQ(DAG->ComputeNumSignBits(Fr2), 5u);
}

TEST_F(AArch64SelectionDAGTest, ComputeNumSignBits_SUB) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@ningxinr ningxinr Sep 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

Expand Down