[PATCH] D57779: [SLP] Add support for throttling.

[Dinar Temirbulatov via Phabricator via llvm-commits]

dtemirbulatov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6419
       int Cost = R.getTreeCost();
+      unsigned UserCost = 0;
       CandidateFound = true;
----------------
RKSimon wrote:
> dtemirbulatov wrote:
> > ABataev wrote:
> > > Do you really need this new var here? I don't see where it is used except as an argument of `R.findSubTree(UserCost)` call
> > I think we need to compensate the ExctractCost with that cost of the insert sequence as in case of full-vectorization.
> This still looks wrong - isn't the UserCost only used locally in the CompensateUseCost path?
No, there is another instance of UserCost at 6476, We have to compare the cost to SLPCostThreshold inside findSubTree() and subtract UserCost.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4147-4154
+  SmallString<256> Str;
+  raw_svector_ostream OS(Str);
+  OS << "SLP: Spill Cost = " << SpillCost << ".\n"
+     << "SLP: Extract Cost = " << ExtractCost << ".\n"
+     << "SLP: Total Cost = " << Cost << ".\n";
   LLVM_DEBUG(dbgs() << Str);
   if (ViewSLPTree)
----------------
RKSimon wrote:
> ABataev wrote:
> > All this code must be active only when the debug mode on?
> Maybe pull this NDEBUG change out into its own patch? 
yes, it could go as NFC.


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

https://reviews.llvm.org/D57779