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

mgabka via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 26 03:07:07 PDT 2023

mgabka added inline comments.

Comment at: llvm/lib/CodeGen/InterleavedAccessPass.cpp:116
                              SmallVector<Instruction *, 32> &DeadInsts);
+  bool lowerDeinterleaveIntrinsic(IntrinsicInst *II,
perhaps adding a comment to this and the other function below would be a good idea, at least consistent with the other lowerinterleaved* functions

Comment at: llvm/lib/CodeGen/InterleavedAccessPass.cpp:477
+    IntrinsicInst *II, SmallVector<Instruction *, 32> &DeadInsts) {
+  if (!II->hasOneUse())
+    return false;
could you explain why you are checking it here but not for lowerDeinterleaveIntrinsic?

Comment at: llvm/lib/CodeGen/InterleavedAccessPass.cpp:521
+    if (auto *II = dyn_cast<IntrinsicInst>(&I)) {
+      if (II->getIntrinsicID() == Intrinsic::experimental_vector_deinterleave2)
+        Changed |= lowerDeinterleaveIntrinsic(II, DeadInsts);
I think it is worth to add a comment here explaining why we restrict this operation only to VF=2

Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14765
+  if (EC.isScalable()) {
+    UseScalable = true;
+    return isPowerOf2_32(MinElts) && (MinElts * ElSize) % 128 == 0;
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

Comment at: llvm/test/Transforms/InterleavedAccess/AArch64/fixed-deinterleave-intrinsics.ll:3
+; RUN: opt < %s -interleaved-access -S | FileCheck %s --check-prefix=NEON
+; RUN: opt < %s -interleaved-access -mtriple=aarch64-linux-gnu -mattr=+sve -force-streaming-compatible-sve -S | FileCheck %s --check-prefix=SVE-FIXED
is "-force-streaming-compatible-sve" needed here? I thought that this transformation should happen always for sve, the tests for scalable vectorization have only "target-features"="+sve" added. am I missing something?



More information about the llvm-commits mailing list