[PATCH] D146218: [AArch64][CodeGen] Lower (de)interleave2 intrinsics to ld2/st2

[mgabka via Phabricator via llvm-commits]

mgabka added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14765
+  if (EC.isScalable()) {
+    UseScalable = true;
+    return isPowerOf2_32(MinElts) && (MinElts * ElSize) % 128 == 0;
----------------
huntergr wrote:
> mgabka wrote:
> > is it correct to set it here to true or only if the condition below is true?
> > 
> > from what I can see, before UseScalable was only set to true if this function was returning true
> It's fine -- if the type is not a legal shuffle type, no transformation will be performed.
if callers check correctly for the return value of this function.
However, the UseScalable is already set to false, even if the function returns false, my point is that this function should modify state of UseScalable only if it returns true.
But that is a just my personal preference I guess.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146218