[PATCH] D115457: [AArch64] Convert sra(X, elt_size(X)-1) to cmlt(X, 0)

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

Can you add a reasoning to the commit message? As far as I understand, the CMLT has a higher throughput on many cpus than the sshr.



================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:11229
     if (isVShiftRImm(Op.getOperand(1), VT, false, Cnt) && Cnt < EltSize) {
+      const bool Accumulates = any_of(Op.getNode()->uses(), [](SDNode *N) {
+        return N->getOpcode() == ISD::ADD;
----------------
This add exemption makes it a bit awkward. Do we need to convert this early, or could we just add tablegen patterns and let it pick the "best" one for the given code, like it is fairly descent at.

Something like this, for each type, might be enough:
```
def : Pat<(v16i8 (AArch64vashr (v16i8 V128:$Rn), (i32 7))),
          (CMLTv16i8rz V128:$Rn)>;
```


================
Comment at: llvm/test/CodeGen/AArch64/arm64-vshr.ll:49
 
-define <1 x i64> @sshr_v1i64(<1 x i64> %A) nounwind {
-; CHECK-LABEL: sshr_v1i64:
-; CHECK: sshr d0, d0, #63
+define <1 x i64> @cmlt_v1i64(<1 x i64> %A) nounwind {
+; CHECK-LABEL: cmlt_v1i64:
----------------
I would leave the old test name, or maybe change the constant so it's no longer 63 and still tests a sshr is produced.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115457/new/

https://reviews.llvm.org/D115457