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

Anton Afanasyev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 4 23:57:41 PST 2020


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


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2848
+        // Vectorizing non-consecutive loads with `llvm.masked.gather`.
+        TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
+                                     ReuseShuffleIndicies);
----------------
ABataev wrote:
> Why it is `vectorized` if this is actually kind of gather?
Actually I think `vectorized` is more appropriate here than `gathered` in sense it is used throughout SLPVectorizer module. Since we actually modify several instruction to _one_. I can change it to `scatter-vectorized`.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1581
+    /// Boolean value indicating that pointer operands are scattered.
+    bool IsScatteredOps = false;
+
----------------
ABataev wrote:
> anton-afanasyev wrote:
> > 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.
> It is different thing. `ReuseShuffleIndices/ReorderIndices` can be used by many different entries while `IsScattered` member is used only for a single particular kind of node. It increases memory usage because we add an extra field which is just ignored in many cases. Plus, to me, this field just a mark of the new kind of gather.
Ok, sounds convincing, I can change it. But I don't think this field is "just the new kind of gather". I'm planning to use `IsScattered` for scattered stores (`@llvm.store.scatter` intrinsic) at the next step, of course. This is not the gather like `NeedToGather` entry state.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90445



More information about the llvm-commits mailing list