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

[Kyle Butt via llvm-commits]

iteratee added inline comments.

================
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()) {
----------------
haicheng wrote:
> Is this check you added covered by the rest existing conditions?  Do you have a test case for this?
You're right it's covered. Looking at AnalyzeBranch, Cond.empty && !TBB implies fallthrough.

I'll remove it.



================
Comment at: lib/CodeGen/BranchFolding.cpp:969
@@ -966,1 +968,3 @@
     MachineBasicBlock *PredBB = &*std::prev(I);
+    if (PredBB && !PredBB->isSuccessor(IBB))
+      PredBB = nullptr;
----------------
No, there is no test case that changes, and nothing in the test suite changes hashes.

However, the comments documenting the function disagree with you. See the comments on lines 796-797. The fact that nothing bad has happened so far isn't a good argument for not fixing the code to match the assumptions given in the comments.


http://reviews.llvm.org/D20379