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

[Reid Kleckner via llvm-commits]

Author: Reid Kleckner
Date: 2020-03-13T11:57:55-07:00
New Revision: 478b06e68706610ecb3bb3d3693a21d98f5e339c

URL: https://github.com/llvm/llvm-project/commit/478b06e68706610ecb3bb3d3693a21d98f5e339c
DIFF: https://github.com/llvm/llvm-project/commit/478b06e68706610ecb3bb3d3693a21d98f5e339c.diff

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

This reverts commit 5c3117b0a98dd11717eaffd7fb583985d39544b2

This should not be necessary after
7593a480dbce4e26f7dda4aa8f15bffd03acbfdb, and Florian Hahn has confirmed
that the problem no longer reproduces with this patch.

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

Reviewed By: fhahn, dexonsmith

Differential Revision: https://reviews.llvm.org/D76075

Added: 
    

Modified: 
    llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp

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


################################################################################
diff  --git a/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
index ecf8220ae95d..4a7b5ded3b21 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
@@ -47,10 +47,6 @@ using namespace llvm::objcarc;
 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
 //===----------------------------------------------------------------------===//
@@ -580,23 +576,6 @@ bool ObjCARCContract::runOnFunction(Function &F) {
   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++;
 
@@ -614,7 +593,7 @@ bool ObjCARCContract::runOnFunction(Function &F) {
     // 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;
@@ -626,17 +605,6 @@ bool ObjCARCContract::runOnFunction(Function &F) {
         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
@@ -689,7 +657,6 @@ bool ObjCARCContract::runOnFunction(Function &F) {
       }
     };
 
-
     Value *Arg = cast<CallInst>(Inst)->getArgOperand(0);
     Value *OrigArg = Arg;
 

diff  --git a/llvm/test/Transforms/ObjCARC/contract-max-bb-size.ll b/llvm/test/Transforms/ObjCARC/contract-max-bb-size.ll
deleted file mode 100644
index c591738d0137..000000000000
--- a/llvm/test/Transforms/ObjCARC/contract-max-bb-size.ll
+++ /dev/null
@@ -1,17 +0,0 @@
-; 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
-
- at 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*)