Revert commit r207302 since build failures
authorGerolf Hoflehner <ghoflehner@apple.com>
Sat, 26 Apr 2014 02:03:17 +0000 (02:03 +0000)
committerGerolf Hoflehner <ghoflehner@apple.com>
Sat, 26 Apr 2014 02:03:17 +0000 (02:03 +0000)
have been reported.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@207303 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Transforms/Scalar/LoopInstSimplify.cpp
lib/Transforms/Utils/CloneFunction.cpp
lib/Transforms/Utils/SimplifyInstructions.cpp
test/Transforms/InstSimplify/dead-code-removal.ll [deleted file]

index ab1a9393c526c7c05377945ea4364405544a42ea..a61923cabf1e094ac579a84292ec4bf5fb9b4b62 100644 (file)
@@ -127,15 +127,7 @@ bool LoopInstSimplify::runOnLoop(Loop *L, LPPassManager &LPM) {
             ++NumSimplified;
           }
         }
             ++NumSimplified;
           }
         }
-        bool res = RecursivelyDeleteTriviallyDeadInstructions(I, TLI);
-        if (res) {
-          // RecursivelyDeleteTriviallyDeadInstruction can remove
-          // more than one instruction, so simply incrementing the
-          // iterator does not work. When instructions get deleted
-          // re-iterate instead.
-          BI = BB->begin(); BE = BB->end();
-          LocalChanged |= res;
-        }
+        LocalChanged |= RecursivelyDeleteTriviallyDeadInstructions(I, TLI);
 
         if (IsSubloopHeader && !isa<PHINode>(I))
           break;
 
         if (IsSubloopHeader && !isa<PHINode>(I))
           break;
index 5c8f20d5f884161767c0c01cd2b1146c252e1ffe..1a9513751d73b6d64c6b558b02cdb6dca2a72bf0 100644 (file)
@@ -32,6 +32,7 @@
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
 #include <map>
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Utils/ValueMapper.h"
 #include <map>
+#include <set>
 using namespace llvm;
 
 // CloneBasicBlock - See comments in Cloning.h
 using namespace llvm;
 
 // CloneBasicBlock - See comments in Cloning.h
@@ -272,42 +273,28 @@ namespace {
       NameSuffix(nameSuffix), CodeInfo(codeInfo), DL(DL) {
     }
 
       NameSuffix(nameSuffix), CodeInfo(codeInfo), DL(DL) {
     }
 
-    /// CloneBlock - The specified block is found to be reachable, clone it and
-    /// anything that it can reach.
+    /// CloneBlock - The specified block is found to be reachable, so clone it
+    /// into newBB.
     void CloneBlock(const BasicBlock *BB,
     void CloneBlock(const BasicBlock *BB,
-                    std::vector<const BasicBlock*> &ToClone);
+                    BasicBlock *NewBB,
+                    std::vector<const BasicBlock *> &ToClone,
+                    std::set<const BasicBlock *> &OrigBBs);
   };
 }
 
   };
 }
 
