[PATCH] D28522: Codegen: Make chains from lattice-shaped CFGs

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:920
+  auto SecondBestB = std::next(BestB);
+  assert (BestA != BestEdges[0].end() && BestB != BestEdges[1].end()
+          && SecondBestA != BestEdges[0].end()
----------------
The assert seems redundant - lattice shape check already checks number of predecessors.


================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:924
+          && "Should have found predecessors for both successors.");
+  // Arrange for the correct answer to be in BestA and BestB
+  // If the 2 best edges don't conflict, the answer is already there.
----------------
Since the non-lattice based layout algorithm looks at cfg edges in forward direction (i.e. look at successor edges), it looks wrong to use best incoming edges to detect conflict.  The conflict exists when the best outgoing edges from two predecessor share the same successor.  Example:  (skip to the end of this example to see general algorithm).


```
  BB    Pred
  |  \      /|
  |    \  /  |
  |      /\  |
  |   /    S2  
  | /      
S1  
```

1. If best outgoing edges of BB is BB->S1, while the best outgoing edges of Pred is Pred->S2, then there is no conflict.

  1.1 If there is no triangle (from S2->S1), then the best successor for BB here is S1.
  1.2 If there is an edge from S2 to S1 (forming triangle), if Freq(S2->S1) > Freq(BB->S1), then we prefer layout Pred->S2->S1, so BB's best successor should be null.

2. If the best outgoing edge of BB and Pred is the same say S1, then there is a conflict.
   2.1. no triangle: if Freq(BB->S1) > Freq(Pred->S1), return S1 as the best successor for BB, otherwise S2
   2.2. there is an edge from S2 to S1: 
   ....

Actually if you look at all the special cases, there is a more general algorithm to get the optimal solution for all shapes:  among all 4 edges (5 edges for the triangular case), find two non conflicting edges as the fallthrough edges such that their frequency sum is maximal.  (two edges are conflicting if they share either source or dest BB).



https://reviews.llvm.org/D28522