[PATCH] D98714: [SLP] Add insertelement instructions to vectorizable tree

[Anton Afanasyev via Phabricator via llvm-commits]

anton-afanasyev marked an inline comment as done.
anton-afanasyev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4215-4217
+    // No extract cost for vector "scalar"
+    if (isa<FixedVectorType>(EU.Scalar->getType()))
+      continue;
----------------
ABataev wrote:
> anton-afanasyev wrote:
> > ABataev wrote:
> > > I don't think this is correct. I think you need to use code from my patch D101555 for better cost estimation here.
> > Why? We don't generate any extract instructions for external using of vector, so no need to cost it.
> > 
> > I think you mixed two different cases up.
> > The patch D101555 you referenced is about cost estimation when insert is _user_, but here is the case when insert is _used_. We do not really need to "extract" it, since its user uses vector value rather than scalar one.
> > 
> > Also I don't think we need to use code from D101555 in this patch, since it does the same by the other way. The main idea of this patch is to unify the way we process inserts (the only vector tree node for now) vs ordinary tree nodes. The inserts are tree node now, and they are sorted by index, so no need to shuffle them. We could use `ReorderIndicies` if needed, but no need, since operands are sorted as well.
> I think you're missing shuffle cost here. If the external user is a vector and extracted element is inserted into the different lane - it is at least shuffle and need to add its cost.
> I think you're missing shuffle cost here. 
The cost of what shuffle? We don't generate any shuffle.

> If the external user is a vector ...
Not "the external user is a vector", but its operand is a vector. We do not need to extract any special lane, since _whole_ vector is using and replacing after "vectorization". In this special case (when tree node is inserts) we have vector "scalars" (inserts have vector type) and their "vectorization" is just removing (i.e. replacing by vectorized operands).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98714