Skip to content

Conversation

@AZero13
Copy link
Contributor

@AZero13 AZero13 commented Jun 14, 2025

Basically, we were failing to take into account the possibility of commuting the cmn, which meant that we failed to counter-act the cmn if it could go on the right-side anyway.

@llvmbot
Copy link
Member

llvmbot commented Jun 14, 2025

@llvm/pr-subscribers-backend-aarch64

Author: AZero13 (AZero13)

Changes

Basically, we were failing to take into account the possibility of commuting the cmn, which meant that we failed to counter-act the cmn if it could go on the right-side anyway.


Full diff: https://github.com/llvm/llvm-project/pull/144234.diff

2 Files Affected:

  • (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+11-2)
  • (modified) llvm/test/CodeGen/AArch64/cmp-to-cmn.ll (+4-4)
diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index 7519ac5260a64..4da51fbcfcb83 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -3436,6 +3436,12 @@ static bool isCMN(SDValue Op, ISD::CondCode CC, SelectionDAG &DAG) {
           (isSignedIntSetCC(CC) && isSafeSignedCMN(Op, DAG)));
 }
 
+static bool canBeCommutedToCMN(SDValue LHS, SDValue RHS, ISD::CondCode CC) {
+  if (LHS.getOpcode() != ISD::SUB || !isNullConstant(LHS.getOperand(0)))
+    return false;
+  return isIntEqualitySetCC(CC);
+}
+
 static SDValue emitStrictFPComparison(SDValue LHS, SDValue RHS, const SDLoc &dl,
                                       SelectionDAG &DAG, SDValue Chain,
                                       bool IsSignaling) {
@@ -3480,8 +3486,7 @@ static SDValue emitComparison(SDValue LHS, SDValue RHS, ISD::CondCode CC,
     // Can we combine a (CMP op1, (sub 0, op2) into a CMN instruction ?
     Opcode = AArch64ISD::ADDS;
     RHS = RHS.getOperand(1);
-  } else if (LHS.getOpcode() == ISD::SUB && isNullConstant(LHS.getOperand(0)) &&
-             isIntEqualitySetCC(CC)) {
+  } else if (canBeCommutedToCMN(LHS, RHS, CC)) {
     // As we are looking for EQ/NE compares, the operands can be commuted ; can
     // we combine a (CMP (sub 0, op1), op2) into a CMN instruction ?
     Opcode = AArch64ISD::ADDS;
@@ -3937,6 +3942,10 @@ static SDValue getAArch64Cmp(SDValue LHS, SDValue RHS, ISD::CondCode CC,
     SDValue TheLHS = LHSIsCMN ? LHS.getOperand(1) : LHS;
     SDValue TheRHS = RHSIsCMN ? RHS.getOperand(1) : RHS;
 
+    // Do not count twice if the CMN can be commuted, hence OR.
+    LHSIsCMN |= canBeCommutedToCMN(RHS, LHS, CC);
+    RHSIsCMN |= canBeCommutedToCMN(LHS, RHS, CC);
+
     if (getCmpOperandFoldingProfit(TheLHS) + (LHSIsCMN ? 1 : 0) >
         getCmpOperandFoldingProfit(TheRHS) + (RHSIsCMN ? 1 : 0)) {
       std::swap(LHS, RHS);
diff --git a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll
index 5765e0acae269..73b7f5b6ebf8e 100644
--- a/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll
+++ b/llvm/test/CodeGen/AArch64/cmp-to-cmn.ll
@@ -6,7 +6,7 @@ target triple = "arm64"
 define i1 @test_EQ_IllEbT(i64 %a, i64 %b) {
 ; CHECK-LABEL: test_EQ_IllEbT:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cmn x0, x1
+; CHECK-NEXT:    cmn x1, x0
 ; CHECK-NEXT:    cset w0, eq
 ; CHECK-NEXT:    ret
 entry:
@@ -70,7 +70,7 @@ entry:
 define i1 @test_EQ_IiiEbT(i32 %a, i32 %b) {
 ; CHECK-LABEL: test_EQ_IiiEbT:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cmn w0, w1
+; CHECK-NEXT:    cmn w1, w0
 ; CHECK-NEXT:    cset w0, eq
 ; CHECK-NEXT:    ret
 entry:
@@ -220,7 +220,7 @@ entry:
 define i1 @test_NE_IllEbT(i64 %a, i64 %b) {
 ; CHECK-LABEL: test_NE_IllEbT:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cmn x0, x1
+; CHECK-NEXT:    cmn x1, x0
 ; CHECK-NEXT:    cset w0, ne
 ; CHECK-NEXT:    ret
 entry:
@@ -284,7 +284,7 @@ entry:
 define i1 @test_NE_IiiEbT(i32 %a, i32 %b) {
 ; CHECK-LABEL: test_NE_IiiEbT:
 ; CHECK:       // %bb.0: // %entry
-; CHECK-NEXT:    cmn w0, w1
+; CHECK-NEXT:    cmn w1, w0
 ; CHECK-NEXT:    cset w0, ne
 ; CHECK-NEXT:    ret
 entry:

@AZero13 AZero13 force-pushed the finally-cmn branch 2 times, most recently from 3b5745c to aecfbb3 Compare June 28, 2025 15:11
@AZero13 AZero13 changed the title [AArch64] Factor in the possible cmn commute by emitComparison [AArch64] Allow commuting if both sides of the comparison are safe to be negated Jun 28, 2025
@AZero13 AZero13 changed the title [AArch64] Allow commuting if both sides of the comparison are safe to be negated [AArch64] Allow commuting the cmn if both sides of the comparison are safe to be negated Jun 28, 2025
@AZero13 AZero13 force-pushed the finally-cmn branch 4 times, most recently from 435ec03 to 9256127 Compare June 28, 2025 18:08
@AZero13 AZero13 changed the title [AArch64] Allow commuting the cmn if both sides of the comparison are safe to be negated [AArch64] Factor in the possible cmn commute by emitComparison Jun 28, 2025
Basically, we were failing to take into account the possibility of commuting the cmn, which meant that we failed to counter-act the cmn if it could go on the right-side anyway.
@AZero13 AZero13 closed this Jul 24, 2025
@AZero13 AZero13 deleted the finally-cmn branch July 24, 2025 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants