diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp --- a/clang/lib/CodeGen/CGCall.cpp +++ b/clang/lib/CodeGen/CGCall.cpp @@ -3112,6 +3112,13 @@ return CGF.Builder.CreateBitCast(result, resultType); } +static bool isObjCRetain(CodeGenFunction &CGF, const llvm::Value *V) { + auto *CI = dyn_cast_or_null(V); + if (!CI) + return false; + return CI->getCalledOperand() == CGF.CGM.getObjCEntrypoints().objc_retain; +} + /// If this is a +1 of the value of an immutable 'self', remove it. static llvm::Value *tryRemoveRetainOfSelf(CodeGenFunction &CGF, llvm::Value *result) { @@ -3124,9 +3131,9 @@ // Look for a retain call. llvm::CallInst *retainCall = - dyn_cast(result->stripPointerCasts()); - if (!retainCall || retainCall->getCalledOperand() != - CGF.CGM.getObjCEntrypoints().objc_retain) + dyn_cast(result->stripPointerCasts( + [&](const llvm::Value *V) -> bool { return !isObjCRetain(CGF, V); })); + if (!isObjCRetain(CGF, retainCall)) return nullptr; // Look for an ordinary load of 'self'. diff --git a/clang/test/CodeGenObjC/arc.m b/clang/test/CodeGenObjC/arc.m --- a/clang/test/CodeGenObjC/arc.m +++ b/clang/test/CodeGenObjC/arc.m @@ -7,30 +7,30 @@ // RUN: %clang_cc1 -fobjc-runtime=macosx-10.7.0 -triple x86_64-apple-darwin11 -Wno-objc-root-class -Wno-incompatible-pointer-types -Wno-arc-unsafe-retained-assign -emit-llvm -fblocks -fobjc-arc -fobjc-runtime-has-weak -o - %s | FileCheck -check-prefix=ARC-NATIVE %s // ARC-ALIEN: declare extern_weak void @llvm.objc.storeStrong(i8**, i8*) -// ARC-ALIEN: declare extern_weak i8* @llvm.objc.retain(i8*) +// ARC-ALIEN: declare extern_weak i8* @llvm.objc.retain(i8* returned) // ARC-ALIEN: declare extern_weak i8* @llvm.objc.autoreleaseReturnValue(i8*) // ARC-ALIEN: declare i8* @objc_msgSend(i8*, i8*, ...) [[NLB:#[0-9]+]] // ARC-ALIEN: declare extern_weak void @llvm.objc.release(i8*) -// ARC-ALIEN: declare extern_weak i8* @llvm.objc.retainAutoreleasedReturnValue(i8*) +// ARC-ALIEN: declare extern_weak i8* @llvm.objc.retainAutoreleasedReturnValue(i8* returned) // ARC-ALIEN: declare extern_weak i8* @llvm.objc.initWeak(i8**, i8*) // ARC-ALIEN: declare extern_weak i8* @llvm.objc.storeWeak(i8**, i8*) // ARC-ALIEN: declare extern_weak i8* @llvm.objc.loadWeakRetained(i8**) // ARC-ALIEN: declare extern_weak void @llvm.objc.destroyWeak(i8**) -// ARC-ALIEN: declare extern_weak i8* @llvm.objc.autorelease(i8*) -// ARC-ALIEN: declare extern_weak i8* @llvm.objc.retainAutorelease(i8*) +// ARC-ALIEN: declare extern_weak i8* @llvm.objc.autorelease(i8* returned) +// ARC-ALIEN: declare extern_weak i8* @llvm.objc.retainAutorelease(i8* returned) // ARC-NATIVE: declare void @llvm.objc.storeStrong(i8**, i8*) -// ARC-NATIVE: declare i8* @llvm.objc.retain(i8*) +// ARC-NATIVE: declare i8* @llvm.objc.retain(i8* returned) // ARC-NATIVE: declare i8* @llvm.objc.autoreleaseReturnValue(i8*) // ARC-NATIVE: declare i8* @objc_msgSend(i8*, i8*, ...) [[NLB:#[0-9]+]] // ARC-NATIVE: declare void @llvm.objc.release(i8*) -// ARC-NATIVE: declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*) +// ARC-NATIVE: declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8* returned) // ARC-NATIVE: declare i8* @llvm.objc.initWeak(i8**, i8*) // ARC-NATIVE: declare i8* @llvm.objc.storeWeak(i8**, i8*) // ARC-NATIVE: declare i8* @llvm.objc.loadWeakRetained(i8**) // ARC-NATIVE: declare void @llvm.objc.destroyWeak(i8**) -// ARC-NATIVE: declare i8* @llvm.objc.autorelease(i8*) -// ARC-NATIVE: declare i8* @llvm.objc.retainAutorelease(i8*) +// ARC-NATIVE: declare i8* @llvm.objc.autorelease(i8* returned) +// ARC-NATIVE: declare i8* @llvm.objc.retainAutorelease(i8* returned) // CHECK-LABEL: define{{.*}} void @test0 void test0(id x) { diff --git a/clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m b/clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m --- a/clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m +++ b/clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m @@ -28,6 +28,11 @@ // MSGS: {{call.*@objc_msgSend}} // CALLS: {{call.*@objc_alloc}} // CALLS: {{call.*@objc_allocWithZone}} + + // Note that calls to the intrinsics are not allowed for + // retain/release/autorelease they're marked `thisreturn`, which isn't + // guaranteed to be true for classes that define their own `-retain`, for + // example. Be sure to keep these as normal function calls: // CALLS: {{call.*@objc_retain}} // CALLS: {{call.*@objc_release}} // CALLS: {{tail call.*@objc_autorelease}} diff --git a/llvm/include/llvm/IR/Intrinsics.td b/llvm/include/llvm/IR/Intrinsics.td --- a/llvm/include/llvm/IR/Intrinsics.td +++ b/llvm/include/llvm/IR/Intrinsics.td @@ -414,7 +414,8 @@ // eliminate retain and releases where possible. def int_objc_autorelease : Intrinsic<[llvm_ptr_ty], - [llvm_ptr_ty]>; + [llvm_ptr_ty], + [Returned>]>; def int_objc_autoreleasePoolPop : Intrinsic<[], [llvm_ptr_ty]>; def int_objc_autoreleasePoolPush : Intrinsic<[llvm_ptr_ty], []>; def int_objc_autoreleaseReturnValue : Intrinsic<[llvm_ptr_ty], @@ -435,13 +436,17 @@ 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>]>; def int_objc_retainAutorelease : Intrinsic<[llvm_ptr_ty], - [llvm_ptr_ty]>; + [llvm_ptr_ty], + [Returned>]>; def int_objc_retainAutoreleaseReturnValue : Intrinsic<[llvm_ptr_ty], - [llvm_ptr_ty]>; + [llvm_ptr_ty], + [Returned>]>; def int_objc_retainAutoreleasedReturnValue : Intrinsic<[llvm_ptr_ty], - [llvm_ptr_ty]>; + [llvm_ptr_ty], + [Returned>]>; def int_objc_retainBlock : Intrinsic<[llvm_ptr_ty], [llvm_ptr_ty]>; def int_objc_storeStrong : Intrinsic<[], @@ -464,7 +469,8 @@ 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>]>; def int_objc_sync_enter : Intrinsic<[llvm_i32_ty], [llvm_ptr_ty]>; def int_objc_sync_exit : Intrinsic<[llvm_i32_ty], diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h --- a/llvm/include/llvm/IR/Value.h +++ b/llvm/include/llvm/IR/Value.h @@ -628,10 +628,15 @@ /// /// Returns the original uncasted value. If this is called on a non-pointer /// value, it returns 'this'. - const Value *stripPointerCasts() const; - Value *stripPointerCasts() { + /// + /// Queries the callback \p Func at each step, and stops if it returns \p + /// false. + const Value *stripPointerCasts(function_ref Func = + [](const Value *) { return true; }) const; + Value *stripPointerCasts(function_ref Func = + [](const Value *) { return true; }) { return const_cast( - static_cast(this)->stripPointerCasts()); + static_cast(this)->stripPointerCasts(Func)); } /// Strip off pointer casts, all-zero GEPs, address space casts, and aliases. diff --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp --- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp +++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp @@ -110,6 +110,16 @@ CallInst::TailCallKind TCK = CI->getTailCallKind(); NewCI->setTailCallKind(std::max(TCK, OverridingTCK)); + // Transfer the 'returned' attribute from the intrinsic to the call site. + // By applying this only to intrinsic call sites, we avoid applying it to + // non-ARC explicit calls to things like objc_retain which have not been + // auto-upgraded to use the intrinsics. + unsigned Index; + if (F.getAttributes().hasAttrSomewhere(Attribute::Returned, &Index) && + Index) + NewCI->addParamAttr(Index - AttributeList::FirstArgIndex, + Attribute::Returned); + if (!CI->use_empty()) CI->replaceAllUsesWith(NewCI); CI->eraseFromParent(); diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp --- a/llvm/lib/IR/Value.cpp +++ b/llvm/lib/IR/Value.cpp @@ -598,12 +598,14 @@ PSK_InBounds }; -template static void NoopCallback(const Value *) {} +template static bool NoopCallback(const Value *) { + return true; +} template static const Value *stripPointerCastsAndOffsets( const Value *V, - function_ref Func = NoopCallback) { + function_ref Func = NoopCallback) { if (!V->getType()->isPointerTy()) return V; @@ -613,7 +615,8 @@ Visited.insert(V); do { - Func(V); + if (!Func(V)) + return V; if (auto *GEP = dyn_cast(V)) { switch (StripKind) { case PSK_ZeroIndices: @@ -672,8 +675,9 @@ } } // end anonymous namespace -const Value *Value::stripPointerCasts() const { - return stripPointerCastsAndOffsets(this); +const Value * +Value::stripPointerCasts(function_ref Func) const { + return stripPointerCastsAndOffsets(this, Func); } const Value *Value::stripPointerCastsAndAliases() const { @@ -762,7 +766,10 @@ const Value * Value::stripInBoundsOffsets(function_ref Func) const { - return stripPointerCastsAndOffsets(this, Func); + return stripPointerCastsAndOffsets(this, [&](const Value *V) { + Func(V); + return true; + }); } bool Value::canBeFreed() const { diff --git a/llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll b/llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll --- a/llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll +++ b/llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll @@ -6,7 +6,7 @@ define i8* @test_objc_autorelease(i8* %arg0) { ; CHECK-LABEL: test_objc_autorelease ; CHECK-NEXT: entry -; CHECK-NEXT: %0 = notail call i8* @objc_autorelease(i8* %arg0) +; CHECK-NEXT: %0 = notail call i8* @objc_autorelease(i8* returned %arg0) ; CHECK-NEXT: ret i8* %0 entry: %0 = call i8* @llvm.objc.autorelease(i8* %arg0) @@ -113,20 +113,31 @@ ret void } +define i8* @test_objc_retain_intrinsic(i8* %arg0) { +; CHECK-LABEL: test_objc_retain_intrinsic +; CHECK-NEXT: entry +; CHECK-NEXT: %0 = tail call i8* @objc_retain(i8* returned %arg0) +; CHECK-NEXT: ret i8* %0 +entry: + %0 = call i8* @llvm.objc.retain(i8* %arg0) + ret i8* %0 +} + +; Explicit objc_retain calls should not be upgraded to be "thisreturn". define i8* @test_objc_retain(i8* %arg0) { ; CHECK-LABEL: test_objc_retain ; CHECK-NEXT: entry -; CHECK-NEXT: %0 = tail call i8* @objc_retain(i8* %arg0) +; CHECK-NEXT: %0 = call i8* @objc_retain(i8* %arg0) ; CHECK-NEXT: ret i8* %0 entry: - %0 = call i8* @llvm.objc.retain(i8* %arg0) + %0 = call i8* @objc_retain(i8* %arg0) ret i8* %0 } define i8* @test_objc_retainAutorelease(i8* %arg0) { ; CHECK-LABEL: test_objc_retainAutorelease ; CHECK-NEXT: entry -; CHECK-NEXT: %0 = call i8* @objc_retainAutorelease(i8* %arg0) +; CHECK-NEXT: %0 = call i8* @objc_retainAutorelease(i8* returned %arg0) ; CHECK-NEXT: ret i8* %0 entry: %0 = call i8* @llvm.objc.retainAutorelease(i8* %arg0) @@ -136,7 +147,7 @@ define i8* @test_objc_retainAutoreleaseReturnValue(i8* %arg0) { ; CHECK-LABEL: test_objc_retainAutoreleaseReturnValue ; CHECK-NEXT: entry -; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleaseReturnValue(i8* %arg0) +; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleaseReturnValue(i8* returned %arg0) ; CHECK-NEXT: ret i8* %0 entry: %0 = tail call i8* @llvm.objc.retainAutoreleaseReturnValue(i8* %arg0) @@ -146,7 +157,7 @@ define i8* @test_objc_retainAutoreleasedReturnValue(i8* %arg0) { ; CHECK-LABEL: test_objc_retainAutoreleasedReturnValue ; CHECK-NEXT: entry -; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %arg0) +; CHECK-NEXT: %0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* returned %arg0) ; CHECK-NEXT: ret i8* %0 entry: %0 = call i8* @llvm.objc.retainAutoreleasedReturnValue(i8* %arg0) @@ -226,7 +237,7 @@ define i8* @test_objc_retain_autorelease(i8* %arg0) { ; CHECK-LABEL: test_objc_retain_autorelease ; CHECK-NEXT: entry -; CHECK-NEXT: %0 = call i8* @objc_retain_autorelease(i8* %arg0) +; CHECK-NEXT: %0 = call i8* @objc_retain_autorelease(i8* returned %arg0) ; CHECK-NEXT: ret i8* %0 entry: %0 = call i8* @llvm.objc.retain.autorelease(i8* %arg0) @@ -265,6 +276,7 @@ declare void @llvm.objc.moveWeak(i8**, i8**) declare void @llvm.objc.release(i8*) declare i8* @llvm.objc.retain(i8*) +declare i8* @objc_retain(i8*) declare i8* @llvm.objc.retainAutorelease(i8*) declare i8* @llvm.objc.retainAutoreleaseReturnValue(i8*) declare i8* @llvm.objc.retainAutoreleasedReturnValue(i8*) @@ -281,6 +293,7 @@ attributes #0 = { nounwind } +; CHECK: declare i8* @objc_retain(i8*) [[NLB:#[0-9]+]] ; CHECK: declare i8* @objc_autorelease(i8*) ; CHECK: declare void @objc_autoreleasePoolPop(i8*) ; CHECK: declare i8* @objc_autoreleasePoolPush() @@ -291,8 +304,7 @@ ; 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 void @objc_release(i8*) [[NLB]] ; CHECK: declare i8* @objc_retainAutorelease(i8*) ; CHECK: declare i8* @objc_retainAutoreleaseReturnValue(i8*) ; CHECK: declare i8* @objc_retainAutoreleasedReturnValue(i8*)