[PATCH] D90445: [SLP] Make SLPVectorizer to use `llvm.masked.gather` intrinsic

[Anton Afanasyev via Phabricator via llvm-commits]

anton-afanasyev marked 9 inline comments as done.
anton-afanasyev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1581
+    /// Boolean value indicating that pointer operands are scattered.
+    bool IsScatteredOps = false;
+
----------------
ABataev wrote:
> I would think about adding a new `EntryState` instead of adding a new data member.
I've thought about it and discussed this with @dtemirbulatov (his throttling patch is extending `EntryState` as well). I've come that `IsScattered` is like `ReuseShuffleIndices`/`ReorderIndices` (data members) more than an "entry state". Also, the current entry state `NeedToGather` would add ambiguity in that case.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:3702
+            Instruction::Load, VecTy, cast<LoadInst>(VL0)->getPointerOperand(),
+            false, alignment, CostKind, VL0);
+      }
----------------
ABataev wrote:
> Comment for `false` argument
Ok.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4542
+        Value *Val0 = LI->getPointerOperand();
+        FixedVectorType *VecTy =
+            FixedVectorType::get(Val0->getType(), E->Scalars.size());
----------------
ABataev wrote:
> `auto *`
Ok


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4544
+            FixedVectorType::get(Val0->getType(), E->Scalars.size());
+        Value *VecPtr = UndefValue::get(VecTy);
+        unsigned InsIndex = 0;
----------------
ABataev wrote:
> `auto *`
`UndefValue::get()` returns `UndefValue*` type, but we need `Value*` here.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4551
+          if (getTreeEntry(PO))
+            ExternalUses.push_back(
+                ExternalUser(PO, cast<User>(VecPtr), InsIndex));
----------------
ABataev wrote:
> `.emplace_back(PO, cast<User>(VecPtr), InsIndex);`
Ok


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4556
+        Instruction *MG = Builder.CreateMaskedGather(VecPtr, LI->getAlign());
+        V = propagateMetadata(MG, E->Scalars);
+      }
----------------
ABataev wrote:
> Try to merge this code line with the one in line 4539
Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90445