[PATCH] D119536: [SLP] Extract intermediate insertelement for external use

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5690-5711
     if (!isa_and_nonnull<InsertElementInst>(EU.User) &&
         !ExtractCostCalculated.insert(EU.Scalar).second)
       continue;
 
     // Uses by ephemeral values are free (because the ephemeral value will be
     // removed prior to code generation, and so the extraction will be
     // removed as well).
----------------
anton-afanasyev wrote:
> ABataev wrote:
> > anton-afanasyev wrote:
> > > ABataev wrote:
> > > > What if the same insertelement is used in several users? Would not we miss some cost here?
> > > Why? See line 5924 above, "We only add extract cost once for the same scalar", comment states.
> > Currently it is not quite correct for scalars (too optimistic) but will be fixed in future, once improved reductions support is committed. But for insertelements it is not correct for sure. If one instruction is a part of several build vector sequences, you need to calculate the shuffles for each of them independently. Same applies to the final vector emission. 
> If one insertelement is a part of several build vector sequences, it is enough to calculate the only one shuffle for any of them, isn't it? Or do you mean issue with scheduling of new shuffles?
I think you need to calculate the cost for each buildvector sequence, this insertelement is part of. You need to shuffle the resulting vector with n other vectors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119536