Skip to content

Commit 60ceb0e

Browse files
committed
[AArch64] Signed comparison using CMN is safe when the subtraction is nsw
nsw means no signed wrap, and 0 - INT_MIN is a signed wrap. Now, this is going to be a point I need to get out of the way: So is it okay to always transform a > -b into cmn if it is a signed comparison, even if b is INT_MIN because -INT_MIN is undefined, at least in C. Taking advantage of UB doesn't introduce bugs. But, this could change behavior in programs with UB.
1 parent 8fbdcf2 commit 60ceb0e

File tree

2 files changed

+17
-8
lines changed

2 files changed

+17
-8
lines changed

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3348,8 +3348,19 @@ bool isLegalCmpImmed(APInt C) {
33483348
return isLegalArithImmed(C.abs().getZExtValue());
33493349
}
33503350

3351-
static bool cannotBeIntMin(SDValue CheckedVal, SelectionDAG &DAG) {
3352-
KnownBits KnownSrc = DAG.computeKnownBits(CheckedVal);
3351+
static bool isSafeSignedCMN(SDValue Op, SelectionDAG &DAG) {
3352+
// 0 - INT_MIN sign wraps, so no signed wrap means cmn is safe.
3353+
if (Op->getFlags().hasNoSignedWrap())
3354+
return true;
3355+
3356+
// Created nodes may have no flags. SelectionDAG inherits flags from the IR,
3357+
// but if the DAG makes the node, as of now it doesn't set any flags.
3358+
3359+
// FIXME: We can remove this check and simply rely on
3360+
// Op->getFlags().hasNoSignedWrap() if SelectionDAG/ISelLowering never creates
3361+
// subtract nodes, or SelectionDAG/ISelLowering consistently sets them
3362+
// appropiately when making said nodes.
3363+
KnownBits KnownSrc = DAG.computeKnownBits(Op.getOperand(1));
33533364
return !KnownSrc.getSignedMinValue().isMinSignedValue();
33543365
}
33553366

@@ -3358,7 +3369,7 @@ static bool cannotBeIntMin(SDValue CheckedVal, SelectionDAG &DAG) {
33583369
// can be set differently by this operation. It comes down to whether
33593370
// "SInt(~op2)+1 == SInt(~op2+1)" (and the same for UInt). If they are then
33603371
// everything is fine. If not then the optimization is wrong. Thus general
3361-
// comparisons are only valid if op2 != 0.
3372+
// comparisons are only valid if op2 != 0 and op2 != INT_MIN.
33623373
//
33633374
// So, finally, the only LLVM-native comparisons that don't mention C or V
33643375
// are the ones that aren't unsigned comparisons. They're the only ones we can
@@ -3367,7 +3378,7 @@ static bool isCMN(SDValue Op, ISD::CondCode CC, SelectionDAG &DAG) {
33673378
return Op.getOpcode() == ISD::SUB && isNullConstant(Op.getOperand(0)) &&
33683379
(isIntEqualitySetCC(CC) ||
33693380
(isUnsignedIntSetCC(CC) && DAG.isKnownNeverZero(Op.getOperand(1))) ||
3370-
(isSignedIntSetCC(CC) && cannotBeIntMin(Op.getOperand(1), DAG)));
3381+
(isSignedIntSetCC(CC) && isSafeSignedCMN(Op, DAG)));
33713382
}
33723383

33733384
static SDValue emitStrictFPComparison(SDValue LHS, SDValue RHS, const SDLoc &dl,

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -606,8 +606,7 @@ define i1 @almost_immediate_neg_ugt_64(i64 %x) {
606606
define i1 @cmn_nsw(i32 %a, i32 %b) {
607607
; CHECK-LABEL: cmn_nsw:
608608
; CHECK: // %bb.0:
609-
; CHECK-NEXT: neg w8, w1
610-
; CHECK-NEXT: cmp w0, w8
609+
; CHECK-NEXT: cmn w0, w1
611610
; CHECK-NEXT: cset w0, gt
612611
; CHECK-NEXT: ret
613612
%sub = sub nsw i32 0, %b
@@ -618,8 +617,7 @@ ret i1 %cmp
618617
define i1 @cmn_nsw_64(i64 %a, i64 %b) {
619618
; CHECK-LABEL: cmn_nsw_64:
620619
; CHECK: // %bb.0:
621-
; CHECK-NEXT: neg x8, x1
622-
; CHECK-NEXT: cmp x0, x8
620+
; CHECK-NEXT: cmn x0, x1
623621
; CHECK-NEXT: cset w0, gt
624622
; CHECK-NEXT: ret
625623
%sub = sub nsw i64 0, %b

0 commit comments

Comments
 (0)