[PATCH] D23939: IfConversion: Fix branch predication bug.

[David Li via llvm-commits]

davidxl added inline comments.

================
Comment at: lib/CodeGen/IfConversion.cpp:1815-1820
@@ -1782,4 +1814,8 @@
 
-  if (RemoveTrueBranch)
-    BBI1->NonPredSize -= TII->RemoveBranch(*BBI1->BB);
+  // The branches have been checked to match, so it is safe to remove the branch
+  // in BB1 and rely on the copy in BB2
+#ifndef NDEBUG
+  verifySameBranchInstructions(&MBB1, &MBB2);
+#endif
+  BBI1->NonPredSize -= TII->RemoveBranch(*BBI1->BB);
   // Remove duplicated instructions.
----------------
Is it an expectation later that the terminator instruction (in two BBs) are either both removed or they both exist -- i.e. what verifySameInstructions does?

You patch looks right, but can you check why the problem does not appear before ? Is it because NumDups2 computation is different?

================
Comment at: lib/CodeGen/IfConversion.cpp:1843
@@ +1842,3 @@
+    BBI2->NonPredSize -= TII->RemoveBranch(*BBI2->BB);
+  else {
+    do {
----------------
Why not always skip it?


Repository:
  rL LLVM

https://reviews.llvm.org/D23939