[PATCH] D28593: Update loop branch_weight metadata after loop rotation.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopRotation.cpp:561
+  // 1. Loop with really large ATC (20+). This loop probably always executes
+  // a few iterations every time its hit. We give it maximum chance to exit
+  // without a single iteration.
----------------
maximum--> minimum


================
Comment at: lib/Transforms/Scalar/LoopRotation.cpp:578
+  // it is hit.
+  if (ATC > 20)
+    NotTakenOncePercent = 1;
----------------
Avoid using floating point operation here. Just do:

if (LoopBodyWeight  > 20 * LoopExitWeight) ..

Branch Weight Data is actually 32bit, so there is no risk of overflow here.


================
Comment at: lib/Transforms/Scalar/LoopRotation.cpp:584
+    NotTakenOncePercent = 50;
+  else if (ATC > 0.05)
+    NotTakenOncePercent = 95;
----------------
When ACT < 0.2, it seems simpler just do

NotTakenOnceProb = (ExitWeight - BodyWeight)/ExitWeight.


================
Comment at: lib/Transforms/Scalar/LoopRotation.cpp:599
+    return NotTakenOnce;
+  return std::min(LoopExitWeight - LoopBodyWeight, NotTakenOnce);
+}
----------------
This adjustment will create weird results:

For ACT between 1 and 5,  the computed NonTakenOnce Prob is kept which 50%. However, when ACT is between 0.5 and 1, the resulting not taken once Prob is smaller than 50% which contradicts to the trend -- the smaller ACT is, the more likely it is not executed once.

To fix this, do the following:

When ACT is between 0.5 and 5, use 50%
When ACT is less than 0.5, use (ExitWeight - BodyWeight)/ExitWeight

This makes the weight 'continuous and the special handling using 'min' can be removed.


https://reviews.llvm.org/D28593