Skip to content

Commit d2e62bd

Browse files
committed
[AArch64][GISel] Signed comparison using CMN is safe when the subtraction is nsw
llvm#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 e89d485 commit d2e62bd

File tree

4 files changed

+61
-47
lines changed

4 files changed

+61
-47
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

llvm/test/CodeGen/AArch64/cmp-to-cmn.ll

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -781,36 +781,22 @@ define i1 @almost_immediate_neg_ugt_64(i64 %x) {
781781
}
782782

783783
define i1 @cmn_nsw(i32 %a, i32 %b) {
784-
; SDISEL-LABEL: cmn_nsw:
785-
; SDISEL: // %bb.0:
786-
; SDISEL-NEXT: cmn w0, w1
787-
; SDISEL-NEXT: cset w0, gt
788-
; SDISEL-NEXT: ret
789-
;
790-
; GISEL-LABEL: cmn_nsw:
791-
; GISEL: // %bb.0:
792-
; GISEL-NEXT: neg w8, w1
793-
; GISEL-NEXT: cmp w0, w8
794-
; GISEL-NEXT: cset w0, gt
795-
; GISEL-NEXT: ret
784+
; CHECK-LABEL: cmn_nsw:
785+
; CHECK: // %bb.0:
786+
; CHECK-NEXT: cmn w0, w1
787+
; CHECK-NEXT: cset w0, gt
788+
; CHECK-NEXT: ret
796789
%sub = sub nsw i32 0, %b
797790
%cmp = icmp sgt i32 %a, %sub
798791
ret i1 %cmp
799792
}
800793

801794
define i1 @cmn_nsw_64(i64 %a, i64 %b) {
802-
; SDISEL-LABEL: cmn_nsw_64:
803-
; SDISEL: // %bb.0:
804-
; SDISEL-NEXT: cmn x0, x1
805-
; SDISEL-NEXT: cset w0, gt
806-
; SDISEL-NEXT: ret
807-
;
808-
; GISEL-LABEL: cmn_nsw_64:
809-
; GISEL: // %bb.0:
810-
; GISEL-NEXT: neg x8, x1
811-
; GISEL-NEXT: cmp x0, x8
812-
; GISEL-NEXT: cset w0, gt
813-
; GISEL-NEXT: ret
795+
; CHECK-LABEL: cmn_nsw_64:
796+
; CHECK: // %bb.0:
797+
; CHECK-NEXT: cmn x0, x1
798+
; CHECK-NEXT: cset w0, gt
799+
; CHECK-NEXT: ret
814800
%sub = sub nsw i64 0, %b
815801
%cmp = icmp sgt i64 %a, %sub
816802
ret i1 %cmp
@@ -828,6 +814,24 @@ define i1 @cmn_nsw_neg(i32 %a, i32 %b) {
828814
ret i1 %cmp
829815
}
830816

817+
define i1 @cmn_swap(i32 %a, i32 %b) {
818+
; SDISEL-LABEL: cmn_swap:
819+
; SDISEL: // %bb.0:
820+
; SDISEL-NEXT: cmn w0, w1
821+
; SDISEL-NEXT: cset w0, lt
822+
; SDISEL-NEXT: ret
823+
;
824+
; GISEL-LABEL: cmn_swap:
825+
; GISEL: // %bb.0:
826+
; GISEL-NEXT: cmn w1, w0
827+
; GISEL-NEXT: cset w0, lt
828+
; GISEL-NEXT: ret
829+
%sub = sub nsw i32 0, %b
830+
%cmp = icmp sgt i32 %sub, %a
831+
ret i1 %cmp
832+
}
833+
834+
831835
define i1 @cmn_nsw_neg_64(i64 %a, i64 %b) {
832836
; CHECK-LABEL: cmn_nsw_neg_64:
833837
; CHECK: // %bb.0:

0 commit comments

Comments
 (0)