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

[Reid Kleckner via Phabricator via llvm-commits]

rnk created this revision.
rnk added reviewers: ahatanak, pete, vsk.
Herald added subscribers: dexonsmith, hiraditya.
Herald added a project: LLVM.

This reverts commit 5c3117b0a98dd11717eaffd7fb583985d39544b2

This should not be necessary after
7593a480dbce4e26f7dda4aa8f15bffd03acbfdb <https://reviews.llvm.org/rG7593a480dbce4e26f7dda4aa8f15bffd03acbfdb>, 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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76075

Files:
  llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
  llvm/test/Transforms/ObjCARC/contract-max-bb-size.ll


Index: llvm/test/Transforms/ObjCARC/contract-max-bb-size.ll
===================================================================
--- 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*)
Index: llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
===================================================================
--- llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
+++ llvm/lib/Transforms/ObjCARC/ObjCARCContract.cpp
@@ -47,10 +47,6 @@
 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 @@
   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 @@
     // 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 @@
         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


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D76075.249960.patch
Type: text/x-patch
Size: 3702 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200312/7e84e5e4/attachment.bin>