This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SelectionDAG] Perfer CMN for (0 - Y) == Y
AbandonedPublic

Authored by Allen on Feb 18 2023, 12:49 AM.

Details

Summary

cmn a, b means a - (-b) - which under two's complement arithmetic
is exactly equivalent to a + b. so Y == -Y is just match the cmn Y, Y

Fix https://github.com/llvm/llvm-project/issues/60818

Diff Detail

Event Timeline

Allen created this revision.Feb 18 2023, 12:49 AM
Allen requested review of this revision.Feb 18 2023, 12:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 18 2023, 12:49 AM
Allen updated this revision to Diff 498569.Feb 18 2023, 12:50 AM

update comment

Allen planned changes to this revision.Feb 18 2023, 1:12 AM

We should probably view this as a missed canonicalization in IR first:
https://alive2.llvm.org/ce/z/D2Aph4 (that should also work with eq predicates)

The IR with and removes a use of the input variable, so it's better for analysis.
If the codegen here is still considered better, then add a transform from the new canonical form to the cmn form.

Allen added a comment.Feb 20 2023, 5:13 PM

Yes, Agree your idea, thanks @spatel. I also realize that the cmn directive may not exist on all backends? so this is not appropriate.

I try to add a patten match in performSETCCCombine , but then it go Into an infinite, so I'm trying to add a codegen pattern for this issue.

+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -20436,6 +20436,20 @@ static SDValue performSETCCCombine(SDNode *N,
     }
   }
 
+  // setcc (shl x, 1), 0, ne ==> setcc (sub 0, x), x, ne for CMN
+  if (Cond == ISD::SETNE && isNullConstant(RHS) &&
+      LHS->getOpcode() == ISD::SHL && isa<ConstantSDNode>(LHS->getOperand(1)) &&
+      LHS->getConstantOperandVal(1) == 1 && LHS->hasOneUse()) {
+    EVT TstVT = LHS->getValueType(0);
+    if (TstVT.isScalarInteger() && TstVT.getFixedSizeInBits() <= 64) {
+      // this pattern will get better opt in emitComparison
+      SDValue ValX = LHS->getOperand(0);
+      SDValue TST = DAG.getNode(ISD::SUB, DL, TstVT, DAG.getConstant(0, DL, TstVT),
+                                ValX);
+      return DAG.getNode(ISD::SETCC, DL, VT, TST, ValX, N->getOperand(2));
+    }
+  }
Allen abandoned this revision.Mar 11 2023, 3:02 AM

Agree with @spatel , should be redesign in IR level