[PATCH] D60900: [ObjC][ARC] Check the basic block size before calling DominatorTree::dominate

[Akira Hatanaka via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL359027: [ObjC][ARC] Check the basic block size before calling (authored by ahatanak, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60900?vs=195951&id=196301#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60900/new/

https://reviews.llvm.org/D60900

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


Index: llvm/trunk/test/Transforms/ObjCARC/contract-max-bb-size.ll
===================================================================
--- llvm/trunk/test/Transforms/ObjCARC/contract-max-bb-size.ll
+++ llvm/trunk/test/Transforms/ObjCARC/contract-max-bb-size.ll
@@ -0,0 +1,17 @@
+; 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/trunk/lib/Transforms/ObjCARC/ObjCARCContract.cpp
===================================================================
--- llvm/trunk/lib/Transforms/ObjCARC/ObjCARCContract.cpp
+++ llvm/trunk/lib/Transforms/ObjCARC/ObjCARCContract.cpp
@@ -45,6 +45,10 @@
 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
 //===----------------------------------------------------------------------===//
@@ -573,6 +577,24 @@
   // 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++;
 
@@ -590,7 +612,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, this](Value *Arg) {
+    auto ReplaceArgUses = [Inst, IsLargeBB, this](Value *Arg) {
       // If we're compiling bugpointed code, don't get in trouble.
       if (!isa<Instruction>(Arg) && !isa<Argument>(Arg))
         return;
@@ -602,6 +624,17 @@
         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: D60900.196301.patch
Type: text/x-patch
Size: 3814 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190423/3b4346d1/attachment.bin>