[llvm-commits] [llvm] r172287 - in /llvm/trunk: lib/Transforms/Scalar/ObjCARC.cpp test/Transforms/ObjCARC/basic.ll test/Transforms/ObjCARC/contract.ll test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll test/Transforms/ObjCARC/rv.ll test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll
Michael Gottesman
mgottesman at apple.com
Fri Jan 11 17:25:16 PST 2013
Author: mgottesman
Date: Fri Jan 11 19:25:15 2013
New Revision: 172287
URL: http://llvm.org/viewvc/llvm-project?rev=172287&view=rev
Log:
Fixed a bug where we were tail calling objc_autorelease causing an object to not be placed into an autorelease pool.
The reason that this occurs is that tail calling objc_autorelease eventually
tail calls -[NSObject autorelease] which supports fast autorelease. This can
cause us to violate the semantic gaurantees of __autoreleasing variables that
assignment to an __autoreleasing variables always yields an object that is
placed into the innermost autorelease pool.
The fix included in this patch works by:
1. In the peephole optimization function OptimizeIndividualFunctions, always
remove tail call from objc_autorelease.
2. Whenever we convert to/from an objc_autorelease, set/unset the tail call
keyword as appropriate.
*NOTE* I also handled the case where objc_autorelease is converted in
OptimizeReturns to an autoreleaseRV which still violates the ARC semantics. I
will be removing that in a later patch and I wanted to make sure that the tree
is in a consistent state vis-a-vis ARC always.
Additionally some test cases are provided and all tests that have tail call marked
objc_autorelease keywords have been modified so that tail call has been removed.
*NOTE* One test fails due to a separate bug that I am going to commit soon. Thus
I marked the check line TMP: instead of CHECK: so make check does not fail.
Added:
llvm/trunk/test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp
llvm/trunk/test/Transforms/ObjCARC/basic.ll
llvm/trunk/test/Transforms/ObjCARC/contract.ll
llvm/trunk/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll
llvm/trunk/test/Transforms/ObjCARC/rv.ll
Modified: llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp?rev=172287&r1=172286&r2=172287&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/ObjCARC.cpp Fri Jan 11 19:25:15 2013
@@ -426,10 +426,20 @@
// IC_RetainBlock may be given a stack argument.
return Class == IC_Retain ||
Class == IC_RetainRV ||
- Class == IC_Autorelease ||
Class == IC_AutoreleaseRV;
}
+/// \brief Test if the given class represents instructions which are never safe
+/// to mark with the "tail" keyword.
+static bool IsNeverTail(InstructionClass Class) {
+ /// It is never safe to tail call objc_autorelease since by tail calling
+ /// objc_autorelease, we also tail call -[NSObject autorelease] which supports
+ /// fast autoreleasing causing our object to be potentially reclaimed from the
+ /// autorelease pool which violates the semantics of __autoreleasing types in
+ /// ARC.
+ return Class == IC_Autorelease;
+}
+
/// IsNoThrow - Test if the given class represents instructions which are always
/// safe to mark with the nounwind attribute..
static bool IsNoThrow(InstructionClass Class) {
@@ -2306,8 +2316,10 @@
" Old: "
<< *AutoreleaseRV << "\n");
- cast<CallInst>(AutoreleaseRV)->
+ CallInst *AutoreleaseRVCI = cast<CallInst>(AutoreleaseRV);
+ AutoreleaseRVCI->
setCalledFunction(getAutoreleaseCallee(F.getParent()));
+ AutoreleaseRVCI->setTailCall(false); // Never tail call objc_autorelease.
DEBUG(dbgs() << " New: "
<< *AutoreleaseRV << "\n");
@@ -2449,6 +2461,16 @@
cast<CallInst>(Inst)->setTailCall();
}
+ // Ensure that functions that can never have a "tail" keyword due to the
+ // semantics of ARC truly do not do so.
+ if (IsNeverTail(Class)) {
+ Changed = true;
+ DEBUG(dbgs() << "ObjCARCOpt::OptimizeIndividualCalls: Removing tail keyword"
+ " from function: " << *Inst <<
+ "\n");
+ cast<CallInst>(Inst)->setTailCall(false);
+ }
+
// Set nounwind as needed.
if (IsNoThrow(Class)) {
Changed = true;
@@ -3756,6 +3778,7 @@
Autorelease->setCalledFunction(getAutoreleaseRVCallee(F.getParent()));
DEBUG(dbgs() << " Out: " << *Autorelease
<< "\n");
+ Autorelease->setTailCall(); // Always tail call autoreleaseRV.
AutoreleaseClass = IC_AutoreleaseRV;
}
Modified: llvm/trunk/test/Transforms/ObjCARC/basic.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/basic.ll?rev=172287&r1=172286&r2=172287&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/ObjCARC/basic.ll (original)
+++ llvm/trunk/test/Transforms/ObjCARC/basic.ll Fri Jan 11 19:25:15 2013
@@ -405,7 +405,7 @@
; CHECK: define void @test11(
; CHECK: tail call i8* @objc_retain(i8* %x) nounwind
-; CHECK: tail call i8* @objc_autorelease(i8* %0) nounwind
+; CHECK: call i8* @objc_autorelease(i8* %0) nounwind
; CHECK: }
define void @test11(i8* %x) nounwind {
entry:
@@ -465,7 +465,7 @@
; CHECK: tail call i8* @objc_retain(i8* %x) nounwind
; CHECK: tail call i8* @objc_retain(i8* %x) nounwind
; CHECK: @use_pointer(i8* %x)
-; CHECK: tail call i8* @objc_autorelease(i8* %x) nounwind
+; CHECK: call i8* @objc_autorelease(i8* %x) nounwind
; CHECK: }
define void @test13(i8* %x, i64 %n) {
entry:
@@ -1452,7 +1452,7 @@
; CHECK: define void @test46(
; CHECK: tail call i8* @objc_retain(i8* %p) nounwind
; CHECK: true:
-; CHECK: tail call i8* @objc_autorelease(i8* %p) nounwind
+; CHECK: call i8* @objc_autorelease(i8* %p) nounwind
define void @test46(i8* %p, i1 %a) {
entry:
call i8* @objc_retain(i8* %p)
Modified: llvm/trunk/test/Transforms/ObjCARC/contract.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/contract.ll?rev=172287&r1=172286&r2=172287&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/ObjCARC/contract.ll (original)
+++ llvm/trunk/test/Transforms/ObjCARC/contract.ll Fri Jan 11 19:25:15 2013
@@ -39,7 +39,7 @@
define void @test2(i8* %x) nounwind {
entry:
%0 = tail call i8* @objc_retain(i8* %x) nounwind
- tail call i8* @objc_autorelease(i8* %0) nounwind
+ call i8* @objc_autorelease(i8* %0) nounwind
call void @use_pointer(i8* %x)
ret void
}
@@ -66,7 +66,7 @@
entry:
tail call i8* @objc_retain(i8* %x) nounwind
call void @use_pointer(i8* %x)
- tail call i8* @objc_autorelease(i8* %x) nounwind
+ call i8* @objc_autorelease(i8* %x) nounwind
ret void
}
@@ -84,7 +84,7 @@
entry:
tail call i8* @objc_retain(i8* %x) nounwind
call void @use_pointer(i8* %x)
- tail call i8* @objc_autorelease(i8* %x) nounwind
+ call i8* @objc_autorelease(i8* %x) nounwind
tail call void @objc_release(i8* %x) nounwind
ret void
}
@@ -94,7 +94,7 @@
; CHECK: define void @test5(
; CHECK: tail call i8* @objc_retain(i8* %p) nounwind
; CHECK: true:
-; CHECK: tail call i8* @objc_autorelease(i8* %0) nounwind
+; CHECK: call i8* @objc_autorelease(i8* %0) nounwind
; CHECK: }
define void @test5(i8* %p, i1 %a) {
entry:
@@ -102,7 +102,7 @@
br i1 %a, label %true, label %false
true:
- tail call i8* @objc_autorelease(i8* %p) nounwind
+ call i8* @objc_autorelease(i8* %p) nounwind
call void @use_pointer(i8* %p)
ret void
Modified: llvm/trunk/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll?rev=172287&r1=172286&r2=172287&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll (original)
+++ llvm/trunk/test/Transforms/ObjCARC/move-and-form-retain-autorelease.ll Fri Jan 11 19:25:15 2013
@@ -212,7 +212,7 @@
br label %bb104
bb104: ; preds = %bb99, %bb57
- %tmp105 = tail call i8* @objc_autorelease(i8* %tmp72) nounwind
+ %tmp105 = call i8* @objc_autorelease(i8* %tmp72) nounwind
%tmp106 = bitcast i8* %tmp105 to %14*
tail call void @objc_release(i8* %tmp85) nounwind
%tmp107 = bitcast %18* %tmp47 to i8*
Modified: llvm/trunk/test/Transforms/ObjCARC/rv.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/rv.ll?rev=172287&r1=172286&r2=172287&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/ObjCARC/rv.ll (original)
+++ llvm/trunk/test/Transforms/ObjCARC/rv.ll Fri Jan 11 19:25:15 2013
@@ -150,7 +150,7 @@
; Don't apply the RV optimization to autorelease if there's no retain.
; CHECK: define i8* @test9(i8* %p)
-; CHECK: tail call i8* @objc_autorelease(i8* %p)
+; CHECK: call i8* @objc_autorelease(i8* %p)
define i8* @test9(i8* %p) {
call i8* @objc_autorelease(i8* %p)
ret i8* %p
@@ -174,7 +174,7 @@
; CHECK: define i8* @test11(i8* %p)
; CHECK: tail call i8* @objc_retain(i8* %p)
; CHECK-NEXT: call void @use_pointer(i8* %p)
-; CHECK: tail call i8* @objc_autorelease(i8* %p)
+; CHECK: call i8* @objc_autorelease(i8* %p)
; CHECK-NEXT: ret i8* %p
define i8* @test11(i8* %p) {
%1 = call i8* @objc_retain(i8* %p)
@@ -201,7 +201,7 @@
; CHECK: define i8* @test13(
; CHECK: tail call i8* @objc_retainAutoreleasedReturnValue(i8* %p)
-; CHECK: tail call i8* @objc_autorelease(i8* %p)
+; CHECK: call i8* @objc_autorelease(i8* %p)
; CHECK: ret i8* %p
define i8* @test13() {
%p = call i8* @returner()
@@ -323,7 +323,7 @@
; Convert autoreleaseRV to autorelease.
; CHECK: define void @test23(
-; CHECK: tail call i8* @objc_autorelease(i8* %p) nounwind
+; CHECK: call i8* @objc_autorelease(i8* %p) nounwind
define void @test23(i8* %p) {
store i8 0, i8* %p
call i8* @objc_autoreleaseReturnValue(i8* %p)
Added: llvm/trunk/test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll?rev=172287&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll (added)
+++ llvm/trunk/test/Transforms/ObjCARC/tail-call-invariant-enforcement.ll Fri Jan 11 19:25:15 2013
@@ -0,0 +1,84 @@
+; RUN: opt -objc-arc -S < %s | FileCheck %s
+
+declare i8* @objc_release(i8* %x)
+declare i8* @objc_retain(i8* %x)
+declare i8* @objc_autorelease(i8* %x)
+declare i8* @objc_autoreleaseReturnValue(i8* %x)
+declare i8* @objc_retainAutoreleasedReturnValue(i8* %x)
+
+; Never tail call objc_autorelease.
+define i8* @test0(i8* %x) {
+entry:
+ ; CHECK: %tmp0 = call i8* @objc_autorelease(i8* %x)
+ %tmp0 = call i8* @objc_autorelease(i8* %x)
+ ; CHECK: %tmp1 = call i8* @objc_autorelease(i8* %x)
+ %tmp1 = tail call i8* @objc_autorelease(i8* %x)
+
+ ret i8* %x
+}
+
+; Always tail call autoreleaseReturnValue.
+define i8* @test1(i8* %x) {
+entry:
+ ; CHECK: %tmp0 = tail call i8* @objc_autoreleaseReturnValue(i8* %x)
+ %tmp0 = call i8* @objc_autoreleaseReturnValue(i8* %x)
+ ; CHECK: %tmp1 = tail call i8* @objc_autoreleaseReturnValue(i8* %x)
+ %tmp1 = tail call i8* @objc_autoreleaseReturnValue(i8* %x)
+ ret i8* %x
+}
+
+; Always tail call objc_retain.
+define i8* @test2(i8* %x) {
+entry:
+ ; CHECK: %tmp0 = tail call i8* @objc_retain(i8* %x)
+ %tmp0 = call i8* @objc_retain(i8* %x)
+ ; CHECK: %tmp1 = tail call i8* @objc_retain(i8* %x)
+ %tmp1 = tail call i8* @objc_retain(i8* %x)
+ ret i8* %x
+}
+
+define i8* @tmp(i8* %x) {
+ ret i8* %x
+}
+
+; Always tail call objc_retainAutoreleasedReturnValue.
+define i8* @test3(i8* %x) {
+entry:
+ %y = call i8* @tmp(i8* %x)
+ ; CHECK: %tmp0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %y)
+ %tmp0 = call i8* @objc_retainAutoreleasedReturnValue(i8* %y)
+ %z = call i8* @tmp(i8* %x)
+ ; CHECK: %tmp1 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %z)
+ %tmp1 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %z)
+ ret i8* %x
+}
+
+; By itself, we should never change whether or not objc_release is tail called.
+define i8* @test4(i8* %x) {
+entry:
+ ; CHECK: %tmp0 = call i8* @objc_release(i8* %x)
+ %tmp0 = call i8* @objc_release(i8* %x)
+ ; CHECK: %tmp1 = tail call i8* @objc_release(i8* %x)
+ %tmp1 = tail call i8* @objc_release(i8* %x)
+ ret i8* %x
+}
+
+; If we convert a tail called @objc_autoreleaseReturnValue to an
+; @objc_autorelease, ensure that the tail call is removed.
+define i8* @test5(i8* %x) {
+entry:
+ ; TMP: %tmp0 = call i8* @objc_autorelease(i8* %x)
+ %tmp0 = tail call i8* @objc_autoreleaseReturnValue(i8* %x)
+ ret i8* %tmp0
+}
+
+; If we convert a called @objc_autorelease to an @objc_autoreleaseReturnValue,
+; ensure that the tail call is added.
+define i8* @test6(i8* %x) {
+entry:
+ ; CHECK: %tmp0 = tail call i8* @objc_retain(i8* %x)
+ %tmp0 = tail call i8* @objc_retain(i8* %x)
+ ; CHECK: %tmp1 = tail call i8* @objc_autoreleaseReturnValue(i8* %x)
+ %tmp1 = call i8* @objc_autorelease(i8* %x)
+ ret i8* %x
+}
More information about the llvm-commits
mailing list