jroelofs updated this revision to Diff 369584.
jroelofs added a comment.
Herald added a subscriber: dexonsmith.
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105671/new/
https://reviews.llvm.org/D105671
Files:
clang/lib/CodeGen/CGCall.cpp
clang/test/CodeGenObjC/arc.m
clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m
llvm/include/llvm/IR/Intrinsics.td
llvm/include/llvm/IR/Value.h
llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
llvm/lib/IR/Value.cpp
llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
Index: llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
===================================================================
--- llvm/test/Transforms/PreISelIntrinsicLowering/objc-arc.ll
+++ 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*)
Index: llvm/lib/IR/Value.cpp
===================================================================
--- llvm/lib/IR/Value.cpp
+++ llvm/lib/IR/Value.cpp
@@ -598,12 +598,14 @@
PSK_InBounds
};
-template <PointerStripKind StripKind> static void NoopCallback(const Value *) {}
+template <PointerStripKind StripKind> static bool NoopCallback(const Value *) {
+ return true;
+}
template <PointerStripKind StripKind>
static const Value *stripPointerCastsAndOffsets(
const Value *V,
- function_ref<void(const Value *)> Func = NoopCallback<StripKind>) {
+ function_ref<bool(const Value *)> Func = NoopCallback<StripKind>) {
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<GEPOperator>(V)) {
switch (StripKind) {
case PSK_ZeroIndices:
@@ -672,8 +675,9 @@
}
} // end anonymous namespace
-const Value *Value::stripPointerCasts() const {
- return stripPointerCastsAndOffsets<PSK_ZeroIndices>(this);
+const Value *
+Value::stripPointerCasts(function_ref<bool(const Value *)> Func) const {
+ return stripPointerCastsAndOffsets<PSK_ZeroIndices>(this, Func);
}
const Value *Value::stripPointerCastsAndAliases() const {
@@ -762,7 +766,10 @@
const Value *
Value::stripInBoundsOffsets(function_ref<void(const Value *)> Func) const {
- return stripPointerCastsAndOffsets<PSK_InBounds>(this, Func);
+ return stripPointerCastsAndOffsets<PSK_InBounds>(this, [&](const Value *V) {
+ Func(V);
+ return true;
+ });
}
bool Value::canBeFreed() const {
Index: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
===================================================================
--- llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ 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();
Index: llvm/include/llvm/IR/Value.h
===================================================================
--- llvm/include/llvm/IR/Value.h
+++ 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<bool(const Value *)> Func =
+ [](const Value *) { return true; }) const;
+ Value *stripPointerCasts(function_ref<bool(const Value *)> Func =
+ [](const Value *) { return true; }) {
return const_cast<Value *>(
- static_cast<const Value *>(this)->stripPointerCasts());
+ static_cast<const Value *>(this)->stripPointerCasts(Func));
}
/// Strip off pointer casts, all-zero GEPs, address space casts, and aliases.
Index: llvm/include/llvm/IR/Intrinsics.td
===================================================================
--- llvm/include/llvm/IR/Intrinsics.td
+++ 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<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],
@@ -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<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<[],
@@ -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<ArgIndex<0>>]>;
def int_objc_sync_enter : Intrinsic<[llvm_i32_ty],
[llvm_ptr_ty]>;
def int_objc_sync_exit : Intrinsic<[llvm_i32_ty],
Index: clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m
===================================================================
--- clang/test/CodeGenObjC/convert-messages-to-runtime-calls.m
+++ 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}}
Index: clang/test/CodeGenObjC/arc.m
===================================================================
--- clang/test/CodeGenObjC/arc.m
+++ 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) {
Index: clang/lib/CodeGen/CGCall.cpp
===================================================================
--- clang/lib/CodeGen/CGCall.cpp
+++ 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<llvm::CallInst>(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<llvm::CallInst>(result->stripPointerCasts());
- if (!retainCall || retainCall->getCalledOperand() !=
- CGF.CGM.getObjCEntrypoints().objc_retain)
+ dyn_cast<llvm::CallInst>(result->stripPointerCasts(
+ [&](const llvm::Value *V) -> bool { return !isObjCRetain(CGF, V); }));
+ if (!isObjCRetain(CGF, retainCall))
return nullptr;
// Look for an ordinary load of 'self'.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
