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

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm 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
----------------
bcl5980 wrote:
> 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() ?
Works for me but please reverse the tests so that we only call `getFixedSizeInBits` once we know `TstVT` is a scalar integer.


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

https://reviews.llvm.org/D121449