[llvm] 430512e - [ObjC][ARC] Don't move a retain call living outside a loop into the loop

[Akira Hatanaka via llvm-commits]

Author: Akira Hatanaka
Date: 2020-02-25T13:00:10-08:00
New Revision: 430512ed7d7b5e1052fb3b671d8e54fc3afcf490

URL: https://github.com/llvm/llvm-project/commit/430512ed7d7b5e1052fb3b671d8e54fc3afcf490
DIFF: https://github.com/llvm/llvm-project/commit/430512ed7d7b5e1052fb3b671d8e54fc3afcf490.diff

LOG: [ObjC][ARC] Don't move a retain call living outside a loop into the loop
body

We started seeing cases where ARC optimizer would move retain calls into
loop bodies, causing imbalance in the number of retain and release
calls, after changes were made to delete inert ARC calls since the inert
calls that used to block code motion are gone.

Fix the bug by setting the CFG hazard flag when visiting a loop header.

rdar://problem/56908836

Added: 
    

Modified: 
    llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
    llvm/test/Transforms/ObjCARC/cfg-hazards.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
index adab7491d27a..577a744f3062 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -1569,6 +1569,15 @@ ObjCARCOpt::VisitTopDown(BasicBlock *BB,
     }
   }
 
+  // Check that BB and MyStates have the same number of predecessors. This
+  // prevents retain calls that live outside a loop from being moved into the
+  // loop.
+  if (!BB->hasNPredecessors(MyStates.pred_end() - MyStates.pred_begin()))
+    for (auto I = MyStates.top_down_ptr_begin(),
+              E = MyStates.top_down_ptr_end();
+         I != E; ++I)
+      I->second.SetCFGHazardAfflicted(true);
+
   LLVM_DEBUG(dbgs() << "Before:\n"
                     << BBStates[BB] << "\n"
                     << "Performing Dataflow:\n");

diff  --git a/llvm/test/Transforms/ObjCARC/cfg-hazards.ll b/llvm/test/Transforms/ObjCARC/cfg-hazards.ll
index 9559b3c31146..221e51ba458b 100644
--- a/llvm/test/Transforms/ObjCARC/cfg-hazards.ll
+++ b/llvm/test/Transforms/ObjCARC/cfg-hazards.ll
@@ -430,6 +430,48 @@ exit:
   ret void
 }
 
+; The retain call in the entry block shouldn't be moved to the loop body.
+
+; CHECK: define void @test14(i8* %[[VAL0:.*]],
+; CHECK: %[[V1:.*]] = tail call i8* @llvm.objc.retain(i8* %[[VAL0]])
+; CHECK: %[[CMP:.*]] = icmp eq i8* %[[VAL0]], null
+; CHECK: br i1 %[[CMP]], label %{{.*}}, label %{{.*}}
+
+define void @test14(i8* %val0, i8 %val1) {
+entry:
+  %v1 = tail call i8* @llvm.objc.retain(i8* %val0)
+  %cmp = icmp eq i8* %val0, null
+  br i1 %cmp, label %if.end27, label %if.then
+
+if.then:
+  %tobool = icmp eq i8 %val1, 1
+  br label %for.body
+
+for.cond:
+  %cmp6 = icmp eq i8 %val1, 2
+  br i1 %cmp6, label %for.body, label %for.end.loopexit
+
+for.body:
+  call void @callee()
+  %tobool9 = icmp eq i8 %val1, 0
+  br i1 %tobool9, label %for.cond, label %if.then10
+
+if.then10:
+  br label %for.end
+
+for.end.loopexit:
+  br label %for.end
+
+for.end:
+  call void @callee()
+  call void @use_pointer(i8* %v1)
+  br label %if.end27
+
+if.end27:
+  call void @llvm.objc.release(i8* %v1) #0, !clang.imprecise_release !0
+  ret void
+}
+
 ; CHECK: attributes [[NUW]] = { nounwind }
 
 !0 = !{}