[PATCH] D31310: Fix trellis layout when there is triangle.

Kyle Butt via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 23 15:52:13 PDT 2017


iteratee added a comment.

Thanks for catching this. The test case can stay, but I think the fix needs to go in a different spot. Good work.



================
Comment at: lib/CodeGen/MachineBlockPlacement.cpp:842
       // Allow triangle successors, but don't count them.
       if (Successors.count(SuccPred))
         continue;
----------------
I appreciate the test case, but I think the logic needs to go here.

```
      if (Successors.count(SuccPred)) {
        // Make sure that it is actually a triangle.
        for (MachineBasicBlock *CheckSucc : SuccPred.successors())
          if (!Successors.contains(CheckSucc))
            return false;
        continue;
      }
```


================
Comment at: test/CodeGen/PowerPC/tail-dup-layout.ll:477
 
+; Verify that in the triangular trellis case, if there is hot successor
+; outside of trellis, we should be able to connect them by not connecting
----------------
Change the comment to reflect the fact that we shouldn't mis-identify triangle trellises if it's not really a triangle.


https://reviews.llvm.org/D31310





More information about the llvm-commits mailing list