Skip to content

Commit 730a184

Browse files
authored
Merge pull request #3073 from apple/PR-79292791
PR-79292791
2 parents 6594a47 + f3cd98c commit 730a184

File tree

2 files changed

+220
-16
lines changed

2 files changed

+220
-16
lines changed

llvm/lib/Transforms/ObjCARC/ObjCARCOpts.cpp

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -532,12 +532,15 @@ class ObjCARCOpt {
532532
bool VisitBottomUp(BasicBlock *BB,
533533
DenseMap<const BasicBlock *, BBState> &BBStates,
534534
BlotMapVector<Value *, RRInfo> &Retains);
535-
bool VisitInstructionTopDown(Instruction *Inst,
536-
DenseMap<Value *, RRInfo> &Releases,
537-
BBState &MyStates);
538-
bool VisitTopDown(BasicBlock *BB,
539-
DenseMap<const BasicBlock *, BBState> &BBStates,
540-
DenseMap<Value *, RRInfo> &Releases);
535+
bool VisitInstructionTopDown(
536+
Instruction *Inst, DenseMap<Value *, RRInfo> &Releases, BBState &MyStates,
537+
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
538+
&ReleaseInsertPtToRCIdentityRoots);
539+
bool VisitTopDown(
540+
BasicBlock *BB, DenseMap<const BasicBlock *, BBState> &BBStates,
541+
DenseMap<Value *, RRInfo> &Releases,
542+
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
543+
&ReleaseInsertPtToRCIdentityRoots);
541544
bool Visit(Function &F, DenseMap<const BasicBlock *, BBState> &BBStates,
542545
BlotMapVector<Value *, RRInfo> &Retains,
543546
DenseMap<Value *, RRInfo> &Releases);
@@ -1495,14 +1498,62 @@ bool ObjCARCOpt::VisitBottomUp(BasicBlock *BB,
14951498
return NestingDetected;
14961499
}
14971500

1498-
bool
1499-
ObjCARCOpt::VisitInstructionTopDown(Instruction *Inst,
1500-
DenseMap<Value *, RRInfo> &Releases,
1501-
BBState &MyStates) {
1501+
// Fill ReleaseInsertPtToRCIdentityRoots, which is a map from insertion points
1502+
// to the set of RC identity roots that would be released by the release calls
1503+
// moved to the insertion points.
1504+
static void collectReleaseInsertPts(
1505+
const BlotMapVector<Value *, RRInfo> &Retains,
1506+
DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
1507+
&ReleaseInsertPtToRCIdentityRoots) {
1508+
for (auto &P : Retains) {
1509+
// Retains is a map from an objc_retain call to a RRInfo of the RC identity
1510+
// root of the call. Get the RC identity root of the objc_retain call.
1511+
Instruction *Retain = cast<Instruction>(P.first);
1512+
Value *Root = GetRCIdentityRoot(Retain->getOperand(0));
1513+
// Collect all the insertion points of the objc_release calls that release
1514+
// the RC identity root of the objc_retain call.
1515+
for (const Instruction *InsertPt : P.second.ReverseInsertPts)
1516+
ReleaseInsertPtToRCIdentityRoots[InsertPt].insert(Root);
1517+
}
1518+
}
1519+
1520+
// Get the RC identity roots from an insertion point of an objc_release call.
1521+
// Return nullptr if the passed instruction isn't an insertion point.
1522+
static const SmallPtrSet<const Value *, 2> *
1523+
getRCIdentityRootsFromReleaseInsertPt(
1524+
const Instruction *InsertPt,
1525+
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
1526+
&ReleaseInsertPtToRCIdentityRoots) {
1527+
auto I = ReleaseInsertPtToRCIdentityRoots.find(InsertPt);
1528+
if (I == ReleaseInsertPtToRCIdentityRoots.end())
1529+
return nullptr;
1530+
return &I->second;
1531+
}
1532+
1533+
bool ObjCARCOpt::VisitInstructionTopDown(
1534+
Instruction *Inst, DenseMap<Value *, RRInfo> &Releases, BBState &MyStates,
1535+
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
1536+
&ReleaseInsertPtToRCIdentityRoots) {
15021537
bool NestingDetected = false;
15031538
ARCInstKind Class = GetARCInstKind(Inst);
15041539
const Value *Arg = nullptr;
15051540

1541+
// Make sure a call to objc_retain isn't moved past insertion points of calls
1542+
// to objc_release.
1543+
if (const SmallPtrSet<const Value *, 2> *Roots =
1544+
getRCIdentityRootsFromReleaseInsertPt(
1545+
Inst, ReleaseInsertPtToRCIdentityRoots))
1546+
for (auto *Root : *Roots) {
1547+
TopDownPtrState &S = MyStates.getPtrTopDownState(Root);
1548+
// Disable code motion if the current position is S_Retain to prevent
1549+
// moving the objc_retain call past objc_release calls. If it's
1550+
// S_CanRelease or larger, it's not necessary to disable code motion as
1551+
// the insertion points that prevent the objc_retain call from moving down
1552+
// should have been set already.
1553+
if (S.GetSeq() == S_Retain)
1554+
S.SetCFGHazardAfflicted(true);
1555+
}
1556+
15061557
LLVM_DEBUG(dbgs() << " Class: " << Class << "\n");
15071558

15081559
switch (Class) {
@@ -1565,10 +1616,11 @@ ObjCARCOpt::VisitInstructionTopDown(Instruction *Inst,
15651616
return NestingDetected;
15661617
}
15671618

1568-
bool
1569-
ObjCARCOpt::VisitTopDown(BasicBlock *BB,
1570-
DenseMap<const BasicBlock *, BBState> &BBStates,
1571-
DenseMap<Value *, RRInfo> &Releases) {
1619+
bool ObjCARCOpt::VisitTopDown(
1620+
BasicBlock *BB, DenseMap<const BasicBlock *, BBState> &BBStates,
1621+
DenseMap<Value *, RRInfo> &Releases,
1622+
const DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
1623+
&ReleaseInsertPtToRCIdentityRoots) {
15721624
LLVM_DEBUG(dbgs() << "\n== ObjCARCOpt::VisitTopDown ==\n");
15731625
bool NestingDetected = false;
15741626
BBState &MyStates = BBStates[BB];
@@ -1608,7 +1660,8 @@ ObjCARCOpt::VisitTopDown(BasicBlock *BB,
16081660
for (Instruction &Inst : *BB) {
16091661
LLVM_DEBUG(dbgs() << " Visiting " << Inst << "\n");
16101662

1611-
NestingDetected |= VisitInstructionTopDown(&Inst, Releases, MyStates);
1663+
NestingDetected |= VisitInstructionTopDown(
1664+
&Inst, Releases, MyStates, ReleaseInsertPtToRCIdentityRoots);
16121665

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

1784+
DenseMap<const Instruction *, SmallPtrSet<const Value *, 2>>
1785+
ReleaseInsertPtToRCIdentityRoots;
1786+
collectReleaseInsertPts(Retains, ReleaseInsertPtToRCIdentityRoots);
1787+
17311788
// Use reverse-postorder for top-down.
17321789
bool TopDownNestingDetected = false;
17331790
for (BasicBlock *BB : llvm::reverse(PostOrder)) {
1734-
TopDownNestingDetected |= VisitTopDown(BB, BBStates, Releases);
1791+
TopDownNestingDetected |=
1792+
VisitTopDown(BB, BBStates, Releases, ReleaseInsertPtToRCIdentityRoots);
17351793
if (DisableRetainReleasePairing)
17361794
return false;
17371795
}

llvm/test/Transforms/ObjCARC/code-motion.ll

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
declare void @alterRefCount()
44
declare void @use(i8*)
5+
declare void @readOnlyFunc(i8*, i8*)
56

67
@g0 = global i8* null, align 8
78

@@ -38,10 +39,155 @@ define void @test2() {
3839
ret void
3940
}
4041

42+
; Check that code motion is disabled in @test3 and @test4.
43+
; Previously, ARC optimizer would move the release past the retain.
44+
45+
; if.then:
46+
; call void @readOnlyFunc(i8* %obj, i8* null)
47+
; call void @llvm.objc.release(i8* %obj) #1, !clang.imprecise_release !2
48+
; %1 = add i32 1, 2
49+
; %2 = tail call i8* @llvm.objc.retain(i8* %obj)
50+
;
51+
; Ideally, the retain/release pairs in BB if.then should be removed.
52+
53+
define void @test3(i8* %obj, i1 %cond) {
54+
; CHECK-LABEL: @test3(
55+
; CHECK-NEXT: [[TMP2:%.*]] = tail call i8* @llvm.objc.retain(i8* [[OBJ:%.*]])
56+
; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
57+
; CHECK: if.then:
58+
; CHECK-NEXT: call void @readOnlyFunc(i8* [[OBJ]], i8* null)
59+
; CHECK-NEXT: [[TMP1:%.*]] = add i32 1, 2
60+
; CHECK-NEXT: call void @alterRefCount()
61+
; CHECK-NEXT: br label [[JOIN:%.*]]
62+
; CHECK: if.else:
63+
; CHECK-NEXT: call void @alterRefCount()
64+
; CHECK-NEXT: call void @use(i8* [[OBJ]])
65+
; CHECK-NEXT: br label [[JOIN]]
66+
; CHECK: join:
67+
; CHECK-NEXT: call void @llvm.objc.release(i8* [[OBJ]]) {{.*}}, !clang.imprecise_release !2
68+
; CHECK-NEXT: ret void
69+
;
70+
%v0 = call i8* @llvm.objc.retain(i8* %obj)
71+
br i1 %cond, label %if.then, label %if.else
72+
73+
if.then:
74+
call void @readOnlyFunc(i8* %obj, i8* null) #0
75+
add i32 1, 2
76+
call void @alterRefCount()
77+
br label %join
78+
79+
if.else:
80+
call void @alterRefCount()
81+
call void @use(i8* %obj)
82+
br label %join
83+
84+
join:
85+
call void @llvm.objc.release(i8* %obj), !clang.imprecise_release !9
86+
ret void
87+
}
88+
89+
define void @test4(i8* %obj0, i8* %obj1, i1 %cond) {
90+
; CHECK-LABEL: @test4(
91+
; CHECK-NEXT: [[TMP3:%.*]] = tail call i8* @llvm.objc.retain(i8* [[OBJ0:%.*]])
92+
; CHECK-NEXT: [[TMP2:%.*]] = tail call i8* @llvm.objc.retain(i8* [[OBJ1:%.*]])
93+
; CHECK-NEXT: br i1 [[COND:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
94+
; CHECK: if.then:
95+
; CHECK-NEXT: call void @readOnlyFunc(i8* [[OBJ0]], i8* [[OBJ1]])
96+
; CHECK-NEXT: [[TMP1:%.*]] = add i32 1, 2
97+
; CHECK-NEXT: call void @alterRefCount()
98+
; CHECK-NEXT: br label [[JOIN:%.*]]
99+
; CHECK: if.else:
100+
; CHECK-NEXT: call void @alterRefCount()
101+
; CHECK-NEXT: call void @use(i8* [[OBJ0]])
102+
; CHECK-NEXT: call void @use(i8* [[OBJ1]])
103+
; CHECK-NEXT: br label [[JOIN]]
104+
; CHECK: join:
105+
; CHECK-NEXT: call void @llvm.objc.release(i8* [[OBJ0]]) {{.*}}, !clang.imprecise_release !2
106+
; CHECK-NEXT: call void @llvm.objc.release(i8* [[OBJ1]]) {{.*}}, !clang.imprecise_release !2
107+
; CHECK-NEXT: ret void
108+
;
109+
%v0 = call i8* @llvm.objc.retain(i8* %obj0)
110+
%v1 = call i8* @llvm.objc.retain(i8* %obj1)
111+
br i1 %cond, label %if.then, label %if.else
112+
113+
if.then:
114+
call void @readOnlyFunc(i8* %obj0, i8* %obj1) #0
115+
add i32 1, 2
116+
call void @alterRefCount()
117+
br label %join
118+
119+
if.else:
120+
call void @alterRefCount()
121+
call void @use(i8* %obj0)
122+
call void @use(i8* %obj1)
123+
br label %join
124+
125+
join:
126+
call void @llvm.objc.release(i8* %obj0), !clang.imprecise_release !9
127+
call void @llvm.objc.release(i8* %obj1), !clang.imprecise_release !9
128+
ret void
129+
}
130+
131+
; In this test, insertion points for the retain and release calls that could be
132+
; eliminated are in different blocks (bb1 and if.then).
133+
134+
define void @test5(i8* %obj, i1 %cond0, i1 %cond1) {
135+
; CHECK-LABEL: @test5(
136+
; CHECK-NEXT: [[V0:%.*]] = tail call i8* @llvm.objc.retain(i8* [[OBJ:%.*]])
137+
; CHECK-NEXT: br i1 [[COND0:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
138+
; CHECK: if.then:
139+
; CHECK-NEXT: call void @readOnlyFunc(i8* [[OBJ]], i8* null)
140+
; CHECK-NEXT: br i1 [[COND1:%.*]], label [[IF_THEN2:%.*]], label [[IF_ELSE2:%.*]]
141+
; CHECK: if.then2:
142+
; CHECK-NEXT: br label [[BB1:%.*]]
143+
; CHECK: if.else2:
144+
; CHECK-NEXT: br label [[BB1]]
145+
; CHECK: bb1:
146+
; CHECK-NEXT: [[TMP1:%.*]] = add i32 1, 2
147+
; CHECK-NEXT: call void @alterRefCount()
148+
; CHECK-NEXT: br label [[JOIN:%.*]]
149+
; CHECK: if.else:
150+
; CHECK-NEXT: call void @alterRefCount()
151+
; CHECK-NEXT: call void @use(i8* [[OBJ]])
152+
; CHECK-NEXT: br label [[JOIN]]
153+
; CHECK: join:
154+
; CHECK-NEXT: call void @llvm.objc.release(i8* [[OBJ]])
155+
; CHECK-NEXT: ret void
156+
;
157+
%v0 = call i8* @llvm.objc.retain(i8* %obj)
158+
br i1 %cond0, label %if.then, label %if.else
159+
160+
if.then:
161+
call void @readOnlyFunc(i8* %obj, i8* null) #0
162+
br i1 %cond1, label %if.then2, label %if.else2
163+
164+
if.then2:
165+
br label %bb1
166+
167+
if.else2:
168+
br label %bb1
169+
170+
bb1:
171+
add i32 1, 2
172+
call void @alterRefCount()
173+
br label %join
174+
175+
if.else:
176+
call void @alterRefCount()
177+
call void @use(i8* %obj)
178+
br label %join
179+
180+
join:
181+
call void @llvm.objc.release(i8* %obj), !clang.imprecise_release !9
182+
ret void
183+
}
184+
41185
declare void @llvm.dbg.declare(metadata, metadata, metadata)
42186
declare i8* @llvm.objc.retain(i8*) local_unnamed_addr
43187
declare void @llvm.objc.release(i8*) local_unnamed_addr
44188

189+
attributes #0 = { readonly }
190+
45191
!llvm.module.flags = !{!0, !1}
46192

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

0 commit comments

Comments
 (0)