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

Haicheng Wu via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 2 14:44:47 PDT 2016


haicheng added a comment.

Hi Kyle,

Do you have test cases for your change in line 969 and 1270?

Haicheng


================
Comment at: lib/CodeGen/BranchFolding.cpp:969
@@ -966,1 +968,3 @@
     MachineBasicBlock *PredBB = &*std::prev(I);
+    if (PredBB && !PredBB->isSuccessor(IBB))
+      PredBB = nullptr;
----------------
My understanding is that PredBB is just the layout predecessor of MBB.  The CFG predecessors of MBB are stored in MergePotentials and they are compared with PredBB to see if any of them is also the layout predecessor.  I know the debug info is misleading....

Do you have a test case that the current code miscompiles?

================
Comment at: lib/CodeGen/BranchFolding.cpp:1270
@@ -1265,3 +1269,3 @@
     if (PriorCond.empty() && !PriorTBB && MBB->pred_size() == 1 &&
-        PrevBB.succ_size() == 1 &&
+        PrevBB.succ_size() == 1 && PrevBB.isSuccessor(MBB) &&
         !MBB->hasAddressTaken() && !MBB->isEHPad()) {
----------------
Is this check you added covered by the rest existing conditions?  Do you have a test case for this?


http://reviews.llvm.org/D20379





More information about the llvm-commits mailing list