[PATCH] D140100: [SLP]Integrate looking through shuffles logic into ShuffleInstructionBuilder.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6449
+  template <typename ShuffleBuilderTy>
+  static Value *createShuffle(Value *V1, Value *V2, ArrayRef<int> Mask,
+                              ShuffleBuilderTy &Builder) {
----------------
vdmitrie wrote:
> ABataev wrote:
> > vdmitrie wrote:
> > > Could you share some insights why you put this into BaseShuffleAnalysis class method rather than into ShuffleInstructionBuilder? Since it produces new code technically it is not a pure analysis method. And why you want it to be a template (it seems to be instantiated just once with  ShuffleIRBuilder)?
> > It will be used for ShuffleCostEstimator too to avoid duplication/diffs in analysis/emission and cost estimation logic.
> Ah, I see. There is actually a comment to the class saying that, which I overlooked. So you are going to produce another class from it which will serve as cost estimator. Perhaps having "Analysis" as a part of its name isn't quite accurate. It probably makes sense to rename it to something like ShuffleBuilderBase to avoid confusion in future.
It will be used not only for ShuffleBuilder, but for ShuffleCostEstimator, that's why I named it just ShuffleBaseAnalaysis


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140100