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

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:250
+static void updateBranchWeights(BranchInst &PreHeaderBI, BranchInst &LoopBI,
+                                bool ConditionalPreHeader, bool SuccsSwapped) {
+  MDNode *WeightMD = getBranchWeightMDNode(PreHeaderBI);
----------------
nit: `HasConditionalPreHeader`. 


================
Comment at: llvm/lib/Transforms/Utils/LoopRotationUtils.cpp:294
+
+  uint32_t ExitWeight0; // aka x0
+  if (!ConditionalPreHeader) {
----------------
MatzeB wrote:
> 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.
Is it safe to rely on warnings though - is there at least a bot that treats uninitialized values as errors? But that aside, looks like 0 is the value when `ConditionalPreHeader` anyway, meaning 0 wouldn't be some safe default, rather, it's exactly what you'd want (assuming the if condition were swapped)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157462