[PATCH] D149993: [SLP][NFC] Cleanup: Separate vectorization of Inserts and CmpInsts.

[Vasileios Porpodas via Phabricator via llvm-commits]

vporpo added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Vectorize/SLPVectorizer.h:148
   /// Tries to vectorize \p CmpInts. \Returns true on success.
-  bool vectorizeCmpInsts(ArrayRef<CmpInst *> CmpInsts, BasicBlock *BB,
+  template <typename RangeT>
+  bool vectorizeCmpInsts(RangeT CmpInsts, BasicBlock *BB,
----------------
ABataev wrote:
> vporpo wrote:
> > ABataev wrote:
> > > Why need a template here since looks like it accepts only ArrayRef?
> > We are passing an `iterator_range` and there is no `ArrayRef` constructor with an `iterator_range` argument.
> > I could use the actual type `iterator_range<std::reverse_iterator<SmallSetVector<CmpInst *, 8>::iterator>>` but it is pretty ugly and if we ever need to remove the reverse order at the call-site we would have to change this type too. Wdyt?
> SmallSetVector class, used to store instruction, has getArrayRef() member function, which can return ArrayRef.
I don't see how getting the ArrayRef from the SmallSetVector would help. We still need to pass an iterator_range as we need to iterate in reverse.
The only way this would work is if we reverse the iteration within `vectorizeCmpInsts` which I don't think we should do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149993