[llvm] r182670 - [objc-arc] KnownSafe does not imply that it is safe to perform code motion across CFG edges since even if it is safe to remove RR pairs, we may still be able to move a retain/release into a loop.
Michael Gottesman
mgottesman at apple.com
Fri May 24 13:44:05 PDT 2013
Author: mgottesman
Date: Fri May 24 15:44:05 2013
New Revision: 182670
URL: http://llvm.org/viewvc/llvm-project?rev=182670&view=rev
Log:
[objc-arc] KnownSafe does not imply that it is safe to perform code motion across CFG edges since even if it is safe to remove RR pairs, we may still be able to move a retain/release into a loop.
rdar://13949644
Modified:
llvm/trunk/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
llvm/trunk/test/Transforms/ObjCARC/allocas.ll
Modified: llvm/trunk/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/ObjCARC/ObjCARCOpts.cpp?rev=182670&r1=182669&r2=182670&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/ObjCARC/ObjCARCOpts.cpp (original)
+++ llvm/trunk/lib/Transforms/ObjCARC/ObjCARCOpts.cpp Fri May 24 15:44:05 2013
@@ -455,8 +455,13 @@ namespace {
/// sequence.
SmallPtrSet<Instruction *, 2> ReverseInsertPts;
+ /// If this is true, we cannot perform code motion but can still remove
+ /// retain/release pairs.
+ bool CFGHazardAfflicted;
+
RRInfo() :
- KnownSafe(false), IsTailCallRelease(false), ReleaseMetadata(0) {}
+ KnownSafe(false), IsTailCallRelease(false), ReleaseMetadata(0),
+ CFGHazardAfflicted(false) {}
void clear();
@@ -472,6 +477,7 @@ void RRInfo::clear() {
ReleaseMetadata = 0;
Calls.clear();
ReverseInsertPts.clear();
+ CFGHazardAfflicted = false;
}
namespace {
@@ -559,6 +565,7 @@ PtrState::Merge(const PtrState &Other, b
RRI.IsTailCallRelease = RRI.IsTailCallRelease &&
Other.RRI.IsTailCallRelease;
RRI.Calls.insert(Other.RRI.Calls.begin(), Other.RRI.Calls.end());
+ RRI.CFGHazardAfflicted |= Other.RRI.CFGHazardAfflicted;
// Merge the insert point sets. If there are any differences,
// that makes this a partial merge.
@@ -1690,6 +1697,7 @@ static void CheckForUseCFGHazard(const S
PtrState &S,
bool &SomeSuccHasSame,
bool &AllSuccsHaveSame,
+ bool &NotAllSeqEqualButKnownSafe,
bool &ShouldContinue) {
switch (SuccSSeq) {
case S_CanRelease: {
@@ -1697,6 +1705,7 @@ static void CheckForUseCFGHazard(const S
S.ClearSequenceProgress();
break;
}
+ S.RRI.CFGHazardAfflicted = true;
ShouldContinue = true;
break;
}
@@ -1708,6 +1717,8 @@ static void CheckForUseCFGHazard(const S
case S_MovableRelease:
if (!S.RRI.KnownSafe && !SuccSRRIKnownSafe)
AllSuccsHaveSame = false;
+ else
+ NotAllSeqEqualButKnownSafe = true;
break;
case S_Retain:
llvm_unreachable("bottom-up pointer in retain state!");
@@ -1723,7 +1734,8 @@ static void CheckForCanReleaseCFGHazard(
const bool SuccSRRIKnownSafe,
PtrState &S,
bool &SomeSuccHasSame,
- bool &AllSuccsHaveSame) {
+ bool &AllSuccsHaveSame,
+ bool &NotAllSeqEqualButKnownSafe) {
switch (SuccSSeq) {
case S_CanRelease:
SomeSuccHasSame = true;
@@ -1734,6 +1746,8 @@ static void CheckForCanReleaseCFGHazard(
case S_Use:
if (!S.RRI.KnownSafe && !SuccSRRIKnownSafe)
AllSuccsHaveSame = false;
+ else
+ NotAllSeqEqualButKnownSafe = true;
break;
case S_Retain:
llvm_unreachable("bottom-up pointer in retain state!");
@@ -1769,6 +1783,7 @@ ObjCARCOpt::CheckForCFGHazards(const Bas
const TerminatorInst *TI = cast<TerminatorInst>(&BB->back());
bool SomeSuccHasSame = false;
bool AllSuccsHaveSame = true;
+ bool NotAllSeqEqualButKnownSafe = false;
succ_const_iterator SI(TI), SE(TI, false);
@@ -1800,17 +1815,17 @@ ObjCARCOpt::CheckForCFGHazards(const Bas
switch(S.GetSeq()) {
case S_Use: {
bool ShouldContinue = false;
- CheckForUseCFGHazard(SuccSSeq, SuccSRRIKnownSafe, S,
- SomeSuccHasSame, AllSuccsHaveSame,
+ CheckForUseCFGHazard(SuccSSeq, SuccSRRIKnownSafe, S, SomeSuccHasSame,
+ AllSuccsHaveSame, NotAllSeqEqualButKnownSafe,
ShouldContinue);
if (ShouldContinue)
continue;
break;
}
case S_CanRelease: {
- CheckForCanReleaseCFGHazard(SuccSSeq, SuccSRRIKnownSafe,
- S, SomeSuccHasSame,
- AllSuccsHaveSame);
+ CheckForCanReleaseCFGHazard(SuccSSeq, SuccSRRIKnownSafe, S,
+ SomeSuccHasSame, AllSuccsHaveSame,
+ NotAllSeqEqualButKnownSafe);
break;
}
case S_Retain:
@@ -1825,8 +1840,15 @@ ObjCARCOpt::CheckForCFGHazards(const Bas
// If the state at the other end of any of the successor edges
// matches the current state, require all edges to match. This
// guards against loops in the middle of a sequence.
- if (SomeSuccHasSame && !AllSuccsHaveSame)
+ if (SomeSuccHasSame && !AllSuccsHaveSame) {
S.ClearSequenceProgress();
+ } else if (NotAllSeqEqualButKnownSafe) {
+ // If we would have cleared the state foregoing the fact that we are known
+ // safe, stop code motion. This is because whether or not it is safe to
+ // remove RR pairs via KnownSafe is an orthogonal concept to whether we
+ // are allowed to perform code motion.
+ S.RRI.CFGHazardAfflicted = true;
+ }
}
}
@@ -2489,6 +2511,7 @@ ObjCARCOpt::ConnectTDBUTraversals(DenseM
// we are dealing with a retainable object with multiple provenance sources.
bool KnownSafeTD = true, KnownSafeBU = true;
bool MultipleOwners = false;
+ bool CFGHazardAfflicted = false;
// Connect the dots between the top-down-collected RetainsToMove and
// bottom-up-collected ReleasesToMove to form sets of related calls.
@@ -2565,6 +2588,7 @@ ObjCARCOpt::ConnectTDBUTraversals(DenseM
assert(It != Releases.end());
const RRInfo &NewReleaseRRI = It->second;
KnownSafeBU &= NewReleaseRRI.KnownSafe;
+ CFGHazardAfflicted |= NewReleaseRRI.CFGHazardAfflicted;
for (SmallPtrSet<Instruction *, 2>::const_iterator
LI = NewReleaseRRI.Calls.begin(),
LE = NewReleaseRRI.Calls.end(); LI != LE; ++LI) {
@@ -2618,6 +2642,14 @@ ObjCARCOpt::ConnectTDBUTraversals(DenseM
// less aggressive solution which is.
if (NewDelta != 0)
return false;
+
+ // At this point, we are not going to remove any RR pairs, but we still are
+ // able to move RR pairs. If one of our pointers is afflicted with
+ // CFGHazards, we cannot perform such code motion so exit early.
+ const bool WillPerformCodeMotion = RetainsToMove.ReverseInsertPts.size() ||
+ ReleasesToMove.ReverseInsertPts.size();
+ if (CFGHazardAfflicted && WillPerformCodeMotion)
+ return false;
}
// Determine whether the original call points are balanced in the retain and
Modified: llvm/trunk/test/Transforms/ObjCARC/allocas.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/ObjCARC/allocas.ll?rev=182670&r1=182669&r2=182670&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/ObjCARC/allocas.ll (original)
+++ llvm/trunk/test/Transforms/ObjCARC/allocas.ll Fri May 24 15:44:05 2013
@@ -18,6 +18,8 @@ declare void @callee()
declare void @callee_fnptr(void ()*)
declare void @invokee()
declare i8* @returner()
+declare i8* @returner1()
+declare i8* @returner2()
declare void @bar(i32 ()*)
declare void @use_alloca(i8**)
@@ -295,6 +297,204 @@ bb3:
ret void
}
+; CHECK: define void @test2d(i8* %x)
+; CHECK: @objc_retain(i8* %x)
+; CHECK: @objc_retain(i8* %x)
+; CHECK: @objc_release(i8* %y)
+; CHECK: @objc_release(i8* %x)
+; CHECK: ret void
+; CHECK: }
+define void @test2d(i8* %x) {
+entry:
+ tail call i8* @objc_retain(i8* %x)
+ br label %bb1
+
+bb1:
+ %Abb1 = alloca i8*, i32 3
+ %gepbb11 = getelementptr i8** %Abb1, i32 2
+ store i8* %x, i8** %gepbb11, align 8
+ %gepbb12 = getelementptr i8** %Abb1, i32 2
+ %ybb1 = load i8** %gepbb12
+ br label %bb3
+
+bb2:
+ %Abb2 = alloca i8*, i32 4
+ %gepbb21 = getelementptr i8** %Abb2, i32 2
+ store i8* %x, i8** %gepbb21, align 8
+ %gepbb22 = getelementptr i8** %Abb2, i32 2
+ %ybb2 = load i8** %gepbb22
+ br label %bb3
+
+bb3:
+ %A = phi i8** [ %Abb1, %bb1 ], [ %Abb2, %bb2 ]
+ %y = phi i8* [ %ybb1, %bb1 ], [ %ybb2, %bb2 ]
+ tail call i8* @objc_retain(i8* %x)
+ call void @use_alloca(i8** %A)
+ call void @objc_release(i8* %y), !clang.imprecise_release !0
+ call void @use_pointer(i8* %x)
+ call void @objc_release(i8* %x), !clang.imprecise_release !0
+ ret void
+}
+
+; Make sure in the presense of allocas, if we find a cfghazard we do not perform
+; code motion even if we are known safe. These two concepts are separate and
+; should be treated as such.
+;
+; rdar://13949644
+
+; CHECK: define void @test3a() {
+; CHECK: entry:
+; CHECK: @objc_retainAutoreleasedReturnValue
+; CHECK: @objc_retain
+; CHECK: @objc_retain
+; CHECK: @objc_retain
+; CHECK: @objc_retain
+; CHECK: arraydestroy.body:
+; CHECK: @objc_release
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.done:
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.body1:
+; CHECK: @objc_release
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.done1:
+; CHECK: @objc_release
+; CHECK: ret void
+; CHECK: }
+define void @test3a() {
+entry:
+ %keys = alloca [2 x i8*], align 16
+ %objs = alloca [2 x i8*], align 16
+
+ %call1 = call i8* @returner()
+ %tmp0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call1)
+
+ %objs.begin = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 0
+ tail call i8* @objc_retain(i8* %call1)
+ store i8* %call1, i8** %objs.begin, align 8
+ %objs.elt = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 1
+ tail call i8* @objc_retain(i8* %call1)
+ store i8* %call1, i8** %objs.elt
+
+ %call2 = call i8* @returner1()
+ %call3 = call i8* @returner2()
+ %keys.begin = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 0
+ tail call i8* @objc_retain(i8* %call2)
+ store i8* %call2, i8** %keys.begin, align 8
+ %keys.elt = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 1
+ tail call i8* @objc_retain(i8* %call3)
+ store i8* %call3, i8** %keys.elt
+
+ %gep = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 2
+ br label %arraydestroy.body
+
+arraydestroy.body:
+ %arraydestroy.elementPast = phi i8** [ %gep, %entry ], [ %arraydestroy.element, %arraydestroy.body ]
+ %arraydestroy.element = getelementptr inbounds i8** %arraydestroy.elementPast, i64 -1
+ %destroy_tmp = load i8** %arraydestroy.element, align 8
+ call void @objc_release(i8* %destroy_tmp), !clang.imprecise_release !0
+ %objs_ptr = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 0
+ %arraydestroy.cmp = icmp eq i8** %arraydestroy.element, %objs_ptr
+ br i1 %arraydestroy.cmp, label %arraydestroy.done, label %arraydestroy.body
+
+arraydestroy.done:
+ %gep1 = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 2
+ br label %arraydestroy.body1
+
+arraydestroy.body1:
+ %arraydestroy.elementPast1 = phi i8** [ %gep1, %arraydestroy.done ], [ %arraydestroy.element1, %arraydestroy.body1 ]
+ %arraydestroy.element1 = getelementptr inbounds i8** %arraydestroy.elementPast1, i64 -1
+ %destroy_tmp1 = load i8** %arraydestroy.element1, align 8
+ call void @objc_release(i8* %destroy_tmp1), !clang.imprecise_release !0
+ %keys_ptr = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 0
+ %arraydestroy.cmp1 = icmp eq i8** %arraydestroy.element1, %keys_ptr
+ br i1 %arraydestroy.cmp1, label %arraydestroy.done1, label %arraydestroy.body1
+
+arraydestroy.done1:
+ call void @objc_release(i8* %call1), !clang.imprecise_release !0
+ ret void
+}
+
+; Make sure that even though we stop said code motion we still allow for
+; pointers to be removed if we are known safe in both directions.
+;
+; rdar://13949644
+
+; CHECK: define void @test3b() {
+; CHECK: entry:
+; CHECK: @objc_retainAutoreleasedReturnValue
+; CHECK: @objc_retain
+; CHECK: @objc_retain
+; CHECK: @objc_retain
+; CHECK: @objc_retain
+; CHECK: arraydestroy.body:
+; CHECK: @objc_release
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.done:
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.body1:
+; CHECK: @objc_release
+; CHECK-NOT: @objc_release
+; CHECK: arraydestroy.done1:
+; CHECK: @objc_release
+; CHECK: ret void
+; CHECK: }
+define void @test3b() {
+entry:
+ %keys = alloca [2 x i8*], align 16
+ %objs = alloca [2 x i8*], align 16
+
+ %call1 = call i8* @returner()
+ %tmp0 = tail call i8* @objc_retainAutoreleasedReturnValue(i8* %call1)
+ %tmp1 = tail call i8* @objc_retain(i8* %call1)
+
+ %objs.begin = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 0
+ tail call i8* @objc_retain(i8* %call1)
+ store i8* %call1, i8** %objs.begin, align 8
+ %objs.elt = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 1
+ tail call i8* @objc_retain(i8* %call1)
+ store i8* %call1, i8** %objs.elt
+
+ %call2 = call i8* @returner1()
+ %call3 = call i8* @returner2()
+ %keys.begin = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 0
+ tail call i8* @objc_retain(i8* %call2)
+ store i8* %call2, i8** %keys.begin, align 8
+ %keys.elt = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 1
+ tail call i8* @objc_retain(i8* %call3)
+ store i8* %call3, i8** %keys.elt
+
+ %gep = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 2
+ br label %arraydestroy.body
+
+arraydestroy.body:
+ %arraydestroy.elementPast = phi i8** [ %gep, %entry ], [ %arraydestroy.element, %arraydestroy.body ]
+ %arraydestroy.element = getelementptr inbounds i8** %arraydestroy.elementPast, i64 -1
+ %destroy_tmp = load i8** %arraydestroy.element, align 8
+ call void @objc_release(i8* %destroy_tmp), !clang.imprecise_release !0
+ %objs_ptr = getelementptr inbounds [2 x i8*]* %objs, i64 0, i64 0
+ %arraydestroy.cmp = icmp eq i8** %arraydestroy.element, %objs_ptr
+ br i1 %arraydestroy.cmp, label %arraydestroy.done, label %arraydestroy.body
+
+arraydestroy.done:
+ %gep1 = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 2
+ br label %arraydestroy.body1
+
+arraydestroy.body1:
+ %arraydestroy.elementPast1 = phi i8** [ %gep1, %arraydestroy.done ], [ %arraydestroy.element1, %arraydestroy.body1 ]
+ %arraydestroy.element1 = getelementptr inbounds i8** %arraydestroy.elementPast1, i64 -1
+ %destroy_tmp1 = load i8** %arraydestroy.element1, align 8
+ call void @objc_release(i8* %destroy_tmp1), !clang.imprecise_release !0
+ %keys_ptr = getelementptr inbounds [2 x i8*]* %keys, i64 0, i64 0
+ %arraydestroy.cmp1 = icmp eq i8** %arraydestroy.element1, %keys_ptr
+ br i1 %arraydestroy.cmp1, label %arraydestroy.done1, label %arraydestroy.body1
+
+arraydestroy.done1:
+ call void @objc_release(i8* %call1), !clang.imprecise_release !0
+ call void @objc_release(i8* %call1), !clang.imprecise_release !0
+ ret void
+}
+
!0 = metadata !{}
declare i32 @__gxx_personality_v0(...)
More information about the llvm-commits
mailing list