From c496f19ca9833ab4ccebf9f999f2f5723bd72c27 Mon Sep 17 00:00:00 2001 From: Rose Date: Thu, 29 May 2025 12:52:29 -0400 Subject: [PATCH 1/2] [AArch64] Pre-commit tests (NFC) --- llvm/test/CodeGen/AArch64/cmp-to-cmn.ll | 48 +++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll index c5fd9b63cce97..dddeccadf2443 100644 --- a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll +++ b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll @@ -602,3 +602,51 @@ define i1 @almost_immediate_neg_ugt_64(i64 %x) { %cmp = icmp ugt i64 %x, -16773121 ret i1 %cmp } + +define i1 @cmn_nsw(i32 %a, i32 %b) { +; CHECK-LABEL: cmn_nsw: +; CHECK: // %bb.0: +; CHECK-NEXT: neg w8, w1 +; CHECK-NEXT: cmp w0, w8 +; CHECK-NEXT: cset w0, gt +; CHECK-NEXT: ret +%sub = sub nsw i32 0, %b +%cmp = icmp sgt i32 %a, %sub +ret i1 %cmp +} + +define i1 @cmn_nsw_64(i64 %a, i64 %b) { +; CHECK-LABEL: cmn_nsw_64: +; CHECK: // %bb.0: +; CHECK-NEXT: neg x8, x1 +; CHECK-NEXT: cmp x0, x8 +; CHECK-NEXT: cset w0, gt +; CHECK-NEXT: ret +%sub = sub nsw i64 0, %b +%cmp = icmp sgt i64 %a, %sub +ret i1 %cmp +} + +define i1 @cmn_nsw_neg(i32 %a, i32 %b) { +; CHECK-LABEL: cmn_nsw_neg: +; CHECK: // %bb.0: +; CHECK-NEXT: neg w8, w1 +; CHECK-NEXT: cmp w0, w8 +; CHECK-NEXT: cset w0, gt +; CHECK-NEXT: ret +%sub = sub i32 0, %b +%cmp = icmp sgt i32 %a, %sub +ret i1 %cmp +} + +define i1 @cmn_nsw_neg_64(i64 %a, i64 %b) { +; CHECK-LABEL: cmn_nsw_neg_64: +; CHECK: // %bb.0: +; CHECK-NEXT: neg x8, x1 +; CHECK-NEXT: cmp x0, x8 +; CHECK-NEXT: cset w0, gt +; CHECK-NEXT: ret +%sub = sub i64 0, %b +%cmp = icmp sgt i64 %a, %sub +ret i1 %cmp +} From c9cb9f88598595e12b244e65d0cc6ccb5387b146 Mon Sep 17 00:00:00 2001 From: Rose Date: Thu, 29 May 2025 13:27:19 -0400 Subject: [PATCH 2/2] [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, because unless fwrapv is specified, opt puts nsw on signed integer operations, allowing for more folds anyway. --- .../Target/AArch64/AArch64ISelLowering.cpp | 19 +++++++++--- llvm/test/CodeGen/AArch64/cmp-to-cmn.ll | 30 +++++++++---------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp index ac545534d728b..5b9e699eaa408 100644 --- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp @@ -3392,8 +3392,19 @@ bool isLegalCmpImmed(APInt C) { return isLegalArithImmed(C.abs().getZExtValue()); } -static bool cannotBeIntMin(SDValue CheckedVal, SelectionDAG &DAG) { - KnownBits KnownSrc = DAG.computeKnownBits(CheckedVal); +static bool isSafeSignedCMN(SDValue Op, SelectionDAG &DAG) { + // 0 - INT_MIN sign wraps, so no signed wrap means cmn is safe. + if (Op->getFlags().hasNoSignedWrap()) + return true; + + // We can still figure out if the second operand is safe to use + // in a CMN instruction by checking if it is known to be not the minimum + // signed value. If it is not, then we can safely use CMN. + // Note: We can eventually remove this check and simply rely on + // Op->getFlags().hasNoSignedWrap() once SelectionDAG/ISelLowering + // consistently sets them appropriately when making said nodes. + + KnownBits KnownSrc = DAG.computeKnownBits(Op.getOperand(1)); return !KnownSrc.getSignedMinValue().isMinSignedValue(); } @@ -3402,7 +3413,7 @@ static bool cannotBeIntMin(SDValue CheckedVal, SelectionDAG &DAG) { // can be set differently by this operation. It comes down to whether // "SInt(~op2)+1 == SInt(~op2+1)" (and the same for UInt). If they are then // everything is fine. If not then the optimization is wrong. Thus general -// comparisons are only valid if op2 != 0. +// comparisons are only valid if op2 != 0 and op2 != INT_MIN. // // So, finally, the only LLVM-native comparisons that don't mention C or V // are the ones that aren't unsigned comparisons. They're the only ones we can @@ -3411,7 +3422,7 @@ static bool isCMN(SDValue Op, ISD::CondCode CC, SelectionDAG &DAG) { return Op.getOpcode() == ISD::SUB && isNullConstant(Op.getOperand(0)) && (isIntEqualitySetCC(CC) || (isUnsignedIntSetCC(CC) && DAG.isKnownNeverZero(Op.getOperand(1))) || - (isSignedIntSetCC(CC) && cannotBeIntMin(Op.getOperand(1), DAG))); + (isSignedIntSetCC(CC) && isSafeSignedCMN(Op, DAG))); } static SDValue emitStrictFPComparison(SDValue LHS, SDValue RHS, const SDLoc &dl, diff --git a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll index dddeccadf2443..5765e0acae269 100644 --- a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll +++ b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll @@ -606,25 +606,23 @@ define i1 @almost_immediate_neg_ugt_64(i64 %x) { define i1 @cmn_nsw(i32 %a, i32 %b) { ; CHECK-LABEL: cmn_nsw: ; CHECK: // %bb.0: -; CHECK-NEXT: neg w8, w1 -; CHECK-NEXT: cmp w0, w8 +; CHECK-NEXT: cmn w0, w1 ; CHECK-NEXT: cset w0, gt ; CHECK-NEXT: ret -%sub = sub nsw i32 0, %b -%cmp = icmp sgt i32 %a, %sub -ret i1 %cmp + %sub = sub nsw i32 0, %b + %cmp = icmp sgt i32 %a, %sub + ret i1 %cmp } define i1 @cmn_nsw_64(i64 %a, i64 %b) { ; CHECK-LABEL: cmn_nsw_64: ; CHECK: // %bb.0: -; CHECK-NEXT: neg x8, x1 -; CHECK-NEXT: cmp x0, x8 +; CHECK-NEXT: cmn x0, x1 ; CHECK-NEXT: cset w0, gt ; CHECK-NEXT: ret -%sub = sub nsw i64 0, %b -%cmp = icmp sgt i64 %a, %sub -ret i1 %cmp + %sub = sub nsw i64 0, %b + %cmp = icmp sgt i64 %a, %sub + ret i1 %cmp } define i1 @cmn_nsw_neg(i32 %a, i32 %b) { @@ -634,9 +632,9 @@ define i1 @cmn_nsw_neg(i32 %a, i32 %b) { ; CHECK-NEXT: cmp w0, w8 ; CHECK-NEXT: cset w0, gt ; CHECK-NEXT: ret -%sub = sub i32 0, %b -%cmp = icmp sgt i32 %a, %sub -ret i1 %cmp + %sub = sub i32 0, %b + %cmp = icmp sgt i32 %a, %sub + ret i1 %cmp } define i1 @cmn_nsw_neg_64(i64 %a, i64 %b) { @@ -646,7 +644,7 @@ define i1 @cmn_nsw_neg_64(i64 %a, i64 %b) { ; CHECK-NEXT: cmp x0, x8 ; CHECK-NEXT: cset w0, gt ; CHECK-NEXT: ret -%sub = sub i64 0, %b -%cmp = icmp sgt i64 %a, %sub -ret i1 %cmp + %sub = sub i64 0, %b + %cmp = icmp sgt i64 %a, %sub + ret i1 %cmp }