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

Alexey Bataev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 16 12:21:31 PST 2022


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



More information about the llvm-commits mailing list