[PATCH] D86346: [SimplifyCFG] Accumulate cost against budget

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri requested changes to this revision.
lebedev.ri added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:2051
   int BudgetRemaining =
     PHINodeFoldingThreshold * TargetTransformInfo::TCC_Basic;
   for (PHINode &PN : EndBB->phis()) {
----------------
samparker wrote:
> lebedev.ri wrote:
> > Please add a new `cl::opt` for that.
> > I believe, the threshold should be the branch misprediction cost,
> > i.e. if it takes less cost (latency) to execute the branch
> > then to wrongly predict that it won't be executed,
> > but then rewind and execute it anyways, we should just execute it.
> > 
> > So i'd go with `20`.
> Most of the cost modelling framework actually tries to assume that branches are always predicated though, and I don't understand why making arbitrary decisions about machine details would be a good idea here. Plus, it's also completely irrelevant to code size...  It would seem to be better to query the latency / size of the branch, but I highly doubt that would produce results that anyone would want...
I'll rephrase: given that the cost modelling logic changed,
the remaining value of `PHINodeFoldingThreshold` is completely wrong.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86346