[Intrinsics][ObjC] Mark objc_retain and friends as thisreturn.

[jroelofs]

Summary

https://clang.llvm.org/docs/AutomaticReferenceCounting.html#arc-runtime-objc-retain

rdar://79869679

[jroelofs]

clang-format

[jroelofs]

ping

[rjmccall]

IIUC, we sometimes turn -retain message sends into calls to objc_retain and so on. Outside of ARC (the only time they're valid), these calls don't have the semantics of guaranteeing to return their self argument. So I think this change is only fine if we're not applying it when doing that non-ARC transformation.

[jroelofs]

Apply the attribute only to call sites emitted via calls to the intrinsics.

[jroelofs]

Explain why the attribute goes on the call site and not the callee.

[jroelofs]

Add a note/check to the [x retain] => objc_retain(x) test explaining why it shouldn't be lowered as an intrinsic call in the future.

[jroelofs]

In D105671#2884483, @rjmccall wrote:

IIUC, we sometimes turn -retain message sends into calls to objc_retain and so on. Outside of ARC (the only time they're valid), these calls don't have the semantics of guaranteeing to return their self argument. So I think this change is only fine if we're not applying it when doing that non-ARC transformation.

I believe this is covered now. The -retain => objc_retain transformation emits normal function calls, and objc_retain calls are only transformed into intrinsic calls when ARC is enabled.

[rjmccall]

Okay, thanks, seems like a good approach.

clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m

34 ↗

(On Diff #364215)

This only actually tests for the absence of these lines between the CHECK-LABEL and the first CALLS line below, so I don't think it does much.

[jroelofs]

Rebased.

Also, turns out that stripPointerCasts() can see through the thisreturn attribute, which defeats a self retain optimization, breaking one of the clang tests. I tweaked a callback to make it configurable. Let me know if you see a better way of dealing with that.

[rjmccall]

How does stripPointerCasts "see through" an attribute on the underlying call instruction?

[jroelofs]

In D105671#2978248, @rjmccall wrote:

How does stripPointerCasts "see through" an attribute on the underlying call instruction?

https://github.com/llvm/llvm-project/blob/bf77b11277411f6725cf09a66feb36d2c14bc8a7/llvm/lib/IR/Value.cpp#L652

[rjmccall]

Whoa. That does not seem like it's in the contract for stripPointerCasts() generically. At best that should be enabled by the strip kind.

[nikic]

Please separate the change to stripPointerCasts() into a separate review.

[jroelofs]

In D105671#4547848, @nikic wrote:

Please separate the change to stripPointerCasts() into a separate review.

https://reviews.llvm.org/D156735

[jroelofs]

Move the stripPointerCasts() change into its own review: https://reviews.llvm.org/D156735

[jroelofs]

oops, uploaded the wrong patch to this review.

[jroelofs]

Rebase. Avoid the dependency on D156735

llvm/include/llvm/IR/Intrinsics.td

def int_gcwrite : Intrinsic<[],
                              NoCapture<ArgIndex<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]>;
+                                                        [llvm_ptr_ty],
+                                                        [Returned<ArgIndex<0>>]>;
 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]>;
+                                                        [llvm_ptr_ty],
+                                                        [Returned<ArgIndex<0>>]>;
 def int_objc_retainAutorelease              : Intrinsic<[llvm_ptr_ty],
-                                                        [llvm_ptr_ty]>;
+                                                        [llvm_ptr_ty],
+                                                        [Returned<ArgIndex<0>>]>;
 def int_objc_retainAutoreleaseReturnValue   : Intrinsic<[llvm_ptr_ty],
-                                                        [llvm_ptr_ty]>;
+                                                        [llvm_ptr_ty],
+                                                        [Returned<ArgIndex<0>>]>;
 def int_objc_retainAutoreleasedReturnValue  : Intrinsic<[llvm_ptr_ty],
-                                                        [llvm_ptr_ty]>;
+                                                        [llvm_ptr_ty],
+                                                        [Returned<ArgIndex<0>>]>;
 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]>;
 def int_objc_clang_arc_use                  : Intrinsic<[],
                                                         [llvm_vararg_ty]>;
 def int_objc_clang_arc_noop_use : DefaultAttrsIntrinsic<[],
                                                         [llvm_vararg_ty],
                                                         [IntrInaccessibleMemOnly]>;
 def int_objc_unsafeClaimAutoreleasedReturnValue : Intrinsic<[llvm_ptr_ty],
                                                             [llvm_ptr_ty]>;
 def int_objc_retainedObject                 : Intrinsic<[llvm_ptr_ty],
                                                         [llvm_ptr_ty]>;
 def int_objc_unretainedObject               : Intrinsic<[llvm_ptr_ty],
                                                         [llvm_ptr_ty]>;
 def int_objc_unretainedPointer              : Intrinsic<[llvm_ptr_ty],
                                                         [llvm_ptr_ty]>;
 def int_objc_retain_autorelease             : Intrinsic<[llvm_ptr_ty],
-                                                        [llvm_ptr_ty]>;
+                                                        [llvm_ptr_ty],
+                                                        [Returned<ArgIndex<0>>]>;
 def int_objc_sync_enter                     : Intrinsic<[llvm_i32_ty],
                                                         [llvm_ptr_ty]>;
 def int_objc_sync_exit                      : Intrinsic<[llvm_i32_ty],
                                                         [llvm_ptr_ty]>;
 def int_objc_arc_annotation_topdown_bbstart : Intrinsic<[],
                                                         [llvm_ptrptr_ty,
                                                          llvm_ptrptr_ty]>;
 def int_objc_arc_annotation_topdown_bbend   : Intrinsic<[],

llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp

static bool lowerObjCCall(Function &F, const char *NewFn,
 
   if (Function *Fn = dyn_cast<Function>(FCache.getCallee())) {
     Fn->setLinkage(F.getLinkage());
     if (setNonLazyBind && !Fn->isWeakForLinker()) {
       // If we have Native ARC, set nonlazybind attribute for these APIs for
       // performance.
       Fn->addFnAttr(Attribute::NonLazyBind);
     }
+
+    // Transfer the 'returned' attribute from the intrinsic to the callee.
+    unsigned Index;
+    if (F.getAttributes().hasAttrSomewhere(Attribute::Returned, &Index) &&
+        Index)
+      Fn->addParamAttr(Index - AttributeList::FirstArgIndex,
+                       Attribute::Returned);
   }
 
   CallInst::TailCallKind OverridingTCK = getOverridingTailCallKind(F);
 
   for (auto I = F.use_begin(), E = F.use_end(); I != E;) {
     auto *CI = cast<CallInst>(I->getUser());
     assert(CI->getCalledFunction() && "Cannot lower an indirect call!");
     ++I;

llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll

 ; CHECK-NEXT: entry
 ; CHECK-NEXT: %0 = call i32 @objc_sync_exit(i8* %arg0)
 ; CHECK-NEXT: ret i32 %0
 entry:
   %0 = call i32 @llvm.objc.sync.exit(i8* %arg0)
   ret i32 %0
 }
 
-declare i8* @llvm.objc.autorelease(i8*)
+declare i8* @llvm.objc.autorelease(i8* returned)
 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 extern_weak 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.retain(i8* returned)
+declare i8* @llvm.objc.retainAutorelease(i8* returned)
+declare i8* @llvm.objc.retainAutoreleaseReturnValue(i8* returned)
+declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8* returned)
 declare i8* @llvm.objc.retainBlock(i8*)
 declare void @llvm.objc.storeStrong(i8**, i8*)
 declare i8* @llvm.objc.storeWeak(i8**, i8*)
 declare i8* @llvm.objc.unsafeClaimAutoreleasedReturnValue(i8*)
 declare i8* @llvm.objc.retainedObject(i8*)
 declare i8* @llvm.objc.unretainedObject(i8*)
 declare i8* @llvm.objc.unretainedPointer(i8*)
-declare i8* @llvm.objc.retain.autorelease(i8*)
+declare i8* @llvm.objc.retain.autorelease(i8* returned)
 declare i32 @llvm.objc.sync.enter(i8*)
 declare i32 @llvm.objc.sync.exit(i8*)
 
 attributes #0 = { nounwind }
 
-; CHECK: declare i8* @objc_autorelease(i8*)
+; CHECK: declare i8* @objc_autorelease(i8* returned)
 ; CHECK: declare void @objc_autoreleasePoolPop(i8*)
 ; CHECK: declare i8* @objc_autoreleasePoolPush()
 ; CHECK: declare i8* @objc_autoreleaseReturnValue(i8*)
 ; CHECK: declare void @objc_copyWeak(i8**, i8**)
 ; CHECK: declare void @objc_destroyWeak(i8**)
 ; CHECK: declare extern_weak i8* @objc_initWeak(i8**, i8*)
 ; CHECK: declare i8* @objc_loadWeak(i8**)
 ; CHECK: declare i8* @objc_loadWeakRetained(i8**)
 ; CHECK: declare void @objc_moveWeak(i8**, i8**)
 ; CHECK: declare void @objc_release(i8*) [[NLB:#[0-9]+]]
-; CHECK: declare i8* @objc_retain(i8*) [[NLB]]
-; CHECK: declare i8* @objc_retainAutorelease(i8*)
-; CHECK: declare i8* @objc_retainAutoreleaseReturnValue(i8*)
-; CHECK: declare i8* @objc_retainAutoreleasedReturnValue(i8*)
+; CHECK: declare i8* @objc_retain(i8* returned) [[NLB]]
+; CHECK: declare i8* @objc_retainAutorelease(i8* returned)
+; CHECK: declare i8* @objc_retainAutoreleaseReturnValue(i8* returned)
+; CHECK: declare i8* @objc_retainAutoreleasedReturnValue(i8* returned)
 ; CHECK: declare i8* @objc_retainBlock(i8*)
 ; CHECK: declare void @objc_storeStrong(i8**, i8*)
 ; CHECK: declare i8* @objc_storeWeak(i8**, i8*)
 ; CHECK: declare i8* @objc_unsafeClaimAutoreleasedReturnValue(i8*)
 ; CHECK: declare i8* @objc_retainedObject(i8*)
 ; CHECK: declare i8* @objc_unretainedObject(i8*)
 ; CHECK: declare i8* @objc_unretainedPointer(i8*)
-; CHECK: declare i8* @objc_retain_autorelease(i8*)
+; CHECK: declare i8* @objc_retain_autorelease(i8* returned)
 ; CHECK: declare i32 @objc_sync_enter(i8*)
 ; CHECK: declare i32 @objc_sync_exit(i8*)
 
 ; CHECK: attributes #0 = { nounwind }
 ; CHECK: attributes [[NLB]] = { nonlazybind }