[PATCH] D121449: [AArch64] Combine ISD::SETCC into AArch64ISD::ANDS

[chenglin.bi via Phabricator via llvm-commits]

bcl5980 added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17311
+    EVT TstVT = LHS->getValueType(0);
+    if (TstVT.getFixedSizeInBits() <= 64) {
+      // this pattern will get better opt in emitComparison
----------------
paulwalker-arm wrote:
> This test is fragile because for example it will assert when `TstVT` is a scalable vector, whilst also return true for `v2i32`, which I doubt you want. I'm guessing it's only the `isa<ConstantSDNode>(LHS->getOperand(1))` condition that's saving you here.
> 
> Given you're emitting general ISD nodes do you really care what the type is? If not then perhaps you just need `if (TstVTisScalarInteger())`?
Thanks for the mention. ScalarInterger check is necessary. 
We can get benifit from SRL pattern when the data type is i128. This check is only used for avoid i128.
How about TstVT.getFixedSizeInBits() <= 64 && TstVT.isScalarInteger() ?


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

https://reviews.llvm.org/D121449