[PATCH] D132590: [SLP] Try to match reductions first in a vector build sequence.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:11812
 
-bool SLPVectorizerPass::vectorizeInsertValueInst(InsertValueInst *IVI,
-                                                 BasicBlock *BB, BoUpSLP &R) {
----------------
ABataev wrote:
> Why did you decide to remove these functions and embed their code instead?
Both methods are only called (and make sense) in context of vector build sequence and that is already ensured by skipping them if findBuildAggregate  did not detect a sequence in the loop where they used. If we keep the call inside the methods it will be redundant. If remove that call from both of the methods they would become non self sufficient , i.e. would rely on assumption that they are called within certain context. But assumptions are dangerous things.
As all that context is contained within just a small loop I decided that for better clarity it is be better to submerge these methods into the loop rather than keep them with assumptions (or add an ugly assertions to show that assumption).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132590