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

Mon Feb 27 17:34:43 PST 2023

vdmitrie marked 2 inline comments as done.
vdmitrie added inline comments.

================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7183
+      if (MultipleUseGEPs.size() == Ptrs.size())
+        VecCost = ScalarCost;
+      else
----------------
RKSimon wrote:
> 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?
This is just a short cut. If we have all the scalar GEPs to remain in vectorized code then vector cost would be the same. We could just return TCC_Free here but as you noted I'd like to have costs printed.

================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:7211
+
+    return VecCost - ScalarCost;
   };
----------------
ABataev wrote:
> RKSimon wrote:
> > It's amazing how even a patch that tries to pull functionality out of SLP increases the size and complexity of SLPVectorizer.cpp :-( 
> It not just tries to pull something out, it introduces new interfaces, which will improve cost modeling. This, of course, may increase the complexity. Unfortunate, but required.
I tried to add/reword some comments. May be that helps a bit.
We need to sink this target specific cost estimation down to TTI somehow (hence enable target specific implementations). I'm open for any suggestions that could help to solve this issue better way.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144770