Hi Justin,<div><br></div><div>Is it possible to separate out the "instrument to get counters" from "annotate based on information" parts of the PGO class?</div><div><br></div><div>It would be great to see a top level description of the algorithm you're using to do region creation etc along with an example. It seems that you're creating regions from a way different than most previous literature on the topic so a literature link would be good if you've got one, else a description is going to be key.</div>
<div><br></div><div>Few comments on the code:</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
+  PGO.setCurrentRegionCount(0);<br>
</blockquote><div><br></div><div>Should comment this as it's not obvious from reading the surrounding code.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>

+  Cnt.beginRegion(Builder);<br>
   eval.begin(*this);<br>
   LValue lhs = EmitLValue(expr->getTrueExpr()<u></u>);<br>
   eval.end(*this);<br>
+  Cnt.adjustFallThroughCount();<br>
<br></blockquote><div><br></div><div>The adjust stuff is wonky enough that you should comment all uses of it. It isn't obvious why/when/where you'd want to do it. Same with when you apply adjustments.</div><div><br>
</div><div>Basically if it's going to be a part of any abnormal edge then perhaps it makes more sense to handle it as part of abnormal edge calculations? What about the code needs this sort of handling? Ditto with the parent count subtractions you see at loops.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    PGO.setCurrentRegionCount(0);<br>
</blockquote><div><br></div><div>Go ahead document why you're resetting the region count here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
   // As long as the condition is true, iterate the loop.<br>
-  if (EmitBoolCondBranch)<br>
-    Builder.CreateCondBr(<u></u>BoolCondVal, LoopBody, LoopExit.getBlock());<br>
+  if (EmitBoolCondBranch) {<br>
+    Builder.CreateCondBr(<u></u>BoolCondVal, LoopBody, LoopExit.getBlock(),<br>
+                         PGO.createBranchWeights(<u></u>LoopCount, ExitCount));<br>
+  }<br>
<br></blockquote><div><br></div><div>Don't think the braces are necessary here.</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    uint64_t Total = CaseCnt.getCount() - CaseCnt.getParentCount();<br>
+    unsigned NCases = Range.getZExtValue() + 1;<br>
+    // Divide the weights evenly between the cases, ensuring that the total<br>
+    // weight is preserved. Ie, a weight of 5 over three cases will be<br>
+    // distributed as weights of 2, 2, and 1.<br>
+    uint64_t Weight = Total / NCases, Rem = Total % NCases;<br>
+    for (unsigned I = 0; I != NCases; ++I) {<br>
+      if (SwitchWeights)<br>
+        SwitchWeights->push_back(<u></u>Weight + (Rem ? 1 : 0));<br>
+      if (Rem)<br>
+        Rem--;<br></blockquote><div><br></div><div>Is this related to propagating some difference between parent counts and "normal" counts?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

-  // If the body of the case is just a 'break', and if there was no fallthrough,<br>
-  // try to not emit an empty block.<br>
-  if ((CGM.getCodeGenOpts().<u></u>OptimizationLevel > 0) &&<br>
+  // If the body of the case is just a 'break', try to not emit an empty block.<br>
+  // If we're profiling or we're not optimizing, leave the block in for better<br>
+  // debug and coverage analysis.<br>
+  if (!CGM.getCodeGenOpts().<u></u>ProfileInstrGenerate &&<br>
+      CGM.getCodeGenOpts().<u></u>OptimizationLevel > 0 &&<br>
       isa<BreakStmt>(S.getSubStmt())<u></u>) {<br>
     JumpDest Block = BreakContinueStack.back().<u></u>BreakBlock;<br>
<br></blockquote><div><br></div><div>Is it really worth that much for code gen/optimization time to not just always have it? Also, do we really want instrumentation to change the cfg we emit? Also, it doesn't change in debug mode, just without optimization so that part of the comment isn't quite accurate :)</div>
<div><br></div></div><div>There's probably a bit more, but this seems like a good place to start given the questions at the top.</div><div><br></div><div>-eric</div>