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

[Matthias Braun via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:291-306
+  uint32_t ExitCount0; // aka x0
+  if (!ConditionalPreHeader) {
+    // pre-header turned into an unconditional jump.
+    ExitCount0 = 0;
+  } else {
+    // Here we cannot know how many 0-trip count loops we have, so we guess:
+    if (Weights[1] > Weights[0]) {
----------------
paulkirth wrote:
> This could probably be a helper function. It would also allow for expressive names for the `Weight[0]`/`Weight[1]` values, which will probably make it easier to follow. 
> 
> Alternatively, you could group all of these counts into a struct. I'm not convinced that is worth it in this case, but it may make the code simpler to reason about and keep all the logic regarding the heuristic together.
> 
> This does come down to personal coding style, so don't consider these suggestions blocking.
What's the point though? The only other thing here is the construction of two MDNodes. Feels needlessly complicated to introduce a struct and separate function just for that...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157462