-/// CloneBlock - The specified block is found to be reachable, clone it and
-/// anything that it can reach.
+/// CloneBlock - The specified block is found to be reachable, so clone it
+/// into newBB.
 void PruningFunctionCloner::CloneBlock(const BasicBlock *BB,
 void PruningFunctionCloner::CloneBlock(const BasicBlock *BB,
-                                       std::vector<const BasicBlock*> &ToClone){
-  WeakVH &BBEntry = VMap[BB];
-
-  // Have we already cloned this block?
-  if (BBEntry) return;
+                                       BasicBlock *NewBB,
+                                       std::vector<const BasicBlock *> &ToClone,
+                                       std::set<const BasicBlock *> &OrigBBs) {
   
   
-  // Nope, clone it now.
-  BasicBlock *NewBB;
-  BBEntry = NewBB = BasicBlock::Create(BB->getContext());
-  if (BB->hasName()) NewBB->setName(BB->getName()+NameSuffix);
+  // Remove BB from list of blocks to clone.
+  // When it was not in the list, it has been cloned already, so
+  // don't clone again.
+  if (!OrigBBs.erase(BB)) return;
 
 
-  // It is only legal to clone a function if a block address within that
-  // function is never referenced outside of the function.  Given that, we
-  // want to map block addresses from the old function to block addresses in
-  // the clone. (This is different from the generic ValueMapper
-  // implementation, which generates an invalid blockaddress when
-  // cloning a function.)
-  //
-  // Note that we don't need to fix the mapping for unreachable blocks;
-  // the default mapping there is safe.
-  if (BB->hasAddressTaken()) {
-    Constant *OldBBAddr = BlockAddress::get(const_cast<Function*>(OldFunc),
-                                            const_cast<BasicBlock*>(BB));
-    VMap[OldBBAddr] = BlockAddress::get(NewFunc, NewBB);
-  }
-    
+  // Nope, clone it now.
 
   bool hasCalls = false, hasDynamicAllocas = false, hasStaticAllocas = false;
   
 
   bool hasCalls = false, hasDynamicAllocas = false, hasStaticAllocas = false;
   
@@ -425,7 +412,7 @@ void llvm::CloneAndPruneFunctionInto(Function *NewFunc, const Function *OldFunc,
                                      const DataLayout *DL,
                                      Instruction *TheCall) {
   assert(NameSuffix && "NameSuffix cannot be null!");
                                      const DataLayout *DL,
                                      Instruction *TheCall) {
   assert(NameSuffix && "NameSuffix cannot be null!");
-  
+
 #ifndef NDEBUG
   for (Function::const_arg_iterator II = OldFunc->arg_begin(), 
        E = OldFunc->arg_end(); II != E; ++II)
 #ifndef NDEBUG
   for (Function::const_arg_iterator II = OldFunc->arg_begin(), 
        E = OldFunc->arg_end(); II != E; ++II)
@@ -435,15 +422,87 @@ void llvm::CloneAndPruneFunctionInto(Function *NewFunc, const Function *OldFunc,
   PruningFunctionCloner PFC(NewFunc, OldFunc, VMap, ModuleLevelChanges,
                             NameSuffix, CodeInfo, DL);
 
   PruningFunctionCloner PFC(NewFunc, OldFunc, VMap, ModuleLevelChanges,
                             NameSuffix, CodeInfo, DL);
 
-  // Clone the entry block, and anything recursively reachable from it.
+  // Since all BB address references need to be known before block-by-block
+  // processing, we need to create all reachable blocks before processing
+  // them for instruction cloning and pruning. Some of these blocks may
+  // be removed due to later pruning.
   std::vector<const BasicBlock*> CloneWorklist;
   std::vector<const BasicBlock*> CloneWorklist;
+  //
+  // OrigBBs consists of all blocks reachable from the entry
+  // block.
+  // This list will be pruned down by the CloneFunction() currently
+  // (March 2014) due to two optimizations:
+  // First, when a conditional branch target is known at compile-time,
+  // only the actual branch destination block needs to be cloned.
+  // Second, when a switch statement target is known at compile-time,
+  // only the actual case statement needs to be cloned.
+  std::set<const BasicBlock*> OrigBBs;
+
   CloneWorklist.push_back(&OldFunc->getEntryBlock());
   while (!CloneWorklist.empty()) {
     const BasicBlock *BB = CloneWorklist.back();
     CloneWorklist.pop_back();
   CloneWorklist.push_back(&OldFunc->getEntryBlock());
   while (!CloneWorklist.empty()) {
     const BasicBlock *BB = CloneWorklist.back();
     CloneWorklist.pop_back();
-    PFC.CloneBlock(BB, CloneWorklist);
+
+    // Don't revisit blocks.
+    if (VMap.count(BB))
+      continue;
+
+    BasicBlock *NewBB = BasicBlock::Create(BB->getContext());
+    if (BB->hasName()) NewBB->setName(BB->getName()+NameSuffix);
+
+    // It is only legal to clone a function if a block address within that
+    // function is never referenced outside of the function.  Given that, we
+    // want to map block addresses from the old function to block addresses in
+    // the clone. (This is different from the generic ValueMapper
+    // implementation, which generates an invalid blockaddress when
+    // cloning a function.)
+    //
+    // Note that we don't need to fix the mapping for unreachable blocks;
+    // the default mapping there is safe.
+    if (BB->hasAddressTaken()) {
+      Constant *OldBBAddr = BlockAddress::get(const_cast<Function*>(OldFunc),
+                                              const_cast<BasicBlock*>(BB));
+      VMap[OldBBAddr] = BlockAddress::get(NewFunc, NewBB);
+    }
+
+    OrigBBs.insert(BB);
+    VMap[BB] = NewBB;
+    // Iterate over all possible successors and add them to the CloneWorklist.
+    const TerminatorInst *Term = BB->getTerminator();
+    for (unsigned i = 0, e = Term->getNumSuccessors(); i != e; ++i) {
+      BasicBlock *Succ = Term->getSuccessor(i);
+      CloneWorklist.push_back(Succ);
+    }
   }
   }
-  
+
+  // Now, fill only the reachable blocks with the cloned contents
+  // of the originals.
+  assert(CloneWorklist.empty() && "Dirty worklist before re-use\n");
+  CloneWorklist.push_back(&OldFunc->getEntryBlock());
+  while (!CloneWorklist.empty()) {
+    const BasicBlock *BB = CloneWorklist.back();
+    CloneWorklist.pop_back();
+    PFC.CloneBlock(BB, cast<BasicBlock>(VMap[BB]), CloneWorklist,
+                   OrigBBs);
+  }
+
+  // Removed BB's that were created that turned out to be prunable.
+  // Actual cloning may have found pruning opportunities since
+  // branch or switch statement target may have been known at compile-time.
+  // Alternatively we could write a routine CloneFunction and add a) a
+  // parameter to actually do the cloning and b) a return parameter that
+  // gives a list of blocks that need to be cloned also. Then we could
+  // call CloneFunction when we collect the blocks to call, but suppress
+  // cloning. And actually *do* the cloning in the while loop above. Also
+  // the cleanup here would become redundant, and so would be the OrigBBs.
+  for (std::set<const BasicBlock *>::iterator Oi = OrigBBs.begin(),
+       Oe = OrigBBs.end(); Oi != Oe; ++Oi) {
+    const BasicBlock *Orig = *Oi;
+    BasicBlock *NewBB = cast<BasicBlock>(VMap[Orig]);
+    delete NewBB;
+    VMap[Orig] = 0;
+  }
+
   // Loop over all of the basic blocks in the old function.  If the block was
   // reachable, we have cloned it and the old block is now in the value map:
   // insert it into the new function in the right order.  If not, ignore it.
   // Loop over all of the basic blocks in the old function.  If the block was
   // reachable, we have cloned it and the old block is now in the value map:
   // insert it into the new function in the right order.  If not, ignore it.
index 33b36378027d01715456f9b9382e62225e142eac..c62aa663f6d0efaff465cc666f9ceae6bc45c594 100644 (file)
@@ -76,15 +76,7 @@ namespace {
                 ++NumSimplified;
                 Changed = true;
               }
                 ++NumSimplified;
                 Changed = true;
               }
-            bool res = RecursivelyDeleteTriviallyDeadInstructions(I, TLI);
-            if (res)  {
-              // RecursivelyDeleteTriviallyDeadInstruction can remove
-              // more than one instruction, so simply incrementing the
-              // iterator does not work. When instructions get deleted
-              // re-iterate instead.
-              BI = BB->begin(); BE = BB->end();
-              Changed |= res;
-            }
+            Changed |= RecursivelyDeleteTriviallyDeadInstructions(I, TLI);
           }
 
         // Place the list of instructions to simplify on the next loop iteration
           }
 
         // Place the list of instructions to simplify on the next loop iteration
diff --git a/test/Transforms/InstSimplify/dead-code-removal.ll b/test/Transforms/InstSimplify/dead-code-removal.ll
deleted file mode 100644 (file)
index 89c9d9c..0000000
+++ /dev/null
@@ -1,15 +0,0 @@
-; RUN: opts -instsimplify -S < %s | FileCheck %s
-
-define void @foo() nounwind {
-  br i1 undef, label %1, label %4
-
-; <label>:1                                       ; preds = %1, %0
-; CHECK-NOT: phi
-; CHECK-NOT: sub
-  %2 = phi i32 [ %3, %1 ], [ undef, %0 ]
-  %3 = sub i32 0, undef
-  br label %1
-
-; <label>:4                                       ; preds = %0
-  ret void
-}