[PATCH] D120232: [SelectOpti][4/5] Loop Heuristics

Sotiris Apostolakis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 4 08:55:18 PDT 2022


apostolakis added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectOptimize.cpp:745
+        InstCostMap[&I] = {IPredCost, INonPredCost};
+        MaxCost.PredCost = std::max(MaxCost.PredCost, IPredCost);
+        MaxCost.NonPredCost = std::max(MaxCost.NonPredCost, INonPredCost);
----------------
bsmith wrote:
> apostolakis wrote:
> > bsmith wrote:
> > > apostolakis wrote:
> > > > davidxl wrote:
> > > > > Should Sum (of cost in a SI group) be used instead of max?
> > > > This computes the critical path which essentially is the cost of the last instruction of the critical path (and thus the instruction with the highest latency). This is regardless of how many select groups are included in the loop. These groups may or may not be part of the path. 
> > > Is this really what we want though? As an example, a workload I'm looking at has a structure akin to:
> > > 
> > > ```
> > > ----------------------------critical path
> > >      \------------select-store
> > >      \------------select-store
> > >      \------------select-store
> > >      \------------select-store
> > >      \------------select-store
> > > ```
> > > 
> > > None of the selects are on the critical path, however they are still significant in terms of cost. This may be for a future patch that looks at resources, but it may be better to cost each path with a select on it individually, rather than costing the loop as a whole.
> > There are definitely more cases that we might want to consider beyond the critical path. Yet, focusing on the critical path for this initial series of this new optimization will help catch the most obvious patterns and reduce the chances of regressions. Cases such as the one you mention will indeed need modeling of the resources and more testing, since there are definitely cases where you want to avoid a branch for non-critical-path selects. 
> > 
> > I could add an option to disable the loop-level heuristics (that checks the reduction in the critical path, see line 462) and thus reduce the decision to the cost of each individual select (see line 465 and 478). This will allow experimenting with some of these cases. These costs will capture the gains for all the paths but it will not explicitly capture the total cost of each path. For your case, it will report the gain of converting the selects to branches (new vs old select cost) but it will not report the total cost of each path, i.e., select_cost + store_cost. Could try to add monitoring of all the local maximum in the loop and report them if you want.
> An option to disable the critical path check would be good in the meantime I think. If I manually remove that check then this patch series, alongside the other patch you have to sink instructions, provide a very significant speedup for my case.
Added the option to disable the critical path check. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120232



More information about the llvm-commits mailing list