[PATCH] D38848: {ARM} IfConversion does not handle un-analyzable branch correctly

[Gael Jobin via Phabricator via llvm-commits]

gael.jobin updated this revision to Diff 119470.
gael.jobin added a comment.

Instead of trying to manually handle the case of branches not removed by the removeBranch(), I have chosen another approach.

The CountDuplicatedInstructions() set Dups1 and Dups2 respectively. The first one is the number of common instructions from the beginning between two blocks (TrueBB and FalseBB) and the second is the number of common instructions from the end. Dups2 counts any instruction except debug info instruction or branches (isBranch()).

The real problem si when Dups2 is used in DiamondCommon(). The loop that remove common instructions is missing a condition to ignore branch instructions (it was maybe assumed that no branch instructions can exist because of the removeBranch() call). As we have seen, removeBranch() does not remove all kind of branches. Then, we need to add a condition into the loop.

This patch simply apply the same logic inside CountDuplicatedInstructions() into DiamondCommon() when dealing with Dups2 variable, which makes sense.


https://reviews.llvm.org/D38848

Files:
  lib/CodeGen/IfConversion.cpp


Index: lib/CodeGen/IfConversion.cpp
===================================================================
--- lib/CodeGen/IfConversion.cpp
+++ lib/CodeGen/IfConversion.cpp
@@ -1797,7 +1797,7 @@
     assert(DI1 != MBB1.begin());
     --DI1;
     // skip dbg_value instructions
-    if (!DI1->isDebugValue())
+    if (!DI1->isDebugValue() && !DI1->isBranch())
       ++i;
   }
   MBB1.erase(DI1, MBB1.end());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D38848.119470.patch
Type: text/x-patch
Size: 405 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171018/a9c81750/attachment.bin>