[PATCH] D20379: Codegen: Fix broken assumption in Tail Merge.

Haicheng Wu via llvm-commits llvm-commits at lists.llvm.org
Thu May 19 08:14:06 PDT 2016


haicheng added a comment.

Hi Kyle,

I think FallThrough does not have to be the CFG successor of MBB.  It might be easier to understand it as the potential fallthrough block of either MBB or PrevBB.

Haicheng


================
Comment at: lib/CodeGen/BranchFolding.cpp:1345
@@ -1337,3 +1344,3 @@
     if (MBB->succ_empty() && !PriorCond.empty() && !PriorFBB &&
         MachineFunction::iterator(PriorTBB) == FallThrough &&
         !MBB->canFallThrough()) {
----------------
If you change FallThrough to MF.end(), I think you may miss this optimization.

This piece of code tries to optimize this case

```
PrevBB---MBB  FallThrough
      |____________|
      
```
Where MBB is the return block, PrevBB is both the layout predecessor and CFG predecessor of MBB, Fallthourgh is just a layout successor of MBB and must *not* be the CFG successor of MBB.  In this case, MBB can be moved to the bottom of the MF and PrevBB can fallthrough to FallThrough.

================
Comment at: lib/CodeGen/BranchFolding.cpp:1567
@@ -1559,3 +1566,3 @@
       while (FallThrough != MF.end() && FallThrough->isEHPad())
         ++FallThrough;
       // Now check to see if the current block is sitting between PrevBB and
----------------
Similar situation here.  Fallthrough is used to iterate all blocks below MBB in the layout to find the first non-EHPad.  I think changing it to MF.end()  may miss this one too.


Repository:
  rL LLVM

http://reviews.llvm.org/D20379





More information about the llvm-commits mailing list