Skip to content

Commit 10b323b

Browse files
authored
[AArch64][GISel] Signed comparison using CMN is safe when the subtraction is nsw (#150480)
#141993 but for GISel, though for LHS, we now do the inverse compare, something that SelDAG does not do as of now but I will do in a future patch.
1 parent 3313cf4 commit 10b323b

File tree

4 files changed

+349
-144
lines changed

4 files changed

+349
-144
lines changed

llvm/lib/Target/AArch64/GISel/AArch64GlobalISelUtils.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,10 @@ bool AArch64GISelUtils::isCMN(const MachineInstr *MaybeSub,
5050
//
5151
// %sub = G_SUB 0, %y
5252
// %cmp = G_ICMP eq/ne, %z, %sub
53+
// or with signed comparisons with the no-signed-wrap flag set
5354
if (!MaybeSub || MaybeSub->getOpcode() != TargetOpcode::G_SUB ||
54-
!CmpInst::isEquality(Pred))
55+
(!CmpInst::isEquality(Pred) &&
56+
!(CmpInst::isSigned(Pred) && MaybeSub->getFlag(MachineInstr::NoSWrap))))
5557
return false;
5658
auto MaybeZero =
5759
getIConstantVRegValWithLookThrough(MaybeSub->getOperand(1).getReg(), MRI);

llvm/lib/Target/AArch64/GISel/AArch64InstructionSelector.cpp

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1810,7 +1810,7 @@ bool AArch64InstructionSelector::selectCompareBranchFedByICmp(
18101810

18111811
// Couldn't optimize. Emit a compare + a Bcc.
18121812
MachineBasicBlock *DestMBB = I.getOperand(1).getMBB();
1813-
auto PredOp = ICmp.getOperand(1);
1813+
auto &PredOp = ICmp.getOperand(1);
18141814
emitIntegerCompare(ICmp.getOperand(2), ICmp.getOperand(3), PredOp, MIB);
18151815
const AArch64CC::CondCode CC = changeICMPPredToAArch64CC(
18161816
static_cast<CmpInst::Predicate>(PredOp.getPredicate()));
@@ -2506,12 +2506,12 @@ bool AArch64InstructionSelector::earlySelect(MachineInstr &I) {
25062506
return false;
25072507
}
25082508
auto &PredOp = Cmp->getOperand(1);
2509-
auto Pred = static_cast<CmpInst::Predicate>(PredOp.getPredicate());
2510-
const AArch64CC::CondCode InvCC =
2511-
changeICMPPredToAArch64CC(CmpInst::getInversePredicate(Pred));
25122509
MIB.setInstrAndDebugLoc(I);
25132510
emitIntegerCompare(/*LHS=*/Cmp->getOperand(2),
25142511
/*RHS=*/Cmp->getOperand(3), PredOp, MIB);
2512+
auto Pred = static_cast<CmpInst::Predicate>(PredOp.getPredicate());
2513+
const AArch64CC::CondCode InvCC =
2514+
changeICMPPredToAArch64CC(CmpInst::getInversePredicate(Pred));
25152515
emitCSINC(/*Dst=*/AddDst, /*Src =*/AddLHS, /*Src2=*/AddLHS, InvCC, MIB);
25162516
I.eraseFromParent();
25172517
return true;
@@ -3574,10 +3574,11 @@ bool AArch64InstructionSelector::select(MachineInstr &I) {
35743574
return false;
35753575
}
35763576

3577-
auto Pred = static_cast<CmpInst::Predicate>(I.getOperand(1).getPredicate());
3577+
auto &PredOp = I.getOperand(1);
3578+
emitIntegerCompare(I.getOperand(2), I.getOperand(3), PredOp, MIB);
3579+
auto Pred = static_cast<CmpInst::Predicate>(PredOp.getPredicate());
35783580
const AArch64CC::CondCode InvCC =
35793581
changeICMPPredToAArch64CC(CmpInst::getInversePredicate(Pred));
3580-
emitIntegerCompare(I.getOperand(2), I.getOperand(3), I.getOperand(1), MIB);
35813582
emitCSINC(/*Dst=*/I.getOperand(0).getReg(), /*Src1=*/AArch64::WZR,
35823583
/*Src2=*/AArch64::WZR, InvCC, MIB);
35833584
I.eraseFromParent();
@@ -5096,11 +5097,11 @@ bool AArch64InstructionSelector::tryOptSelect(GSelect &I) {
50965097

50975098
AArch64CC::CondCode CondCode;
50985099
if (CondOpc == TargetOpcode::G_ICMP) {
5099-
auto Pred =
5100-
static_cast<CmpInst::Predicate>(CondDef->getOperand(1).getPredicate());
5100+
auto &PredOp = CondDef->getOperand(1);
5101+
emitIntegerCompare(CondDef->getOperand(2), CondDef->getOperand(3), PredOp,
5102+
MIB);
5103+
auto Pred = static_cast<CmpInst::Predicate>(PredOp.getPredicate());
51015104
CondCode = changeICMPPredToAArch64CC(Pred);
5102-
emitIntegerCompare(CondDef->getOperand(2), CondDef->getOperand(3),
5103-
CondDef->getOperand(1), MIB);
51045105
} else {
51055106
// Get the condition code for the select.
51065107
auto Pred =
@@ -5148,29 +5149,37 @@ MachineInstr *AArch64InstructionSelector::tryFoldIntegerCompare(
51485149
MachineInstr *LHSDef = getDefIgnoringCopies(LHS.getReg(), MRI);
51495150
MachineInstr *RHSDef = getDefIgnoringCopies(RHS.getReg(), MRI);
51505151
auto P = static_cast<CmpInst::Predicate>(Predicate.getPredicate());
5152+
51515153
// Given this:
51525154
//
51535155
// x = G_SUB 0, y
5154-
// G_ICMP x, z
5156+
// G_ICMP z, x
51555157
//
51565158
// Produce this:
51575159
//
5158-
// cmn y, z
5159-
if (isCMN(LHSDef, P, MRI))
5160-
return emitCMN(LHSDef->getOperand(2), RHS, MIRBuilder);
5160+
// cmn z, y
5161+
if (isCMN(RHSDef, P, MRI))
5162+
return emitCMN(LHS, RHSDef->getOperand(2), MIRBuilder);
51615163

5162-
// Same idea here, but with the RHS of the compare instead:
5164+
// Same idea here, but with the LHS of the compare instead:
51635165
//
51645166
// Given this:
51655167
//
51665168
// x = G_SUB 0, y
5167-
// G_ICMP z, x
5169+
// G_ICMP x, z
51685170
//
51695171
// Produce this:
51705172
//
5171-
// cmn z, y
5172-
if (isCMN(RHSDef, P, MRI))
5173-
return emitCMN(LHS, RHSDef->getOperand(2), MIRBuilder);
5173+
// cmn y, z
5174+
//
5175+
// But be careful! We need to swap the predicate!
5176+
if (isCMN(LHSDef, P, MRI)) {
5177+
if (!CmpInst::isEquality(P)) {
5178+
P = CmpInst::getSwappedPredicate(P);
5179+
Predicate = MachineOperand::CreatePredicate(P);
5180+
}
5181+
return emitCMN(LHSDef->getOperand(2), RHS, MIRBuilder);
5182+
}
51745183

51755184
// Given this:
51765185
//

llvm/test/CodeGen/AArch64/GlobalISel/postlegalizer-lowering-swap-compare-operands.mir

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -667,11 +667,10 @@ body: |
667667
; SELECT-NEXT: {{ $}}
668668
; SELECT-NEXT: %zero:gpr64 = COPY $xzr
669669
; SELECT-NEXT: %reg0:gpr64 = COPY $x0
670-
; SELECT-NEXT: %shl:gpr64 = UBFMXri %reg0, 1, 0
670+
; SELECT-NEXT: %cmp_lhs:gpr64 = SUBSXrs %zero, %reg0, 63, implicit-def dead $nzcv
671671
; SELECT-NEXT: %reg1:gpr64 = COPY $x1
672672
; SELECT-NEXT: %sext_in_reg:gpr64 = SBFMXri %reg1, 0, 0
673-
; SELECT-NEXT: %cmp_rhs:gpr64 = SUBSXrs %zero, %sext_in_reg, 131, implicit-def dead $nzcv
674-
; SELECT-NEXT: [[ADDSXrr:%[0-9]+]]:gpr64 = ADDSXrr %shl, %cmp_rhs, implicit-def $nzcv
673+
; SELECT-NEXT: [[ADDSXrs:%[0-9]+]]:gpr64 = ADDSXrs %cmp_lhs, %sext_in_reg, 131, implicit-def $nzcv
675674
; SELECT-NEXT: %cmp:gpr32 = CSINCWr $wzr, $wzr, 1, implicit $nzcv
676675
; SELECT-NEXT: $w0 = COPY %cmp
677676
; SELECT-NEXT: RET_ReallyLR implicit $w0

0 commit comments

Comments
 (0)