Skip to content

PR-79292791 #3073

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 7, 2021
Merged

PR-79292791 #3073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a65ecc4
Precommit test cases in https://reviews.llvm.org/D104953
ahatanaka Jul 2, 2021
cc0d15b
Precommit another test for https://reviews.llvm.org/D104953
ahatanaka Jul 5, 2021
f3cd98c
[ObjC][ARC] Prevent moving objc_retain calls past objc_release calls
ahatanaka Jul 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 74 additions & 16 deletions llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -532,12 +532,15 @@ class ObjCARCOpt {
bool VisitBottomUp(BasicBlock *BB,
DenseMap<const BasicBlock *, BBState> &BBStates,
BlotMapVector<Value *, RRInfo> &Retains);
bool VisitInstructionTopDown(Instruction *Inst,
DenseMap<Value *, RRInfo> &Releases,
BBState &MyStates);
bool VisitTopDown(BasicBlock *BB,
DenseMap<const BasicBlock *, BBState> &BBStates,
DenseMap<Value *, RRInfo> &Releases);
bool VisitInstructionTopDown(
Instruction *Inst, DenseMap<Value *, RRInfo> &Releases, BBState &MyStates,
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
&ReleaseInsertPtToRCIdentityRoots);
bool VisitTopDown(
BasicBlock *BB, DenseMap<const BasicBlock *, BBState> &BBStates,
DenseMap<Value *, RRInfo> &Releases,
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
&ReleaseInsertPtToRCIdentityRoots);
bool Visit(Function &F, DenseMap<const BasicBlock *, BBState> &BBStates,
BlotMapVector<Value *, RRInfo> &Retains,
DenseMap<Value *, RRInfo> &Releases);
Expand Down Expand Up @@ -1495,14 +1498,62 @@ bool ObjCARCOpt::VisitBottomUp(BasicBlock *BB,
return NestingDetected;
}

bool
ObjCARCOpt::VisitInstructionTopDown(Instruction *Inst,
DenseMap<Value *, RRInfo> &Releases,
BBState &MyStates) {
// Fill ReleaseInsertPtToRCIdentityRoots, which is a map from insertion points
// to the set of RC identity roots that would be released by the release calls
// moved to the insertion points.
static void collectReleaseInsertPts(
const BlotMapVector<Value *, RRInfo> &Retains,
DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
&ReleaseInsertPtToRCIdentityRoots) {
for (auto &P : Retains) {
// Retains is a map from an objc_retain call to a RRInfo of the RC identity
// root of the call. Get the RC identity root of the objc_retain call.
Instruction *Retain = cast<Instruction>(P.first);
Value *Root = GetRCIdentityRoot(Retain->getOperand(0));
// Collect all the insertion points of the objc_release calls that release
// the RC identity root of the objc_retain call.
for (const Instruction *InsertPt : P.second.ReverseInsertPts)
ReleaseInsertPtToRCIdentityRoots[InsertPt].insert(Root);
}
}

// Get the RC identity roots from an insertion point of an objc_release call.
// Return nullptr if the passed instruction isn't an insertion point.
static const SmallPtrSet<const Value *, 2> *
getRCIdentityRootsFromReleaseInsertPt(
const Instruction *InsertPt,
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
&ReleaseInsertPtToRCIdentityRoots) {
auto I = ReleaseInsertPtToRCIdentityRoots.find(InsertPt);
if (I == ReleaseInsertPtToRCIdentityRoots.end())
return nullptr;
return &I->second;
}

bool ObjCARCOpt::VisitInstructionTopDown(
Instruction *Inst, DenseMap<Value *, RRInfo> &Releases, BBState &MyStates,
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
&ReleaseInsertPtToRCIdentityRoots) {
bool NestingDetected = false;
ARCInstKind Class = GetARCInstKind(Inst);
const Value *Arg = nullptr;

// Make sure a call to objc_retain isn't moved past insertion points of calls
// to objc_release.
if (const SmallPtrSet<const Value *, 2> *Roots =
getRCIdentityRootsFromReleaseInsertPt(
Inst, ReleaseInsertPtToRCIdentityRoots))
for (auto *Root : *Roots) {
TopDownPtrState &S = MyStates.getPtrTopDownState(Root);
// Disable code motion if the current position is S_Retain to prevent
// moving the objc_retain call past objc_release calls. If it's
// S_CanRelease or larger, it's not necessary to disable code motion as
// the insertion points that prevent the objc_retain call from moving down
// should have been set already.
if (S.GetSeq() == S_Retain)
S.SetCFGHazardAfflicted(true);
}

LLVM_DEBUG(dbgs() << " Class: " << Class << "\n");

switch (Class) {
Expand Down Expand Up @@ -1565,10 +1616,11 @@ ObjCARCOpt::VisitInstructionTopDown(Instruction *Inst,
return NestingDetected;
}

bool
ObjCARCOpt::VisitTopDown(BasicBlock *BB,
DenseMap<const BasicBlock *, BBState> &BBStates,
DenseMap<Value *, RRInfo> &Releases) {
bool ObjCARCOpt::VisitTopDown(
BasicBlock *BB, DenseMap<const BasicBlock *, BBState> &BBStates,
DenseMap<Value *, RRInfo> &Releases,
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
&ReleaseInsertPtToRCIdentityRoots) {
LLVM_DEBUG(dbgs() << "\n== ObjCARCOpt::VisitTopDown ==\n");
bool NestingDetected = false;
BBState &MyStates = BBStates[BB];
Expand Down Expand Up @@ -1608,7 +1660,8 @@ ObjCARCOpt::VisitTopDown(BasicBlock *BB,
for (Instruction &Inst : *BB) {
LLVM_DEBUG(dbgs() << " Visiting " << Inst << "\n");

NestingDetected |= VisitInstructionTopDown(&Inst, Releases, MyStates);
NestingDetected |= VisitInstructionTopDown(
&Inst, Releases, MyStates, ReleaseInsertPtToRCIdentityRoots);

// Bail out if the number of pointers being tracked becomes too large so
// that this pass can complete in a reasonable amount of time.
Expand Down Expand Up @@ -1728,10 +1781,15 @@ bool ObjCARCOpt::Visit(Function &F,
return false;
}

DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
ReleaseInsertPtToRCIdentityRoots;
collectReleaseInsertPts(Retains, ReleaseInsertPtToRCIdentityRoots);

// Use reverse-postorder for top-down.
bool TopDownNestingDetected = false;
for (BasicBlock *BB : llvm::reverse(PostOrder)) {
TopDownNestingDetected |= VisitTopDown(BB, BBStates, Releases);
TopDownNestingDetected |=
VisitTopDown(BB, BBStates, Releases, ReleaseInsertPtToRCIdentityRoots);
if (DisableRetainReleasePairing)
return false;
}
Expand Down
146 changes: 146 additions & 0 deletions llvm/test/Transforms/ObjCARC/code-motion.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

declare void @alterRefCount()
declare void @use(i8*)
declare void @readOnlyFunc(i8*, i8*)

@g0 = global i8* null, align 8

Expand Down Expand Up @@ -38,10 +39,155 @@ define void @test2() {
ret void
}

; Check that code motion is disabled in @test3 and @test4.
; Previously, ARC optimizer would move the release past the retain.

; if.then:
; call void @readOnlyFunc(i8* %obj, i8* null)
; call void @llvm.objc.release(i8* %obj) #1, !clang.imprecise_release !2
; %1 = add i32 1, 2
; %2 = tail call i8* @llvm.objc.retain(i8* %obj)
;
; Ideally, the retain/release pairs in BB if.then should be removed.

define void @test3(i8* %obj, i1 %cond) {
; CHECK-LABEL: @test3(
; CHECK-NEXT: [[TMP2:%.*]] = tail call i8* @llvm.objc.retain(i8* [[OBJ:%.*]])
; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
; CHECK-NEXT: call void @readOnlyFunc(i8* [[OBJ]], i8* null)
; CHECK-NEXT: [[TMP1:%.*]] = add i32 1, 2
; CHECK-NEXT: call void @alterRefCount()
; CHECK-NEXT: br label [[JOIN:%.*]]
; CHECK: if.else:
; CHECK-NEXT: call void @alterRefCount()
; CHECK-NEXT: call void @use(i8* [[OBJ]])
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
; CHECK-NEXT: call void @llvm.objc.release(i8* [[OBJ]]) {{.*}}, !clang.imprecise_release !2
; CHECK-NEXT: ret void
;
%v0 = call i8* @llvm.objc.retain(i8* %obj)
br i1 %cond, label %if.then, label %if.else

if.then:
call void @readOnlyFunc(i8* %obj, i8* null) #0
add i32 1, 2
call void @alterRefCount()
br label %join

if.else:
call void @alterRefCount()
call void @use(i8* %obj)
br label %join

join:
call void @llvm.objc.release(i8* %obj), !clang.imprecise_release !9
ret void
}

define void @test4(i8* %obj0, i8* %obj1, i1 %cond) {
; CHECK-LABEL: @test4(
; CHECK-NEXT: [[TMP3:%.*]] = tail call i8* @llvm.objc.retain(i8* [[OBJ0:%.*]])
; CHECK-NEXT: [[TMP2:%.*]] = tail call i8* @llvm.objc.retain(i8* [[OBJ1:%.*]])
; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
; CHECK-NEXT: call void @readOnlyFunc(i8* [[OBJ0]], i8* [[OBJ1]])
; CHECK-NEXT: [[TMP1:%.*]] = add i32 1, 2
; CHECK-NEXT: call void @alterRefCount()
; CHECK-NEXT: br label [[JOIN:%.*]]
; CHECK: if.else:
; CHECK-NEXT: call void @alterRefCount()
; CHECK-NEXT: call void @use(i8* [[OBJ0]])
; CHECK-NEXT: call void @use(i8* [[OBJ1]])
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
; CHECK-NEXT: call void @llvm.objc.release(i8* [[OBJ0]]) {{.*}}, !clang.imprecise_release !2
; CHECK-NEXT: call void @llvm.objc.release(i8* [[OBJ1]]) {{.*}}, !clang.imprecise_release !2
; CHECK-NEXT: ret void
;
%v0 = call i8* @llvm.objc.retain(i8* %obj0)
%v1 = call i8* @llvm.objc.retain(i8* %obj1)
br i1 %cond, label %if.then, label %if.else

if.then:
call void @readOnlyFunc(i8* %obj0, i8* %obj1) #0
add i32 1, 2
call void @alterRefCount()
br label %join

if.else:
call void @alterRefCount()
call void @use(i8* %obj0)
call void @use(i8* %obj1)
br label %join

join:
call void @llvm.objc.release(i8* %obj0), !clang.imprecise_release !9
call void @llvm.objc.release(i8* %obj1), !clang.imprecise_release !9
ret void
}

; In this test, insertion points for the retain and release calls that could be
; eliminated are in different blocks (bb1 and if.then).

define void @test5(i8* %obj, i1 %cond0, i1 %cond1) {
; CHECK-LABEL: @test5(
; CHECK-NEXT: [[V0:%.*]] = tail call i8* @llvm.objc.retain(i8* [[OBJ:%.*]])
; CHECK-NEXT: br i1 [[COND0:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
; CHECK-NEXT: call void @readOnlyFunc(i8* [[OBJ]], i8* null)
; CHECK-NEXT: br i1 [[COND1:%.*]], label [[IF_THEN2:%.*]], label [[IF_ELSE2:%.*]]
; CHECK: if.then2:
; CHECK-NEXT: br label [[BB1:%.*]]
; CHECK: if.else2:
; CHECK-NEXT: br label [[BB1]]
; CHECK: bb1:
; CHECK-NEXT: [[TMP1:%.*]] = add i32 1, 2
; CHECK-NEXT: call void @alterRefCount()
; CHECK-NEXT: br label [[JOIN:%.*]]
; CHECK: if.else:
; CHECK-NEXT: call void @alterRefCount()
; CHECK-NEXT: call void @use(i8* [[OBJ]])
; CHECK-NEXT: br label [[JOIN]]
; CHECK: join:
; CHECK-NEXT: call void @llvm.objc.release(i8* [[OBJ]])
; CHECK-NEXT: ret void
;
%v0 = call i8* @llvm.objc.retain(i8* %obj)
br i1 %cond0, label %if.then, label %if.else

if.then:
call void @readOnlyFunc(i8* %obj, i8* null) #0
br i1 %cond1, label %if.then2, label %if.else2

if.then2:
br label %bb1

if.else2:
br label %bb1

bb1:
add i32 1, 2
call void @alterRefCount()
br label %join

if.else:
call void @alterRefCount()
call void @use(i8* %obj)
br label %join

join:
call void @llvm.objc.release(i8* %obj), !clang.imprecise_release !9
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

attributes #0 = { readonly }

!llvm.module.flags = !{!0, !1}

!0 = !{i32 2, !"Dwarf Version", i32 4}
Expand Down