[llvm] [AArch64] Signed comparison using CMN is safe when the subtraction is nsw (PR #141993)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 12 08:28:13 PDT 2025
https://github.com/AZero13 updated https://github.com/llvm/llvm-project/pull/141993
>From cf30d69ed94e6f1bbf628a8faa688963eff54c9b 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/7] [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 ea46bf136f0d0bdeb52065e4ed1c71ba991df5a4 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/7] [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 | 20 +++++++++++++++----
llvm/test/CodeGen/AArch64/cmp-to-cmn.ll | 6 ++----
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 121720e7defd4..895783c699153 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3375,8 +3375,20 @@ 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.
+ // FIXME: Can we can remove this check and simply rely on
+ // Op->getFlags().hasNoSignedWrap() once 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();
}
@@ -3385,7 +3397,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
@@ -3394,7 +3406,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
>From 87afca5bda9d46f45f9ff8862f35db834731b2bf Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Mon, 9 Jun 2025 19:41:59 -0400
Subject: [PATCH 3/7] Fix comment
---
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 895783c699153..5db3e732b6755 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3384,9 +3384,8 @@ static bool isSafeSignedCMN(SDValue Op, SelectionDAG &DAG) {
// 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.
// FIXME: Can we can remove this check and simply rely on
- // Op->getFlags().hasNoSignedWrap() once SelectionDAG/ISelLowering never
- // creates subtract nodes, or SelectionDAG/ISelLowering consistently sets them
- // appropiately when making said nodes?
+ // Op->getFlags().hasNoSignedWrap() once SelectionDAG/ISelLowering
+ // consistently sets them appropiately when making said nodes?
KnownBits KnownSrc = DAG.computeKnownBits(Op.getOperand(1));
return !KnownSrc.getSignedMinValue().isMinSignedValue();
>From 3978ed742e52abe03d2c1028aae0e730eda54911 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Tue, 10 Jun 2025 12:43:57 -0400
Subject: [PATCH 4/7] Fix typo
---
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 5db3e732b6755..b2f583e117093 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3385,7 +3385,7 @@ static bool isSafeSignedCMN(SDValue Op, SelectionDAG &DAG) {
// signed value. If it is not, then we can safely use CMN.
// FIXME: Can we can remove this check and simply rely on
// Op->getFlags().hasNoSignedWrap() once SelectionDAG/ISelLowering
- // consistently sets them appropiately when making said nodes?
+ // consistently sets them appropriately when making said nodes?
KnownBits KnownSrc = DAG.computeKnownBits(Op.getOperand(1));
return !KnownSrc.getSignedMinValue().isMinSignedValue();
>From 260515208f92b016abb6f8c484ef1a90a7732f1a Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Wed, 11 Jun 2025 15:58:22 -0400
Subject: [PATCH 5/7] Address comments
---
.../Target/AArch64/AArch64ISelLowering.cpp | 2 +-
llvm/test/CodeGen/AArch64/cmp-to-cmn.ll | 24 +++++++++----------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index b2f583e117093..5f64fc0883f81 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3383,7 +3383,7 @@ static bool isSafeSignedCMN(SDValue Op, SelectionDAG &DAG) {
// 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.
- // FIXME: Can we can remove this check and simply rely on
+ // 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?
diff --git a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll
index be950b7f8ae4f..5765e0acae269 100644
--- a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll
+++ b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll
@@ -609,9 +609,9 @@ define i1 @cmn_nsw(i32 %a, i32 %b) {
; 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) {
@@ -620,9 +620,9 @@ define i1 @cmn_nsw_64(i64 %a, i64 %b) {
; 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) {
@@ -632,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) {
@@ -644,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
}
>From 51ca82f439df1f37a247194d374210ec3305b5fb Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Thu, 12 Jun 2025 09:58:31 -0400
Subject: [PATCH 6/7] Fix punctuation in comment.
---
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 5f64fc0883f81..5363ef8f1b991 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3385,7 +3385,7 @@ static bool isSafeSignedCMN(SDValue Op, SelectionDAG &DAG) {
// 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?
+ // consistently sets them appropriately when making said nodes.
KnownBits KnownSrc = DAG.computeKnownBits(Op.getOperand(1));
return !KnownSrc.getSignedMinValue().isMinSignedValue();
>From 0c1f0bd7278055a96cc8a6c25fa53057faf699b3 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Thu, 12 Jun 2025 11:27:48 -0400
Subject: [PATCH 7/7] Consistency
---
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 5363ef8f1b991..aae5240a71e3e 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3377,14 +3377,14 @@ bool isLegalCmpImmed(APInt C) {
static bool isSafeSignedCMN(SDValue Op, SelectionDAG &DAG) {
// 0 - INT_MIN sign wraps, so no signed wrap means cmn is safe.
- if (Op->getFlags().hasNoSignedWrap())
+ 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
+ // Op.getFlags().hasNoSignedWrap() once SelectionDAG/ISelLowering
// consistently sets them appropriately when making said nodes.
KnownBits KnownSrc = DAG.computeKnownBits(Op.getOperand(1));
More information about the llvm-commits
mailing list