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

[Matthias Braun via Phabricator via llvm-commits]

MatzeB marked 2 inline comments as done.
MatzeB added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:294
+
+  uint32_t ExitWeight0; // aka x0
+  if (!ConditionalPreHeader) {
----------------
mtrofin wrote:
> nit: can you init ExitWeight0 at declaration (to avoid potential joys due to uninitialization later). In fact, setting it to 0 and flipping the conditional below and not having an else would do it.
This is deliberately left uninitialized! The idea is that every branch in the following ifs will set a value for it. Leaving the variable uninitialized here, means we will get a compiler warning if we forget to set the value on any branch below which I think is more valuable than setting some kind of safe default here but not getting a warning anymore.


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:703
+    const Value *Cond = PHBI->getCondition();
+    bool ConditionalPreHeader =
+        !isa<ConstantInt>(Cond) ||
----------------
mtrofin wrote:
> const bool?
Sure, will add. (though FWIW `const` could be applied to the majority of the variables and parameters in all of LLVM. I just have a feeling the existing code usually limits `const` usage to pointer and reference targets...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157462