[ObjC][ARC] Delete ObjC runtime calls if the argument is a global variable

[ahatanak]

Summary

AFAIK, the only time clang emits runtime calls like objc_retain or objc_release on a global variable is when the object is a string literal or a global block and retaining or releasing them is a no-op since NSConstantString and __NSGlobalBlock are retain-agnostic. This patch teaches ARC optimizer to delete the calls when the call argument is a global variable.

rdar://problem/49839633

[rjmccall]

I thought we already did this optimization; you might want to talk to Dan or Michael about it.

It's a good optimization in the abstract.

[ahatanak]

I didn't find anything in the past commit log that mentions this optimization . Dan/Michael, do you know the reason ARC optimizer hasn't tried to remove these no-op calls?

[gottesmm]

I don't remember. That being said, this should be a conservative optimization. What if someone adds in the future a different global where this isn't a no-op. Is it possible if you know what the global is to add a whitelist that you only care about those 2 things.

[ahatanak]

Check the presence of attribute "arc_retain_agnostic" on the global variable before deleting the ARC calls.

[ahatanak]

In D62433#1520061, @gottesmm wrote:

I don't remember. That being said, this should be a conservative optimization. What if someone adds in the future a different global where this isn't a no-op. Is it possible if you know what the global is to add a whitelist that you only care about those 2 things.

I agree we should be conservative here, so I made it check the presence of "arc_retain_agnostic" on the global. I'll post a clang patch that adds the attribute to the globals shortly.

[rjmccall]

Okay, that's fine with me.

[gottesmm]

I looked at the other one. Beyond my question about the name, this looks great!

llvm/trunk/include/llvm/Analysis/ObjCARCInstKind.h

 /// Test if the given class represents instructions which return their
 /// argument verbatim.
 bool IsForwarding(ARCInstKind Class);
 
 /// Test if the given class represents instructions which do nothing if
 /// passed a null pointer.
 bool IsNoopOnNull(ARCInstKind Class);
 
+/// Test if the given class represents instructions which do nothing if
+/// passed a global variable.
+bool IsNoopOnGlobal(ARCInstKind Class);
+
 /// Test if the given class represents instructions which are always safe
 /// to mark with the "tail" keyword.
 bool IsAlwaysTail(ARCInstKind Class);
 
 /// Test if the given class represents instructions which are never safe
 /// to mark with the "tail" keyword.
 bool IsNeverTail(ARCInstKind Class);
 

llvm/trunk/lib/Analysis/ObjCARCInstKind.cpp

bool llvm::objcarc::IsNoopOnNull(ARCInstKind Class) {
   case ARCInstKind::User:
   case ARCInstKind::None:
   case ARCInstKind::NoopCast:
     return false;
   }
   llvm_unreachable("covered switch isn't covered?");
 }
 
