[PATCH] D126885: [SLP]Cost for a constant buildvector.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:5783
+           E->UserTreeIndices.front().UserTE->isAltShuffle())
+              ? 0
+              : E->UserTreeIndices.front().UserTE->getOpcode();
----------------
ABataev wrote:
> vdmitrie wrote:
> > ABataev wrote:
> > > vdmitrie wrote:
> > > > Just wondering is that possible for UserTreeIndices to be empty here? AFAIU it can be for root only but constants do not seed vtree.
> > > > if alternate opcodes are for shl/shr but shift value is splat it is still can be immediate for both of them.
> > > > 
> > > 1. If constants are reduced values in reduction ops.
> > > 2. That's why there is a TODO above.
> > okay. Although I believe it is not SLP vectorizer job to do constant folding.
> Do you suggest to hide it in getConstBuildVectorInstrCost? And return the difference? Or just add a new member function?
Alternate opcodes is SLP vectorizer specific. For that reason trying to sink that logic into inside the TTI interface does not look like right thing to do.
But outlining this whole new code into a separate member is a good idea.

What sounds weird for me is that constants may seed vtree for reduction. Although that is not directly related to this patch but you are placing here work arounds of that. IMO it is unpractical to run constants reduction through SLP vectorizer machinery. 

Probably, to make the work around of that issue simpler in this patch, add an early return:
if (E->UserTreeIndices.empty())
  return 0;

Otherwise it will be returning memory-op cost for a foldable operation.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126885