From b7e85bfcf731c3c132ae6ec8230d2f74783db5bf Mon Sep 17 00:00:00 2001
From: Akira Hatanaka <ahatanaka@apple.com>
Date: Fri, 28 Aug 2020 17:44:56 -0700
Subject: [PATCH] [ObjC][ARC] In HandlePotentialAlterRefCount, check whether an
 instruction can decrement the reference count, not whether it can alter it

This prevents the state transition from S_Use to S_CanRelease when doing
a bottom-up traversal and the transition from S_Retain to S_CanRelease
when doing a top-down traversal when the visited instruction can
increment the ref count but cannot decrement it. This allows the ARC
optimizer to remove retain/release pairs which were previously not
removed.

rdar://problem/21793154
---
 lib/Transforms/ObjCARC/PtrState.cpp        |  4 ++--
 test/Transforms/ObjCARC/basic.ll           | 16 ++++++++++++++++
 test/Transforms/ObjCARC/opt-catchswitch.ll |  4 ++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/lib/Transforms/ObjCARC/PtrState.cpp b/lib/Transforms/ObjCARC/PtrState.cpp
index 26dd416d618..6071ec3e4d9 100644
--- a/lib/Transforms/ObjCARC/PtrState.cpp
+++ b/lib/Transforms/ObjCARC/PtrState.cpp
@@ -232,7 +232,7 @@ bool BottomUpPtrState::HandlePotentialAlterRefCount(Instruction *Inst,
   Sequence S = GetSeq();
 
   // Check for possible releases.
-  if (!CanAlterRefCount(Inst, Ptr, PA, Class))
+  if (!CanDecrementRefCount(Inst, Ptr, PA, Class))
     return false;
 
   LLVM_DEBUG(dbgs() << "            CanAlterRefCount: Seq: " << S << "; "
@@ -383,7 +383,7 @@ bool TopDownPtrState::HandlePotentialAlterRefCount(Instruction *Inst,
                                                    ARCInstKind Class) {
   // Check for possible releases. Treat clang.arc.use as a releasing instruction
   // to prevent sinking a retain past it.
-  if (!CanAlterRefCount(Inst, Ptr, PA, Class) &&
+  if (!CanDecrementRefCount(Inst, Ptr, PA, Class) &&
       Class != ARCInstKind::IntrinsicUser)
     return false;
 
diff --git a/test/Transforms/ObjCARC/basic.ll b/test/Transforms/ObjCARC/basic.ll
index b25049c78be..ca27ca5c150 100644
--- a/test/Transforms/ObjCARC/basic.ll
+++ b/test/Transforms/ObjCARC/basic.ll
@@ -19,6 +19,7 @@ declare i8* @llvm.objc.unretainedPointer(i8*)
 
 declare void @use_pointer(i8*)
 declare void @callee()
+declare void @callee2(i8*, i8*)
 declare void @callee_fnptr(void ()*)
 declare void @invokee()
 declare i8* @returner()
@@ -3057,6 +3058,21 @@ define void @test67(i8* %x) {
   ret void
 }
 
+; CHECK-LABEL: define void @test68(
+; CHECK-NOT:     call
+; CHECK:         call void @callee2(
+; CHECK-NOT:     call
+; CHECK:         ret void
+
+define void @test68(i8* %a, i8* %b) {
+  call i8* @llvm.objc.retain(i8* %a)
+  call i8* @llvm.objc.retain(i8* %b)
+  call void @callee2(i8* %a, i8* %b)
+  call void @llvm.objc.release(i8* %b), !clang.imprecise_release !0
+  call void @llvm.objc.release(i8* %a), !clang.imprecise_release !0
+  ret void
+}
+
 !llvm.module.flags = !{!1}
 !llvm.dbg.cu = !{!3}
 
diff --git a/test/Transforms/ObjCARC/opt-catchswitch.ll b/test/Transforms/ObjCARC/opt-catchswitch.ll
index b627c11d89b..2cce99417b7 100644
--- a/test/Transforms/ObjCARC/opt-catchswitch.ll
+++ b/test/Transforms/ObjCARC/opt-catchswitch.ll
@@ -15,6 +15,9 @@ declare dllimport void @llvm.objc.release(i8*)
 define i8* @g(i8* %p, i8* %q) local_unnamed_addr personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
 entry:
   %0 = tail call i8* @llvm.objc.retain(i8* %p) #0
+  ; the following call prevents ARC optimizer from removing the retain/release
+  ; pair on %p
+  %v1 = call i8* @f(i8* null, i8* null)
   %1 = tail call i8* @llvm.objc.retain(i8* %q) #0
   %call = invoke i8* @f(i8* %p, i8* %q)
           to label %invoke.cont unwind label %catch.dispatch, !clang.arc.no_objc_arc_exceptions !0
@@ -40,6 +43,7 @@ cleanup:
 
 ; CHECK-LABEL: entry:
 ; CHECK-NEXT:    %0 = tail call i8* @llvm.objc.retain(i8* %p) #0
+; CHECK-NEXT:    call i8* @f(i8* null, i8* null)
 ; CHECK-NEXT:    %call = invoke i8* @f(i8* %p, i8* %q)
 ; CHECK-NEXT:            to label %invoke.cont unwind label %catch.dispatch