[llvm] [AArch64] Signed comparison using CMN is safe when the subtraction is nsw (PR #141993)

via llvm-commits llvm-commits at lists.llvm.org
Fri May 30 09:30:02 PDT 2025


https://github.com/AZero13 updated https://github.com/llvm/llvm-project/pull/141993

>From 8fbdcf281bd4667e649b3b494063447982499887 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
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 60ceb0e302ffa52d9cdd6c0879bc98fd1218ba4c Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
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.

Taking advantage of UB doesn't introduce bugs. But, this could change behavior in programs with UB.
---
 .../Target/AArch64/AArch64ISelLowering.cpp    | 19 +++++++++++++++----
 llvm/test/CodeGen/AArch64/cmp-to-cmn.ll       |  6 ++----
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 349bcd95c09f6..bb17911251114 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3348,8 +3348,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;
+
+  // Created nodes may have no flags. SelectionDAG inherits flags from the IR,
+  // but if the DAG makes the node, as of now it doesn't set any flags.
+
+  // FIXME: We can remove this check and simply rely on
+  // Op->getFlags().hasNoSignedWrap() if SelectionDAG/ISelLowering never creates
+  // subtract nodes, or SelectionDAG/ISelLowering consistently sets them
+  // appropiately when making said nodes.
+  KnownBits KnownSrc = DAG.computeKnownBits(Op.getOperand(1));
   return !KnownSrc.getSignedMinValue().isMinSignedValue();
 }
 
@@ -3358,7 +3369,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
@@ -3367,7 +3378,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..be950b7f8ae4f 100644
--- a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll
+++ b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll
@@ -606,8 +606,7 @@ 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
@@ -618,8 +617,7 @@ 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



More information about the llvm-commits mailing list