[llvm] 32dc79c - [ObjC][ARC] Do not perform code motion on precise release calls
Akira Hatanaka via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 15 17:40:03 PST 2021
Author: Akira Hatanaka
Date: 2021-02-15T17:39:37-08:00
New Revision: 32dc79c5efedf06e67daab75249a28e1bc5b889d
URL: https://github.com/llvm/llvm-project/commit/32dc79c5efedf06e67daab75249a28e1bc5b889d
DIFF: https://github.com/llvm/llvm-project/commit/32dc79c5efedf06e67daab75249a28e1bc5b889d.diff
LOG: [ObjC][ARC] Do not perform code motion on precise release calls
This fixes a bug where an object can get deallocated before reaching the
end of its full formal lifetime.
rdar://72110887
rdar://74123176
Added:
Modified:
llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
llvm/lib/Transforms/ObjCARC/PtrState.cpp
llvm/lib/Transforms/ObjCARC/PtrState.h
llvm/test/Transforms/ObjCARC/basic.ll
llvm/test/Transforms/ObjCARC/code-motion.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
index 091ea143d6a2..0769d466be93 100644
--- a/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
+++ b/llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
@@ -1229,7 +1229,6 @@ static void CheckForUseCFGHazard(const Sequence SuccSSeq,
SomeSuccHasSame = true;
break;
case S_Stop:
- case S_Release:
case S_MovableRelease:
if (!S.IsKnownSafe() && !SuccSRRIKnownSafe)
AllSuccsHaveSame = false;
@@ -1257,7 +1256,6 @@ static void CheckForCanReleaseCFGHazard(const Sequence SuccSSeq,
SomeSuccHasSame = true;
break;
case S_Stop:
- case S_Release:
case S_MovableRelease:
case S_Use:
if (!S.IsKnownSafe() && !SuccSRRIKnownSafe)
@@ -1343,7 +1341,6 @@ ObjCARCOpt::CheckForCFGHazards(const BasicBlock *BB,
case S_Retain:
case S_None:
case S_Stop:
- case S_Release:
case S_MovableRelease:
break;
}
diff --git a/llvm/lib/Transforms/ObjCARC/PtrState.cpp b/llvm/lib/Transforms/ObjCARC/PtrState.cpp
index 89937b201400..d10d5851d5ea 100644
--- a/llvm/lib/Transforms/ObjCARC/PtrState.cpp
+++ b/llvm/lib/Transforms/ObjCARC/PtrState.cpp
@@ -44,8 +44,6 @@ raw_ostream &llvm::objcarc::operator<<(raw_ostream &OS, const Sequence S) {
return OS << "S_CanRelease";
case S_Use:
return OS << "S_Use";
- case S_Release:
- return OS << "S_Release";
case S_MovableRelease:
return OS << "S_MovableRelease";
case S_Stop:
@@ -75,12 +73,10 @@ static Sequence MergeSeqs(Sequence A, Sequence B, bool TopDown) {
} else {
// Choose the side which is further along in the sequence.
if ((A == S_Use || A == S_CanRelease) &&
- (B == S_Use || B == S_Release || B == S_Stop || B == S_MovableRelease))
+ (B == S_Use || B == S_Stop || B == S_MovableRelease))
return A;
// If both sides are releases, choose the more conservative one.
- if (A == S_Stop && (B == S_Release || B == S_MovableRelease))
- return A;
- if (A == S_Release && B == S_MovableRelease)
+ if (A == S_Stop && B == S_MovableRelease)
return A;
}
@@ -184,7 +180,7 @@ bool BottomUpPtrState::InitBottomUp(ARCMDKindCache &Cache, Instruction *I) {
// pairs by making PtrState hold a stack of states, but this is
// simple and avoids adding overhead for the non-nested case.
bool NestingDetected = false;
- if (GetSeq() == S_Release || GetSeq() == S_MovableRelease) {
+ if (GetSeq() == S_MovableRelease) {
LLVM_DEBUG(
dbgs() << " Found nested releases (i.e. a release pair)\n");
NestingDetected = true;
@@ -192,8 +188,10 @@ bool BottomUpPtrState::InitBottomUp(ARCMDKindCache &Cache, Instruction *I) {
MDNode *ReleaseMetadata =
I->getMetadata(Cache.get(ARCMDKindID::ImpreciseRelease));
- Sequence NewSeq = ReleaseMetadata ? S_MovableRelease : S_Release;
+ Sequence NewSeq = ReleaseMetadata ? S_MovableRelease : S_Stop;
ResetSequenceProgress(NewSeq);
+ if (NewSeq == S_Stop)
+ InsertReverseInsertPt(I);
SetReleaseMetadata(ReleaseMetadata);
SetKnownSafe(HasKnownPositiveRefCount());
SetTailCallRelease(cast<CallInst>(I)->isTailCall());
@@ -208,7 +206,6 @@ bool BottomUpPtrState::MatchWithRetain() {
Sequence OldSeq = GetSeq();
switch (OldSeq) {
case S_Stop:
- case S_Release:
case S_MovableRelease:
case S_Use:
// If OldSeq is not S_Use or OldSeq is S_Use and we are tracking an
@@ -243,7 +240,6 @@ bool BottomUpPtrState::HandlePotentialAlterRefCount(Instruction *Inst,
SetSeq(S_CanRelease);
return true;
case S_CanRelease:
- case S_Release:
case S_MovableRelease:
case S_Stop:
case S_None:
@@ -292,17 +288,11 @@ void BottomUpPtrState::HandlePotentialUse(BasicBlock *BB, Instruction *Inst,
// Check for possible direct uses.
switch (GetSeq()) {
- case S_Release:
case S_MovableRelease:
if (CanUse(Inst, Ptr, PA, Class)) {
LLVM_DEBUG(dbgs() << " CanUse: Seq: " << GetSeq() << "; "
<< *Ptr << "\n");
SetSeqAndInsertReverseInsertPt(S_Use);
- } else if (Seq == S_Release && IsUser(Class)) {
- LLVM_DEBUG(dbgs() << " PreciseReleaseUse: Seq: " << GetSeq()
- << "; " << *Ptr << "\n");
- // Non-movable releases depend on any possible objc pointer use.
- SetSeqAndInsertReverseInsertPt(S_Stop);
} else if (const auto *Call = getreturnRVOperand(*Inst, Class)) {
if (CanUse(Call, Ptr, PA, GetBasicARCInstKind(Call))) {
LLVM_DEBUG(dbgs() << " ReleaseUse: Seq: " << GetSeq() << "; "
@@ -378,7 +368,6 @@ bool TopDownPtrState::MatchWithRelease(ARCMDKindCache &Cache,
case S_None:
return false;
case S_Stop:
- case S_Release:
case S_MovableRelease:
llvm_unreachable("top-down pointer in bottom up state!");
}
@@ -418,7 +407,6 @@ bool TopDownPtrState::HandlePotentialAlterRefCount(
case S_None:
return false;
case S_Stop:
- case S_Release:
case S_MovableRelease:
llvm_unreachable("top-down pointer in release state!");
}
@@ -442,7 +430,6 @@ void TopDownPtrState::HandlePotentialUse(Instruction *Inst, const Value *Ptr,
case S_None:
return;
case S_Stop:
- case S_Release:
case S_MovableRelease:
llvm_unreachable("top-down pointer in release state!");
}
diff --git a/llvm/lib/Transforms/ObjCARC/PtrState.h b/llvm/lib/Transforms/ObjCARC/PtrState.h
index f143120ad6a4..232db2bd33bc 100644
--- a/llvm/lib/Transforms/ObjCARC/PtrState.h
+++ b/llvm/lib/Transforms/ObjCARC/PtrState.h
@@ -43,8 +43,7 @@ enum Sequence {
S_Retain, ///< objc_retain(x).
S_CanRelease, ///< foo(x) -- x could possibly see a ref count decrement.
S_Use, ///< any use of x.
- S_Stop, ///< like S_Release, but code motion is stopped.
- S_Release, ///< objc_release(x).
+ S_Stop, ///< code motion is stopped.
S_MovableRelease ///< objc_release(x), !clang.imprecise_release.
};
diff --git a/llvm/test/Transforms/ObjCARC/basic.ll b/llvm/test/Transforms/ObjCARC/basic.ll
index 9617eacd59f2..d521ea1de218 100644
--- a/llvm/test/Transforms/ObjCARC/basic.ll
+++ b/llvm/test/Transforms/ObjCARC/basic.ll
@@ -723,14 +723,15 @@ return:
; CHECK-LABEL: define void @test8c(
; CHECK: entry:
; CHECK: t:
-; CHECK: @llvm.objc.retain
+; CHECK-NOT: @llvm.objc.
; CHECK: f:
-; CHECK: @llvm.objc.retain
+; CHECK-NOT: @llvm.objc.
; CHECK: mid:
; CHECK: u:
+; CHECK: @llvm.objc.retain
; CHECK: @llvm.objc.release
; CHECK: g:
-; CHECK: @llvm.objc.release
+; CHECK-NOT: @llvm.objc.
; CHECK: return:
; CHECK: }
define void @test8c(i32* %x, i1 %p, i1 %q) nounwind {
@@ -1549,17 +1550,14 @@ done:
; Like test28. but with two releases.
; CHECK-LABEL: define void @test29(
-; CHECK-NOT: @llvm.objc.
-; CHECK: true:
; CHECK: call i8* @llvm.objc.retain
+; CHECK: true:
; CHECK: call void @callee()
; CHECK: store
-; CHECK: call void @llvm.objc.release
-; CHECK-NOT: @llvm.objc.release
; CHECK: done:
-; CHECK-NOT: @llvm.objc.
+; CHECK: call void @llvm.objc.release
; CHECK: ohno:
-; CHECK-NOT: @llvm.objc.
+; CHECK: call void @llvm.objc.release
; CHECK: }
define void @test29(i8* %p, i1 %x, i1 %y) {
entry:
@@ -1584,19 +1582,15 @@ ohno:
; with an extra release.
; CHECK-LABEL: define void @test30(
-; CHECK-NOT: @llvm.objc.
-; CHECK: true:
; CHECK: call i8* @llvm.objc.retain
+; CHECK: true:
; CHECK: call void @callee()
; CHECK: store
-; CHECK: call void @llvm.objc.release
-; CHECK-NOT: @llvm.objc.release
; CHECK: false:
-; CHECK-NOT: @llvm.objc.
; CHECK: done:
-; CHECK-NOT: @llvm.objc.
+; CHECK: call void @llvm.objc.release
; CHECK: ohno:
-; CHECK-NOT: @llvm.objc.
+; CHECK: call void @llvm.objc.release
; CHECK: }
define void @test30(i8* %p, i1 %x, i1 %y, i1 %z) {
entry:
@@ -1626,14 +1620,11 @@ ohno:
; CHECK: call i8* @llvm.objc.retain(i8* %p)
; CHECK: call void @callee()
; CHECK: store
-; CHECK: call void @llvm.objc.release
-; CHECK-NOT: @llvm.objc.release
; CHECK: true:
-; CHECK-NOT: @llvm.objc.release
+; CHECK: call void @llvm.objc.release
; CHECK: false:
-; CHECK-NOT: @llvm.objc.release
+; CHECK: call void @llvm.objc.release
; CHECK: ret void
-; CHECK-NOT: @llvm.objc.release
; CHECK: }
define void @test31(i8* %p, i1 %x) {
entry:
@@ -1652,14 +1643,12 @@ false:
; Don't consider bitcasts or getelementptrs direct uses.
; CHECK-LABEL: define void @test32(
-; CHECK-NOT: @llvm.objc.
-; CHECK: true:
; CHECK: call i8* @llvm.objc.retain
+; CHECK: true:
; CHECK: call void @callee()
; CHECK: store
-; CHECK: call void @llvm.objc.release
; CHECK: done:
-; CHECK-NOT: @llvm.objc.
+; CHECK: call void @llvm.objc.release
; CHECK: }
define void @test32(i8* %p, i1 %x) {
entry:
@@ -1681,14 +1670,12 @@ done:
; Do consider icmps to be direct uses.
; CHECK-LABEL: define void @test33(
-; CHECK-NOT: @llvm.objc.
-; CHECK: true:
; CHECK: call i8* @llvm.objc.retain
+; CHECK: true:
; CHECK: call void @callee()
; CHECK: icmp
-; CHECK: call void @llvm.objc.release
; CHECK: done:
-; CHECK-NOT: @llvm.objc.
+; CHECK: call void @llvm.objc.release
; CHECK: }
define void @test33(i8* %p, i1 %x, i8* %y) {
entry:
diff --git a/llvm/test/Transforms/ObjCARC/code-motion.ll b/llvm/test/Transforms/ObjCARC/code-motion.ll
index 7c3242ce857f..7f254e698046 100644
--- a/llvm/test/Transforms/ObjCARC/code-motion.ll
+++ b/llvm/test/Transforms/ObjCARC/code-motion.ll
@@ -3,6 +3,8 @@
declare void @alterRefCount()
declare void @use(i8*)
+ at g0 = global i8* null, align 8
+
; Check that ARC optimizer doesn't reverse the order of the retain call and the
; release call when there are debug instructions.
@@ -20,6 +22,22 @@ define i32 @test(i8* %x, i8* %y, i8 %z, i32 %i) {
ret i32 %i
}
+; ARC optimizer shouldn't move the release call, which is a precise release call
+; past the call to @alterRefCount.
+
+; CHECK-LABEL: define void @test2(
+; CHECK: call void @alterRefCount(
+; CHECK: call void @llvm.objc.release(
+
+define void @test2() {
+ %v0 = load i8*, i8** @g0, align 8
+ %v1 = tail call i8* @llvm.objc.retain(i8* %v0)
+ tail call void @use(i8* %v0)
+ tail call void @alterRefCount()
+ tail call void @llvm.objc.release(i8* %v0)
+ ret void
+}
+
declare void @llvm.dbg.declare(metadata, metadata, metadata)
declare i8* @llvm.objc.retain(i8*) local_unnamed_addr
declare void @llvm.objc.release(i8*) local_unnamed_addr
More information about the llvm-commits
mailing list