[llvm-commits] [llvm] r144532 - /llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp

[Chandler Carruth]

Author: chandlerc
Date: Mon Nov 14 04:55:53 2011
New Revision: 144532

URL: http://llvm.org/viewvc/llvm-project?rev=144532&view=rev
Log:
Remove an over-eager assert that was firing on one of the ARM regression
tests when I forcibly enabled block placement.

It is apparantly possible for an unanalyzable block to fallthrough to
a non-loop block. I don't actually beleive this is correct, I believe
that 'canFallThrough' is returning true needlessly for the code
construct, and I've left a bit of a FIXME on the verification code to
try to track down why this is coming up.

Anyways, removing the assert doesn't degrade the correctness of the algorithm.

Modified:
    llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp

Modified: llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp?rev=144532&r1=144531&r2=144532&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineBlockPlacement.cpp Mon Nov 14 04:55:53 2011
@@ -492,9 +492,9 @@
     if (TII->AnalyzeBranch(*BB, TBB, FBB, Cond) && BB->canFallThrough()) {
       MachineFunction::iterator I(BB), NextI(llvm::next(I));
       // Ensure that the layout successor is a viable block, as we know that
-      // fallthrough is a possibility.
+      // fallthrough is a possibility. Note that this may not be a valid block
+      // in the loop, but we allow that to cope with degenerate situations.
       assert(NextI != BB->getParent()->end());
-      assert(!BlockFilter || BlockFilter->count(NextI));
       BestSucc = NextI;
     }
 
@@ -594,7 +594,10 @@
     for (BlockChain::iterator BCI = LoopChain.begin(), BCE = LoopChain.end();
          BCI != BCE; ++BCI)
       if (!LoopBlockSet.erase(*BCI)) {
-        BadLoop = true;
+        // We don't mark the loop as bad here because there are real situations
+        // where this can occur. For example, with an unanalyzable fallthrough
+        // from a loop block to a non-loop block.
+        // FIXME: Such constructs shouldn't exist. Track them down and fix them.
         dbgs() << "Loop chain contains a block not contained by the loop!\n"
                << "  Loop header:  " << getBlockName(*L.block_begin()) << "\n"
                << "  Chain header: " << getBlockName(*LoopChain.begin()) << "\n"