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

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

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

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

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.

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 --------------------===//
+//
+def int_objc_retain  : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty], [Returned<0>]>;
+def int_objc_release  : Intrinsic<[], [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">;

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_retain:
+    return "objc_retain";
+  case Intrinsic::objc_release:
+    return "objc_release";
   }
 }
 
 void SelectionDAGBuilder::visitConstrainedFPIntrinsic(
     const ConstrainedFPIntrinsic &FPI) {
   SDLoc sdl = getCurSDLoc();
   unsigned Opcode;
   switch (FPI.getIntrinsicID()) {

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* @test1(i8* %arg) {
+; CHECK-LABEL: test1
+; CHECK: callq _objc_retain
+entry:
+  %0 = call i8* @llvm.objc.retain(i8* %arg)
+  ret i8* %0
+}
+
+define void @test2(i8* %arg) {
+; CHECK-LABEL: test2
+; CHECK: callq _objc_release
+entry:
+  call void @llvm.objc.release(i8* %arg)
+  ret void
+}
+
+declare i8* @llvm.objc.retain(i8*)
+declare void @llvm.objc.release(i8* %arg)