Revert "[ObjC][ARC] Check the basic block size before calling DominatorTree::dominate"

[rnk]

Summary

This reverts commit 5c3117b0a98dd11717eaffd7fb583985d39544b2

This should not be necessary after
7593a480dbce4e26f7dda4aa8f15bffd03acbfdb, although I do not have a test
case to demonstrate that the problem has gone away.

I happened to notice this code because the FIXME talks about
OrderedBasicBlock.

[rnk]

• pacify clang-format

[dexonsmith]

This SGTM. Ideally we'd confirm the compile-time explosion is fixed before landing this though.

+fhahn, who did some of the initial investigation.

[fhahn]

LGTM thanks. I verified that we spend roughly the same amount of time in the middle-end with and without this patch for the motivating case for this patch (around 30 secs instead of hours before the original patch)>

llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp

 using namespace llvm;
 using namespace llvm::objcarc;
 
 #define DEBUG_TYPE "objc-arc-contract"
 
 STATISTIC(NumPeeps,       "Number of calls peephole-optimized");
 STATISTIC(NumStoreStrongs, "Number objc_storeStrong calls formed");
 
-static cl::opt<unsigned> MaxBBSize("arc-contract-max-bb-size", cl::Hidden,
-    cl::desc("Maximum basic block size to discover the dominance relation of "
-             "two instructions in the same basic block"), cl::init(65535));
-
 //===----------------------------------------------------------------------===//
 //                                Declarations
 //===----------------------------------------------------------------------===//
 
 namespace {
   /// Late ARC optimizations
   ///
   /// These change the IR in a way that makes it difficult to be analyzed by
bool TailOkForStoreStrongs =
       !F.isVarArg() && !F.callsFunctionThatReturnsTwice();
 
   // For ObjC library calls which return their argument, replace uses of the
   // argument with uses of the call return value, if it dominates the use. This
   // reduces register pressure.
   SmallPtrSet<Instruction *, 4> DependingInstructions;
   SmallPtrSet<const BasicBlock *, 4> Visited;
 
-  // Cache the basic block size.
-  DenseMap<const BasicBlock *, unsigned> BBSizeMap;
-
-  // A lambda that lazily computes the size of a basic block and determines
-  // whether the size exceeds MaxBBSize.
-  auto IsLargeBB = [&](const BasicBlock *BB) {
-    unsigned BBSize;
-    auto I = BBSizeMap.find(BB);
-
-    if (I != BBSizeMap.end())
-      BBSize = I->second;
-    else
-      BBSize = BBSizeMap[BB] = BB->size();
-
-    return BBSize > MaxBBSize;
-  };
-
   for (inst_iterator I = inst_begin(&F), E = inst_end(&F); I != E;) {
     Instruction *Inst = &*I++;
 
     LLVM_DEBUG(dbgs() << "Visiting: " << *Inst << "\n");
 
     // First try to peephole Inst. If there is nothing further we can do in
     // terms of undoing objc-arc-expand, process the next inst.
     if (tryToPeepholeInstruction(F, Inst, I, DependingInstructions, Visited,
                                  TailOkForStoreStrongs, BlockColors))
       continue;
 
     // Otherwise, try to undo objc-arc-expand.
 
     // Don't use GetArgRCIdentityRoot because we don't want to look through bitcasts
     // and such; to do the replacement, the argument must have type i8*.
 
     // Function for replacing uses of Arg dominated by Inst.
-    auto ReplaceArgUses = [Inst, IsLargeBB, this](Value *Arg) {
+    auto ReplaceArgUses = [Inst, this](Value *Arg) {
       // If we're compiling bugpointed code, don't get in trouble.
       if (!isa<Instruction>(Arg) && !isa<Argument>(Arg))
         return;
 
       // Look through the uses of the pointer.
       for (Value::use_iterator UI = Arg->use_begin(), UE = Arg->use_end();
            UI != UE; ) {
         // Increment UI now, because we may unlink its element.
         Use &U = *UI++;
         unsigned OperandNo = U.getOperandNo();
 
-        // Don't replace the uses if Inst and the user belong to the same basic
-        // block and the size of the basic block is large. We don't want to call
-        // DominatorTree::dominate in that case. We can remove this check if we
-        // can use OrderedBasicBlock to compute the dominance relation between
-        // two instructions, but that's not currently possible since it doesn't
-        // recompute the instruction ordering when new instructions are inserted
-        // to the basic block.
-        if (Inst->getParent() == cast<Instruction>(U.getUser())->getParent() &&
-            IsLargeBB(Inst->getParent()))
-          continue;
-
         // If the call's return value dominates a use of the call's argument
         // value, rewrite the use to use the return value. We check for
         // reachability here because an unreachable call is considered to
         // trivially dominate itself, which would lead us to rewriting its
         // argument in terms of its return value, which would lead to
         // infinite loops in GetArgRCIdentityRoot.
         if (!DT->isReachableFromEntry(U) || !DT->dominates(Inst, U))
           continue;
auto ReplaceArgUses = [Inst, this](Value *Arg) {
           if (Replacement->getType() != UseTy)
             Replacement = new BitCastInst(Replacement, UseTy, "",
                                           cast<Instruction>(U.getUser()));
           U.set(Replacement);
         }
       }
     };
 
-
     Value *Arg = cast<CallInst>(Inst)->getArgOperand(0);
     Value *OrigArg = Arg;
 
     // TODO: Change this to a do-while.
     for (;;) {
       ReplaceArgUses(Arg);
 
       // If Arg is a no-op casted pointer, strip one level of casts and iterate.

llvm/test/Transforms/ObjCARC/contract-max-bb-size.ll

-; RUN: opt -objc-arc-contract -S < %s | FileCheck -check-prefix=ENABLE %s
-; RUN: opt -objc-arc-contract -arc-contract-max-bb-size=3 -S < %s | FileCheck -check-prefix=DISABLE %s
-
-@g0 = common global i8* null, align 8
-
-; ENABLE: store i8* %2, i8** @g0
-; DISABLE: store i8* %1, i8** @g0
-
-define void @foo0() {
-  %1 = tail call i8* @foo1()
-  %2 = tail call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %1)
-  store i8* %1, i8** @g0, align 8
-  ret void
-}
-
-declare i8* @foo1()
-declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*)