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

[Haicheng Wu via llvm-commits]

haicheng added a comment.

Sorry for the late response.

Have you updated the broken test cases?  I ran your change on my machine and I could pass test-sharedidx.ll always-ext.ll thumb2-ifcvt3.ll


================
Comment at: lib/CodeGen/BranchFolding.cpp:969
@@ -966,1 +968,3 @@
     MachineBasicBlock *PredBB = &*std::prev(I);
+    if (PredBB && !PredBB->isSuccessor(IBB))
+      PredBB = nullptr;
----------------
iteratee wrote:
> iteratee wrote:
> > 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.
> I need to get this in, so if you are set against this check, I can pull it out of this patch and worry about it later.
Comment 796-797 emphasizes *if any*.  So, maybe we can worry about it later.

================
Comment at: test/CodeGen/X86/tail-merge-unreachable.ll:15-18
@@ +14,6 @@
+  ]
+sw.bb:
+  unreachable
+sw.bb2:
+  unreachable
+end:
----------------
Not directly related to your change.  Do you think tail merging should merge sw.bb and sw.bb2?  I know tail merging currently cannot merge empty blocks or unconditional branch only blocks.  I may work on it as next.


http://reviews.llvm.org/D20379