+/// Test if the given class represents instructions which do nothing if
+/// passed a global variable.
+bool llvm::objcarc::IsNoopOnGlobal(ARCInstKind Class) {
+  switch (Class) {
+  case ARCInstKind::Retain:
+  case ARCInstKind::RetainRV:
+  case ARCInstKind::ClaimRV:
+  case ARCInstKind::Release:
+  case ARCInstKind::Autorelease:
+  case ARCInstKind::AutoreleaseRV:
+  case ARCInstKind::RetainBlock:
+  case ARCInstKind::FusedRetainAutorelease:
+  case ARCInstKind::FusedRetainAutoreleaseRV:
+    return true;
+  case ARCInstKind::AutoreleasepoolPush:
+  case ARCInstKind::AutoreleasepoolPop:
+  case ARCInstKind::LoadWeakRetained:
+  case ARCInstKind::StoreWeak:
+  case ARCInstKind::InitWeak:
+  case ARCInstKind::LoadWeak:
+  case ARCInstKind::MoveWeak:
+  case ARCInstKind::CopyWeak:
+  case ARCInstKind::DestroyWeak:
+  case ARCInstKind::StoreStrong:
+  case ARCInstKind::IntrinsicUser:
+  case ARCInstKind::CallOrUser:
+  case ARCInstKind::Call:
+  case ARCInstKind::User:
+  case ARCInstKind::None:
+  case ARCInstKind::NoopCast:
+    return false;
+  }
+  llvm_unreachable("covered switch isn't covered?");
+}
+
 /// Test if the given class represents instructions which are always safe
 /// to mark with the "tail" keyword.
 bool llvm::objcarc::IsAlwaysTail(ARCInstKind Class) {
   // ARCInstKind::RetainBlock may be given a stack argument.
   switch (Class) {
   case ARCInstKind::Retain:
   case ARCInstKind::RetainRV:
   case ARCInstKind::ClaimRV:

llvm/trunk/lib/Transforms/ObjCARC/ObjCARCOpts.cpp

void ObjCARCOpt::OptimizeIndividualCalls(Function &F) {
   // Visit all objc_* calls in F.
   for (inst_iterator I = inst_begin(&F), E = inst_end(&F); I != E; ) {
     Instruction *Inst = &*I++;
 
     ARCInstKind Class = GetBasicARCInstKind(Inst);
 
     LLVM_DEBUG(dbgs() << "Visiting: Class: " << Class << "; " << *Inst << "\n");
 
+    // Some of the ARC calls can be deleted if their arguments are global
+    // variables that are inert in ARC.
+    if (IsNoopOnGlobal(Class)) {
+      Value *Opnd = Inst->getOperand(0);
+      if (auto *GV = dyn_cast<GlobalVariable>(Opnd->stripPointerCasts()))
+        if (GV->hasAttribute("objc_arc_inert")) {
+          if (!Inst->getType()->isVoidTy())
+            Inst->replaceAllUsesWith(Opnd);
+          Inst->eraseFromParent();
+          continue;
+        }
+    }
+
     switch (Class) {
     default: break;
 
     // Delete no-op casts. These function calls have special semantics, but
     // the semantics are entirely implemented via lowering in the front-end,
     // so by the time they reach the optimizer, they are just no-op calls
     // which return their argument.
     //

llvm/trunk/test/Transforms/ObjCARC/inert-global.ll

+; RUN: opt -objc-arc -S < %s | FileCheck %s
+
+%0 = type opaque
+%struct.__NSConstantString_tag = type { i32*, i32, i8*, i64 }
+%struct.__block_descriptor = type { i64, i64 }
+
+@__CFConstantStringClassReference = external global [0 x i32]
+@.str = private unnamed_addr constant [4 x i8] c"abc\00", section "__TEXT,__cstring,cstring_literals", align 1
+@.str1 = private unnamed_addr constant [4 x i8] c"def\00", section "__TEXT,__cstring,cstring_literals", align 1
+@_unnamed_cfstring_ = private global %struct.__NSConstantString_tag { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 1992, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str, i32 0, i32 0), i64 3 }, section "__DATA,__cfstring", align 8 #0
+@_unnamed_cfstring_wo_attr = private global %struct.__NSConstantString_tag { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 1992, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @.str1, i32 0, i32 0), i64 3 }, section "__DATA,__cfstring", align 8
+@_NSConcreteGlobalBlock = external global i8*
+@.str.1 = private unnamed_addr constant [6 x i8] c"v8@?0\00", align 1
+@"__block_descriptor_32_e5_v8@?0l" = linkonce_odr hidden unnamed_addr constant { i64, i64, i8*, i8* } { i64 0, i64 32, i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str.1, i32 0, i32 0), i8* null }, align 8
+@__block_literal_global = internal constant { i8**, i32, i32, i8*, %struct.__block_descriptor* } { i8** @_NSConcreteGlobalBlock, i32 1342177280, i32 0, i8* bitcast (void (i8*)* @__globalBlock_block_invoke to i8*), %struct.__block_descriptor* bitcast ({ i64, i64, i8*, i8* }* @"__block_descriptor_32_e5_v8@?0l" to %struct.__block_descriptor*) }, align 8 #0
+
+; CHECK-LABEL: define %0* @stringLiteral()
+; CHECK-NOT: call
+; CHECK: ret %0* bitcast (%struct.__NSConstantString_tag* @_unnamed_cfstring_ to %0*)
+
+define %0* @stringLiteral() {
+  %1 = tail call i8* @llvm.objc.retain(i8* bitcast (%struct.__NSConstantString_tag* @_unnamed_cfstring_ to i8*))
+  %2 = call i8* @llvm.objc.autorelease(i8* bitcast (%struct.__NSConstantString_tag* @_unnamed_cfstring_ to i8*))
+  ret %0* bitcast (%struct.__NSConstantString_tag* @_unnamed_cfstring_ to %0*)
+}
+
+; CHECK-LABEL: define %0* @stringLiteral1()
+; CHECK-NEXT: call i8* @llvm.objc.retain(
+; CHECK-NEXT: call i8* @llvm.objc.autorelease(
+; CHECK-NEXT: ret %0*
+
+define %0* @stringLiteral1() {
+  %1 = tail call i8* @llvm.objc.retain(i8* bitcast (%struct.__NSConstantString_tag* @_unnamed_cfstring_wo_attr to i8*))
+  %2 = call i8* @llvm.objc.autorelease(i8* bitcast (%struct.__NSConstantString_tag* @_unnamed_cfstring_wo_attr to i8*))
+  ret %0* bitcast (%struct.__NSConstantString_tag* @_unnamed_cfstring_wo_attr to %0*)
+}
+
+; CHECK-LABEL: define void (...)* @globalBlock()
+; CHECK-NOT: call
+; CHECK: %[[V1:.*]] = bitcast i8* bitcast ({ i8**, i32, i32, i8*, %struct.__block_descriptor* }* @__block_literal_global to i8*) to void (...)*
+; CHECK-NEXT: ret void (...)* %[[V1]]
+
+define void (...)* @globalBlock() {
+  %1 = tail call i8* @llvm.objc.retainBlock(i8* bitcast ({ i8**, i32, i32, i8*, %struct.__block_descriptor* }* @__block_literal_global to i8*))
+  %2 = tail call i8* @llvm.objc.retainBlock(i8* %1)
+  %3 = bitcast i8* %2 to void (...)*
+  tail call void @llvm.objc.release(i8* %1)
+  %4 = tail call i8* @llvm.objc.autoreleaseReturnValue(i8* %2)
+  ret void (...)* %3
+}
+
+define internal void @__globalBlock_block_invoke(i8* nocapture readnone) {
+  tail call void @foo()
+  ret void
+}
+
+declare void @foo()
+
+declare i8* @llvm.objc.retain(i8*) local_unnamed_addr
+declare i8* @llvm.objc.autoreleaseReturnValue(i8*) local_unnamed_addr
+declare i8* @llvm.objc.retainBlock(i8*) local_unnamed_addr
+declare void @llvm.objc.release(i8*) local_unnamed_addr
+declare i8* @llvm.objc.autorelease(i8*) local_unnamed_addr
+
+attributes #0 = { "objc_arc_inert" }