[PATCH] D10825: Improvement on computing edge probabilities when choosing the best successor in machine block placement.

[David]

davidxl added inline comments.

================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:381-383
@@ +380,5 @@
+        SkipSucc = true;
+      } else if (Succ != *SuccChain->begin()) {
+        DEBUG(dbgs() << "    " << getBlockName(Succ) << " -> Mid chain!\n");
+        SkipSucc = true;
+      }
----------------
chandlerc wrote:
> This is a bigger change than the rest, and I think it should go in separately.
> 
> The rest of this change is a clear bug-fix for how this code works, and will make the "hot" detection actually reasonable below. This change, however, is shifting a core thing to prefer to outline all optional loop bodies which don't have fallthrough into them. That, to me, is a very significant change, and I'd rather look at it (and test cases and benchmarks showing it is clearly the right call) independently of fixing how we compute the probability of 1 of N successors.
The core idea of Cong's patch is to make trace formation more correctly. The basic principle of the original trace formation (probability based) is that when a BB has multiple viable successors, but none of them is more dominating, then do not use outgoing succ edge frequency to determine layout successor

The bug in the current implementation is that when checking dominating outgoing edges, it also look at non-viable successors that have already being laid out.

The fix here is simply to eliminate the non-candidate when selecting trace successor.

Cong, as I mentioned before, it is much clearer to extract the adjustment code into a helper routine with documentation to make the intention clear.


http://reviews.llvm.org/D10825