Add objc_retain and objc_release intrinsics and codegen them to their runtime methods

[pete]

Summary

This is the first in a series of patches to move objc retain/release to intrinsics and have the ARC optimizer work on those instead.

For some context, see clang r263607 which converted [a retain] to objc_retain(a). That was ultimately reverted as the ARC optimizer couldn't handle the new calls.

The eventual solution is to move the ARC inserted retain/release calls to intrinsics and only optimize the intrinsics. Manually inserted retain/release will then no longer be optimized.

[erik.pilkington]

Can you add these intrinsics to the LangRef? Seems pretty reasonable, but I don't feel comfortable LGTMing.

include/llvm/IR/Intrinsics.td

324 ↗

(On Diff #176459)

I think it would be good to have these for all the entry points used by the arc optimizer, even if it isn't necessary to fix r263607. I guess it doesn't make sense to block this though, but maybe add a FIXME if you agree?

[ahatanak]

include/llvm/IR/Intrinsics.td

324 ↗

(On Diff #176459)

Would it be possible to add a few comments that explain the motivation for these intrinsics?

Also, r294872 added attribute "returned" to some of the ObjC ARC functions, but was later reverted because of a mis-compile caused by a bug in the ARC optimizer. The ARC optimizer bug that caused the mis-compile hasn't been fixed yet, so I wonder whether it's safe to add "Returned<0>" here.

Also, are there any other properties (IntrArgMemOnly, for example) you can use here?

[pete]

Updated to add a comment to Intrinsics.td, remove the ReturnedValue<> and added all of the ARC methods from https://clang.llvm.org/docs/AutomaticReferenceCounting.html#runtime-support

[ahatanak]

LGTM

[pete]

In D55233#1318897, @ahatanak wrote:

LGTM

Thanks!

Erik, did you have anything else required before landing this?

include/llvm/IR/Intrinsics.td

324 ↗

(On Diff #176459)

Yeah, i was torn about doing that or not. I think its a good idea so let me try now and see how much work it is.

Certainly it reduces the churn on the next patch which will actually move the ARC optimizer to use the new methods.

324 ↗

(On Diff #176459)

Ah yeah will add comments.

I was unsure about returned<0> too as it may cause issues. Good idea to remove it for now.

I don't think we can add IntrArgMemOnly or similar as if these methods are overridden they can have arbitrary side effects.

[erik.pilkington]

In D55233#1319010, @pete wrote:

In D55233#1318897, @ahatanak wrote:

LGTM

Thanks!

Erik, did you have anything else required before landing this?

Patch LGTM, but are you planning on adding these to the LangRef? Its probably good enough to just list them and refer to https://clang.llvm.org/docs/AutomaticReferenceCounting.html#runtime-support for their semantics.

[pete]

Discovered that objc_storeStrong is actually wrong in the documentation. Changing it from returning id to returning void. Will also update the docs.

llvm/trunk/include/llvm/IR/Intrinsics.td

def int_gcroot  : Intrinsic<[],
                             [llvm_ptrptr_ty, llvm_ptr_ty]>;
 def int_gcread  : Intrinsic<[llvm_ptr_ty],
                             [llvm_ptr_ty, llvm_ptrptr_ty],
                             [IntrReadMem, IntrArgMemOnly]>;
 def int_gcwrite : Intrinsic<[],
                             [llvm_ptr_ty, llvm_ptr_ty, llvm_ptrptr_ty],
                             [IntrArgMemOnly, NoCapture<1>, NoCapture<2>]>;
 
+//===------------------- ObjC ARC runtime Intrinsics --------------------===//
+//
+// Note these are to support the Objective-C ARC optimizer which wants to
+// eliminate retain and releases where possible.
+
+def int_objc_autorelease                    : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptr_ty]>;
+def int_objc_autoreleasePoolPop             : Intrinsic<[], [llvm_ptr_ty]>;
+def int_objc_autoreleasePoolPush            : Intrinsic<[llvm_ptr_ty], []>;
+def int_objc_autoreleaseReturnValue         : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptr_ty]>;
+def int_objc_copyWeak                       : Intrinsic<[],
+                                                        [llvm_ptrptr_ty,
+                                                         llvm_ptrptr_ty]>;
+def int_objc_destroyWeak                    : Intrinsic<[], [llvm_ptrptr_ty]>;
+def int_objc_initWeak                       : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptrptr_ty,
+                                                         llvm_ptr_ty]>;
+def int_objc_loadWeak                       : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptrptr_ty]>;
+def int_objc_loadWeakRetained               : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptrptr_ty]>;
+def int_objc_moveWeak                       : Intrinsic<[],
+                                                        [llvm_ptrptr_ty,
+                                                         llvm_ptrptr_ty]>;
+def int_objc_release                        : Intrinsic<[], [llvm_ptr_ty]>;
+def int_objc_retain                         : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptr_ty]>;
+def int_objc_retainAutorelease              : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptr_ty]>;
+def int_objc_retainAutoreleaseReturnValue   : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptr_ty]>;
+def int_objc_retainAutoreleasedReturnValue  : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptr_ty]>;
+def int_objc_retainBlock                    : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptr_ty]>;
+def int_objc_storeStrong                    : Intrinsic<[],
+                                                        [llvm_ptrptr_ty,
+                                                         llvm_ptr_ty]>;
+def int_objc_storeWeak                      : Intrinsic<[llvm_ptr_ty],
+                                                        [llvm_ptrptr_ty,
+                                                         llvm_ptr_ty]>;
+
+
 //===--------------------- Code Generator Intrinsics ----------------------===//
 //
 def int_returnaddress : Intrinsic<[llvm_ptr_ty], [llvm_i32_ty], [IntrNoMem]>;
 def int_addressofreturnaddress : Intrinsic<[llvm_ptr_ty], [], [IntrNoMem]>;
 def int_frameaddress  : Intrinsic<[llvm_ptr_ty], [llvm_i32_ty], [IntrNoMem]>;
 def int_sponentry  : Intrinsic<[llvm_ptr_ty], [], [IntrNoMem]>;
 def int_read_register  : Intrinsic<[llvm_anyint_ty], [llvm_metadata_ty],
                                    [IntrReadMem], "llvm.read_register">;

llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

case Intrinsic::icall_branch_funnel: {
     return nullptr;
   }
 
   case Intrinsic::wasm_landingpad_index:
     // Information this intrinsic contained has been transferred to
     // MachineFunction in SelectionDAGISel::PrepareEHLandingPad. We can safely
     // delete it now.
     return nullptr;
+  case Intrinsic::objc_autorelease:
+    return "objc_autorelease";
+  case Intrinsic::objc_autoreleasePoolPop:
+    return "objc_autoreleasePoolPop";
+  case Intrinsic::objc_autoreleasePoolPush:
+    return "objc_autoreleasePoolPush";
+  case Intrinsic::objc_autoreleaseReturnValue:
+    return "objc_autoreleaseReturnValue";
+  case Intrinsic::objc_copyWeak:
+    return "objc_copyWeak";
+  case Intrinsic::objc_destroyWeak:
+    return "objc_destroyWeak";
+  case Intrinsic::objc_initWeak:
+    return "objc_initWeak";
+  case Intrinsic::objc_loadWeak:
+    return "objc_loadWeak";
+  case Intrinsic::objc_loadWeakRetained:
+    return "objc_loadWeakRetained";
+  case Intrinsic::objc_moveWeak:
+    return "objc_moveWeak";
+  case Intrinsic::objc_release:
+    return "objc_release";
+  case Intrinsic::objc_retain:
+    return "objc_retain";
+  case Intrinsic::objc_retainAutorelease:
+    return "objc_retainAutorelease";
+  case Intrinsic::objc_retainAutoreleaseReturnValue:
+    return "objc_retainAutoreleaseReturnValue";
+  case Intrinsic::objc_retainAutoreleasedReturnValue:
+    return "objc_retainAutoreleasedReturnValue";
+  case Intrinsic::objc_retainBlock:
+    return "objc_retainBlock";
+  case Intrinsic::objc_storeStrong:
+    return "objc_storeStrong";
+  case Intrinsic::objc_storeWeak:
+    return "objc_storeWeak";
   }
 }
 
 void SelectionDAGBuilder::visitConstrainedFPIntrinsic(
     const ConstrainedFPIntrinsic &FPI) {
   SDLoc sdl = getCurSDLoc();
   unsigned Opcode;
   switch (FPI.getIntrinsicID()) {

llvm/trunk/test/CodeGen/X86/objc-arc.ll

+; RUN: llc < %s -mtriple=x86_64-apple-darwin | FileCheck %s
+
+; Make sure calls to the objc intrinsics are translated to calls in to the
+; runtime
+
+define i8* @test_objc_autorelease(i8* %arg0) {
+; CHECK-LABEL: test_objc_autorelease
+; CHECK: callq _objc_autorelease
+entry:
+    %0 = call i8* @llvm.objc.autorelease(i8* %arg0)
+  ret i8* %0
+}
+
+define void @test_objc_autoreleasePoolPop(i8* %arg0) {
+; CHECK-LABEL: test_objc_autoreleasePoolPop
+; CHECK: callq _objc_autoreleasePoolPop
+entry:
+    call void @llvm.objc.autoreleasePoolPop(i8* %arg0)
+    ret void
+}
+
+define i8* @test_objc_autoreleasePoolPush() {
+; CHECK-LABEL: test_objc_autoreleasePoolPush
+; CHECK: callq _objc_autoreleasePoolPush
+entry:
+    %0 = call i8* @llvm.objc.autoreleasePoolPush()
+  ret i8* %0
+}
+
+define i8* @test_objc_autoreleaseReturnValue(i8* %arg0) {
+; CHECK-LABEL: test_objc_autoreleaseReturnValue
+; CHECK: callq _objc_autoreleaseReturnValue
+entry:
+    %0 = call i8* @llvm.objc.autoreleaseReturnValue(i8* %arg0)
+  ret i8* %0
+}
+
+define void @test_objc_copyWeak(i8** %arg0, i8** %arg1) {
+; CHECK-LABEL: test_objc_copyWeak
+; CHECK: callq _objc_copyWeak
+entry:
+    call void @llvm.objc.copyWeak(i8** %arg0, i8** %arg1)
+    ret void
+}
+
+define void @test_objc_destroyWeak(i8** %arg0) {
+; CHECK-LABEL: test_objc_destroyWeak
+; CHECK: callq _objc_destroyWeak
+entry:
+    call void @llvm.objc.destroyWeak(i8** %arg0)
+    ret void
+}
+
+define i8* @test_objc_initWeak(i8** %arg0, i8* %arg1) {
+; CHECK-LABEL: test_objc_initWeak
+; CHECK: callq _objc_initWeak
+entry:
+    %0 = call i8* @llvm.objc.initWeak(i8** %arg0, i8* %arg1)
+  ret i8* %0
+}
+
+define i8* @test_objc_loadWeak(i8** %arg0) {
+; CHECK-LABEL: test_objc_loadWeak
+; CHECK: callq _objc_loadWeak
+entry:
+    %0 = call i8* @llvm.objc.loadWeak(i8** %arg0)
+  ret i8* %0
+}
+
+define i8* @test_objc_loadWeakRetained(i8** %arg0) {
+; CHECK-LABEL: test_objc_loadWeakRetained
+; CHECK: callq _objc_loadWeakRetained
+entry:
+    %0 = call i8* @llvm.objc.loadWeakRetained(i8** %arg0)
+  ret i8* %0
+}
+
+define void @test_objc_moveWeak(i8** %arg0, i8** %arg1) {
+; CHECK-LABEL: test_objc_moveWeak
+; CHECK: callq _objc_moveWeak
+entry:
+    call void @llvm.objc.moveWeak(i8** %arg0, i8** %arg1)
+    ret void
+}
+
+define void @test_objc_release(i8* %arg0) {
+; CHECK-LABEL: test_objc_release
+; CHECK: callq _objc_release
+entry:
+    call void @llvm.objc.release(i8* %arg0)
+    ret void
+}
+
+define i8* @test_objc_retain(i8* %arg0) {
+; CHECK-LABEL: test_objc_retain
+; CHECK: callq _objc_retain
+entry:
+    %0 = call i8* @llvm.objc.retain(i8* %arg0)
+  ret i8* %0
+}
+
+define i8* @test_objc_retainAutorelease(i8* %arg0) {
+; CHECK-LABEL: test_objc_retainAutorelease
+; CHECK: callq _objc_retainAutorelease
+entry:
+    %0 = call i8* @llvm.objc.retainAutorelease(i8* %arg0)
+  ret i8* %0
+}
+
+define i8* @test_objc_retainAutoreleaseReturnValue(i8* %arg0) {
+; CHECK-LABEL: test_objc_retainAutoreleaseReturnValue
+; CHECK: callq _objc_retainAutoreleaseReturnValue
+entry:
+    %0 = call i8* @llvm.objc.retainAutoreleaseReturnValue(i8* %arg0)
+  ret i8* %0
+}
+
+define i8* @test_objc_retainAutoreleasedReturnValue(i8* %arg0) {
+; CHECK-LABEL: test_objc_retainAutoreleasedReturnValue
+; CHECK: callq _objc_retainAutoreleasedReturnValue
+entry:
+    %0 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %arg0)
+  ret i8* %0
+}
+
+define i8* @test_objc_retainBlock(i8* %arg0) {
+; CHECK-LABEL: test_objc_retainBlock
+; CHECK: callq _objc_retainBlock
+entry:
+    %0 = call i8* @llvm.objc.retainBlock(i8* %arg0)
+  ret i8* %0
+}
+
+define void @test_objc_storeStrong(i8** %arg0, i8* %arg1) {
+; CHECK-LABEL: test_objc_storeStrong
+; CHECK: callq _objc_storeStrong
+entry:
+    call void @llvm.objc.storeStrong(i8** %arg0, i8* %arg1)
+  ret void
+}
+
+define i8* @test_objc_storeWeak(i8** %arg0, i8* %arg1) {
+; CHECK-LABEL: test_objc_storeWeak
+; CHECK: callq _objc_storeWeak
+entry:
+    %0 = call i8* @llvm.objc.storeWeak(i8** %arg0, i8* %arg1)
+  ret i8* %0
+}
+
+declare i8* @llvm.objc.autorelease(i8*)
+declare void @llvm.objc.autoreleasePoolPop(i8*)
+declare i8* @llvm.objc.autoreleasePoolPush()
+declare i8* @llvm.objc.autoreleaseReturnValue(i8*)
+declare void @llvm.objc.copyWeak(i8**, i8**)
+declare void @llvm.objc.destroyWeak(i8**)
+declare i8* @llvm.objc.initWeak(i8**, i8*)
+declare i8* @llvm.objc.loadWeak(i8**)
+declare i8* @llvm.objc.loadWeakRetained(i8**)
+declare void @llvm.objc.moveWeak(i8**, i8**)
+declare void @llvm.objc.release(i8*)
+declare i8* @llvm.objc.retain(i8*)
+declare i8* @llvm.objc.retainAutorelease(i8*)
+declare i8* @llvm.objc.retainAutoreleaseReturnValue(i8*)
+declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*)
+declare i8* @llvm.objc.retainBlock(i8*)
+declare void @llvm.objc.storeStrong(i8**, i8*)
+declare i8* @llvm.objc.storeWeak(i8**, i8*)