[PATCH] D157462: LoopRotate: Add code to update branch weights

[Matthias Braun via Phabricator via llvm-commits]

MatzeB marked an inline comment as done.
MatzeB added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:302
+      // probabilities as if 0-trip count never happens.
+      ExitCount0 = 0;
+    } else {
----------------
MatzeB wrote:
> wenlei wrote:
> > MatzeB wrote:
> > > wenlei wrote:
> > > > Heuristic is somewhat arbitrary anyways but I would still keep `ExitCount0` as non-zero, so it's a "live" path to be conservative. Also BFI doesn't like zero edge counts is another reason.. 
> > > I had that in earlier versions, but my experience while writing the tests anyway was that BFI handles zero-count-edges fine and the results just seem more natural and easier to reason about this way. While assigning non-zero immediately felt somewhat murky in what sort of "epsilon" should be used at all...
> > Problem with BFI often gets exposed with larger CFG that has implicit constraints, so small test case doesn't always show. Is there any specific problem you ran into with having ExitCount0 as 1? 1 should be a reasonable guess too. There are many places where we try to avoid zero when uncertain, which is sort of an unspoken convention.  
> The question when setting this to "1" is how you set the other case of the condbr. We can't just leave the other side as-is or risk ending up with ratios like 1:2 where the "1" clearly isn't a small number anymore.
> 
> But for putting something else here, do I go for 1:20, 1:100, 1:1000? I don't think I have a good feeling/guidance for that...
I now added some scaling logic and the code attempts to hit a 1:128 or higher ratio for the "nearly never taken" edge.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157462