[PATCH] D30309: CodeGen: BlockPlacement: Precompute layout for chains of triangles.

[Kyle Butt via Phabricator via llvm-commits]

iteratee added inline comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:152
+             "triangle tail duplication heuristic to kick in. 0 to disable."),
+    cl::init(2),
+    cl::Hidden);
----------------
davidxl wrote:
> Should the default be 3?
If I had found anywhere where it was likely to cause problems in my benchmarking, I would bump it to 3, but I haven't.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:1061
+///    longer chains)
+/// 2) The chains are statically correlated. Branch probabilities have a very
+///    U-shaped distribution. If the branches in a chain are all from the same
----------------
davidxl wrote:
> 2) This is not entirely true --  it depends on the length the chain (e.g, when branch proabiity to Pdom blocks are low).  Perhaps remove 2).
No, It's accurate. If you know that the frequencies are correlated, but not whether they're high or low, than this is profitable. Even if the individual branch decisions are independent. 

If by the length of the chain, you mean that the only correlations that matter are neighbors, then that is correct and I can add a note about that.


https://reviews.llvm.org/D30309