[PATCH] D144770: [SLP] Outline GEP chain cost modeling into new TTI interface - NFCI.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

I'd expect this to cause cost changes - have you had any luck creating tests demonstrating that?



================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:2007
+                                       const Value *Base,
+                                       PointersChainInfo Info,
+                                       TargetCostKind CostKind) override {
----------------
Pass by const ref?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:2816
+                     InstructionCost VecCost, InstructionCost ScalarCost,
+                     const char *Banner = nullptr) const {
+    if (Banner)
----------------
Drop the nullptr (maybe convert to StringRef?) and add "Calculated costs for Tree" to the only existing dumpTreeCosts call


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7183
+      if (MultipleUseGEPs.size() == Ptrs.size())
+        VecCost = ScalarCost;
+      else
----------------
This kind of thing always feels like a cheat - is the only reason you don't return TCC_Free here because we still need to do the dumpTreeCosts call below?


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7211
+
+    return VecCost - ScalarCost;
   };
----------------
It's amazing how even a patch that tries to pull functionality out of SLP increases the size and complexity of SLPVectorizer.cpp :-( 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144770