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

[Wenlei He via Phabricator via llvm-commits]

wenlei added a comment.

I think the heuristics are better than doing nothing in theory. Though one other thing to note is that for AutoFDO, if the profiled build also has the loop rotated, the incoming counts won't be right to begin with -- if the two branches after rotate are executed A times and B times, the original branch should be executed A+B times, but AutoFDO will annotate it with max(A,B) times for the original branch.

You may want to double check that for loop rotate happening in pass1 profiling build, the heuristic also generates reasonable counts comparing to ground truth.



================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:289
+  //  - y0 == x - x0 == x1     # how often loop was entered at all.
+  //  - y1 == y - x0
+  // Unfortunately we cannot generally deduce the ratio between x0 and x1 and
----------------
did you mean `y1 == y - y0 # how many iterations loop body was executed`? 


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:290
+  //  - y1 == y - x0
+  // Unfortunately we cannot generally deduce the ratio between x0 and x1 and
+  // have to guess it.
----------------
Yeah, this is what i meant by saying "loop rotate count is unfixable"..


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:302
+      // probabilities as if 0-trip count never happens.
+      ExitCount0 = 0;
+    } else {
----------------
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.. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157462