[PATCH] D55233: Add objc_retain and objc_release intrinsics and codegen them to their runtime methods

[Pete Cooper via Phabricator via llvm-commits]

pete added a comment.

In D55233#1318897 <https://reviews.llvm.org/D55233#1318897>, @ahatanak wrote:

> LGTM


Thanks!

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



================
Comment at: include/llvm/IR/Intrinsics.td:324
+//
+def int_objc_retain  : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty], [Returned<0>]>;
+def int_objc_release  : Intrinsic<[], [llvm_ptr_ty]>;
----------------
erik.pilkington wrote:
> 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?
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.


================
Comment at: include/llvm/IR/Intrinsics.td:324
+//
+def int_objc_retain  : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty], [Returned<0>]>;
+def int_objc_release  : Intrinsic<[], [llvm_ptr_ty]>;
----------------
ahatanak wrote:
> pete wrote:
> > erik.pilkington wrote:
> > > 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?
> > 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.
> 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?
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55233/new/

https://reviews.llvm.org/D55233