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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:1754
+                          TreeEntry::EntryState State = TreeEntry::Vectorize) {
     bool Vectorized = (bool)Bundle;
     VectorizableTree.push_back(std::make_unique<TreeEntry>(VectorizableTree));
----------------
anton-afanasyev wrote:
> ABataev wrote:
> > It may conflict with `State` param value and may lead to incorrect undrstanding/decisions in future. That's why I proposed to merge `EntryState` and `Bundle` into one `Optional<std::pair<>>`, though this solution also is not quite optimal.
> > Maybe better to split this function into 2 overloaded copies, one for gather and one for vectorized state?
> Would it be better to leave all as it was before and just add `setEntryState()` method to accurately set `ScatterVectorize` (or other state in future)?
Nah, it is similar to what was before. Better to set the correct state when constructing the state rather than construct it with the incorrect flag and then trying to fix it on-the-fly. It is bad design decision and always a source of bugs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90445