Skip to content

Commit 319e9e0

Browse files
meg-guptameg-gupta
authored
Merge pull request swiftlang#35840 from meg-gupta/fixrpe
Fix redundant phi elimination for OSSA
2 parents e148032 + b9ac9a8 commit 319e9e0

File tree

2 files changed

+346
-4
lines changed

2 files changed

+346
-4
lines changed

‎lib/SILOptimizer/Transforms/PhiArgumentOptimizations.cpp

Lines changed: 102 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,52 @@ void RedundantPhiEliminationPass::run() {
9393
}
9494
}
9595

96+
#ifndef NDEBUG
97+
static bool hasOnlyNoneOwnershipIncomingValues(SILPhiArgument *phi) {
98+
SmallSetVector<SILPhiArgument *, 4> worklist;
99+
SmallVector<SILValue, 4> incomingValues;
100+
worklist.insert(phi);
101+
102+
// Size of the worklist changes in this loop
103+
for (unsigned idx = 0; idx < worklist.size(); idx++) {
104+
phi->getIncomingPhiValues(incomingValues);
105+
for (auto incomingValue : incomingValues) {
106+
if (incomingValue.getOwnershipKind() == OwnershipKind::None)
107+
continue;
108+
if (auto *incomingPhi = dyn_cast<SILPhiArgument>(incomingValue)) {
109+
if (incomingPhi->isPhiArgument()) {
110+
worklist.insert(incomingPhi);
111+
continue;
112+
}
113+
}
114+
return false;
115+
}
116+
incomingValues.clear();
117+
}
118+
return true;
119+
}
120+
#endif
121+
122+
// FIXME: Replace with generic ownership rauw for phi args when it is
123+
// upstreamed.
124+
static void eraseOwnedPhiArgument(SILBasicBlock *block, unsigned argIdx) {
125+
auto *phi = cast<SILPhiArgument>(block->getArgument(argIdx));
126+
assert(phi->getOwnershipKind() == OwnershipKind::Owned);
127+
128+
auto visitor = [&](Operand *op) {
129+
if (op->isLifetimeEnding()) {
130+
// Insert a destroy
131+
SILBuilderWithScope builder(op->getUser());
132+
builder.createDestroyValue(RegularLocation::getAutoGeneratedLocation(),
133+
op->get());
134+
}
135+
return true;
136+
};
137+
phi->visitIncomingPhiOperands(visitor);
138+
139+
erasePhiArgument(block, argIdx);
140+
}
141+
96142
bool RedundantPhiEliminationPass::optimizeArgs(SILBasicBlock *block) {
97143
// Avoid running into quadratic behavior for blocks which have many arguments.
98144
// This is seldom, anyway.
@@ -111,8 +157,49 @@ bool RedundantPhiEliminationPass::optimizeArgs(SILBasicBlock *block) {
111157
continue;
112158

113159
if (valuesAreEqual(arg1, arg2)) {
114-
arg2->replaceAllUsesWith(arg1);
115-
erasePhiArgument(block, arg2Idx);
160+
if (block->getParent()->hasOwnership()) {
161+
auto *phi1 = cast<SILPhiArgument>(arg1);
162+
auto *phi2 = cast<SILPhiArgument>(arg2);
163+
// @owned phi args can only be equal if all the incoming values had a
164+
// None ownership. To replace, create a copy_value of the duplicate
165+
// arg.
166+
if (phi1->getOwnershipKind() == OwnershipKind::Owned &&
167+
phi2->getOwnershipKind() == OwnershipKind::Owned) {
168+
#ifndef NDEBUG
169+
assert(hasOnlyNoneOwnershipIncomingValues(phi1));
170+
assert(hasOnlyNoneOwnershipIncomingValues(phi2));
171+
#endif
172+
SILBuilderWithScope builder(&block->front());
173+
auto copy = builder.createCopyValue(
174+
RegularLocation::getAutoGeneratedLocation(), phi1);
175+
phi2->replaceAllUsesWith(copy);
176+
eraseOwnedPhiArgument(block, arg2Idx);
177+
}
178+
// If arg2 has none ownership, replace arg1 with arg2
179+
else if (phi1->getOwnershipKind() == OwnershipKind::Owned &&
180+
phi2->getOwnershipKind() == OwnershipKind::None) {
181+
#ifndef NDEBUG
182+
assert(hasOnlyNoneOwnershipIncomingValues(phi1));
183+
#endif
184+
phi1->replaceAllUsesWith(phi2);
185+
eraseOwnedPhiArgument(block, arg1Idx);
186+
}
187+
// If arg1 has none ownership, replace arg2 with arg1
188+
else if (phi1->getOwnershipKind() == OwnershipKind::None &&
189+
phi2->getOwnershipKind() == OwnershipKind::Owned) {
190+
#ifndef NDEBUG
191+
assert(hasOnlyNoneOwnershipIncomingValues(phi2));
192+
#endif
193+
phi2->replaceAllUsesWith(phi1);
194+
eraseOwnedPhiArgument(block, arg2Idx);
195+
} else {
196+
phi2->replaceAllUsesWith(phi1);
197+
erasePhiArgument(block, arg2Idx);
198+
}
199+
} else {
200+
arg2->replaceAllUsesWith(arg1);
201+
erasePhiArgument(block, arg2Idx);
202+
}
116203
changed = true;
117204
} else {
118205
++arg2Idx;
@@ -149,15 +236,26 @@ bool RedundantPhiEliminationPass::valuesAreEqual(SILValue val1, SILValue val2) {
149236

150237
if (val1->getKind() != val2->getKind())
151238
return false;
152-
239+
153240
if (auto *arg1 = dyn_cast<SILPhiArgument>(val1)) {
154241
auto *arg2 = cast<SILPhiArgument>(val2);
155242
SILBasicBlock *argBlock = arg1->getParent();
156243
if (argBlock != arg2->getParent())
157244
return false;
158245
if (arg1->getType() != arg2->getType())
159246
return false;
160-
247+
248+
// Don't optimize if we have a phi arg with @guaranteed ownership.
249+
// Such a phi arg will be redundant only if all the incoming values have
250+
// none ownership.
251+
// In that case, we maybe able to eliminate the @guaranteed phi arg, by
252+
// creating a new borrow scope for the redundant @guaranteed phi arg, and
253+
// re-writing all the consuming uses in a way the new borrow scope is
254+
// within the borrow scope of its operand. This is not handled currently.
255+
if (arg1->getOwnershipKind() == OwnershipKind::Guaranteed ||
256+
arg2->getOwnershipKind() == OwnershipKind::Guaranteed) {
257+
return false;
258+
}
161259
// All incoming phi values must be equal.
162260
for (SILBasicBlock *pred : argBlock->getPredecessorBlocks()) {
163261
SILValue incoming1 = arg1->getIncomingPhiValue(pred);

‎test/SILOptimizer/redundant_phi_elimination_ossa.sil

Lines changed: 244 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,244 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -redundant-phi-elimination | %FileCheck %s
2+
3+
sil_stage canonical
4+
5+
class Klass {
6+
}
7+
8+
struct NonTrivialStruct {
9+
var val:Klass
10+
}
11+
12+
public enum FakeOptional<T> {
13+
case none
14+
case some(T)
15+
}
16+
17+
// CHECK-LABEL: sil [ossa] @test_redundantownedphiarg1 :
18+
// CHECK: bb3([[ARG1:%.*]] : @owned $FakeOptional<Klass>):
19+
// CHECK: [[COPY:%.*]] = copy_value [[ARG1]]
20+
// CHECK-LABEL: } // end sil function 'test_redundantownedphiarg1'
21+
sil [ossa] @test_redundantownedphiarg1 : $@convention(thin) () -> () {
22+
bb0:
23+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
24+
cond_br undef, bb1, bb2
25+
26+
bb1:
27+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
28+
29+
bb2:
30+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
31+
32+
bb3(%1 : @owned $FakeOptional<Klass>, %2 : @owned $FakeOptional<Klass>):
33+
destroy_value %1 : $FakeOptional<Klass>
34+
destroy_value %2 : $FakeOptional<Klass>
35+
%res = tuple ()
36+
return %res : $()
37+
}
38+
39+
// CHECK-LABEL: sil [ossa] @test_redundantownedphiarg2 :
40+
// CHECK: bb3([[ARG1:%.*]] : $FakeOptional<Klass>):
41+
// CHECK-NOT: copy_value
42+
// CHECK-LABEL: } // end sil function 'test_redundantownedphiarg2'
43+
sil [ossa] @test_redundantownedphiarg2 : $@convention(thin) () -> () {
44+
bb0:
45+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
46+
cond_br undef, bb1, bb2
47+
48+
bb1:
49+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
50+
51+
bb2:
52+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
53+
54+
bb3(%1 : @owned $FakeOptional<Klass>, %2 : $FakeOptional<Klass>):
55+
destroy_value %1 : $FakeOptional<Klass>
56+
destroy_value %2 : $FakeOptional<Klass>
57+
%res = tuple ()
58+
return %res : $()
59+
}
60+
61+
// CHECK-LABEL: sil [ossa] @test_redundantownedphiarg3 :
62+
// CHECK: bb3([[ARG1:%.*]] : $FakeOptional<Klass>):
63+
// CHECK-NOT: copy_value
64+
// CHECK-LABEL: } // end sil function 'test_redundantownedphiarg3'
65+
sil [ossa] @test_redundantownedphiarg3 : $@convention(thin) () -> () {
66+
bb0:
67+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
68+
cond_br undef, bb1, bb2
69+
70+
bb1:
71+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
72+
73+
bb2:
74+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
75+
76+
bb3(%1 : @owned $FakeOptional<Klass>, %2 : $FakeOptional<Klass>):
77+
destroy_value %1 : $FakeOptional<Klass>
78+
cond_br undef, bb4, bb5
79+
80+
bb4:
81+
destroy_value %2 : $FakeOptional<Klass>
82+
br bb6
83+
84+
bb5:
85+
br bb6
86+
87+
bb6:
88+
%res = tuple ()
89+
return %res : $()
90+
}
91+
92+
// CHECK-LABEL: sil [ossa] @test_redundantownedphiarg4 :
93+
// CHECK: bb3([[ARG1:%.*]] : $FakeOptional<Klass>):
94+
// CHECK-NOT: copy_value
95+
// CHECK-LABEL: } // end sil function 'test_redundantownedphiarg4'
96+
sil [ossa] @test_redundantownedphiarg4 : $@convention(thin) () -> () {
97+
bb0:
98+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
99+
cond_br undef, bb1, bb2
100+
101+
bb1:
102+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
103+
104+
bb2:
105+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
106+
107+
bb3(%1 : $FakeOptional<Klass>, %2 : @owned $FakeOptional<Klass>):
108+
destroy_value %1 : $FakeOptional<Klass>
109+
destroy_value %2 : $FakeOptional<Klass>
110+
%res = tuple ()
111+
return %res : $()
112+
}
113+
114+
// CHECK-LABEL: sil [ossa] @test_redundantownedphiarg5 :
115+
// CHECK: bb1([[ARG1:%.*]] : @owned $FakeOptional<Klass>):
116+
// CHECK: [[COPY1:%.*]] = copy_value [[ARG1]]
117+
// CHECK: bb3([[ARG2:%.*]] : @owned $FakeOptional<Klass>):
118+
// CHECK: [[COPY2:%.*]] = copy_value [[ARG2]]
119+
// CHECK-LABEL: } // end sil function 'test_redundantownedphiarg5'
120+
sil [ossa] @test_redundantownedphiarg5 : $@convention(thin) () -> () {
121+
bb0:
122+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
123+
cond_br undef, bb2, bb4
124+
125+
bb1(%3 : @owned $FakeOptional<Klass>, %4 : @owned $FakeOptional<Klass>):
126+
destroy_value %3 : $FakeOptional<Klass>
127+
destroy_value %4 : $FakeOptional<Klass>
128+
%res = tuple ()
129+
return %res : $()
130+
131+
bb2:
132+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
133+
134+
bb3(%1 : @owned $FakeOptional<Klass>, %2 : @owned $FakeOptional<Klass>):
135+
br bb1(%1 : $FakeOptional<Klass>, %2 : $FakeOptional<Klass>)
136+
137+
bb4:
138+
br bb1(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
139+
}
140+
141+
142+
// Redundant phi args in bb4 don't get optimized away, because a compensating copy_value is added
143+
// while optimizing the redundant phi args in bb3.
144+
// CHECK-LABEL: sil [ossa] @test_redundantownedphiarg6 :
145+
// CHECK: bb3([[ARG1:%.*]] : @owned $FakeOptional<Klass>):
146+
// CHECK: [[COPY1:%.*]] = copy_value [[ARG1]]
147+
// CHECK: bb4([[ARG2:%.*]] : @owned $FakeOptional<Klass>, [[ARG3:%.*]] : @owned $FakeOptional<Klass>):
148+
// CHECK-NOT: [[COPY2:%.*]] = copy_value [[ARG1]]
149+
// CHECK-LABEL: } // end sil function 'test_redundantownedphiarg6'
150+
sil [ossa] @test_redundantownedphiarg6 : $@convention(thin) () -> () {
151+
bb0:
152+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
153+
cond_br undef, bb1, bb2
154+
155+
bb1:
156+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
157+
158+
bb2:
159+
br bb4(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
160+
161+
bb3(%1 : @owned $FakeOptional<Klass>, %2 : @owned $FakeOptional<Klass>):
162+
br bb4(%1 : $FakeOptional<Klass>, %2 : $FakeOptional<Klass>)
163+
164+
bb4(%3 : @owned $FakeOptional<Klass>, %4 : @owned $FakeOptional<Klass>):
165+
destroy_value %3 : $FakeOptional<Klass>
166+
destroy_value %4 : $FakeOptional<Klass>
167+
%res = tuple ()
168+
return %res : $()
169+
}
170+
171+
// CHECK-LABEL: sil [ossa] @test_redundantguaranteedphiarg1 :
172+
// CHECK: bb3([[ARG1:%.*]] : @guaranteed $FakeOptional<Klass>, [[ARG2:%.*]] : @guaranteed $FakeOptional<Klass>):
173+
// CHECK-LABEL: } // end sil function 'test_redundantguaranteedphiarg1'
174+
sil [ossa] @test_redundantguaranteedphiarg1 : $@convention(thin) () -> () {
175+
bb0:
176+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
177+
cond_br undef, bb1, bb2
178+
179+
bb1:
180+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
181+
182+
bb2:
183+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
184+
185+
bb3(%1 : @guaranteed $FakeOptional<Klass>, %2 : @guaranteed $FakeOptional<Klass>):
186+
end_borrow %1 : $FakeOptional<Klass>
187+
end_borrow %2 : $FakeOptional<Klass>
188+
%res = tuple ()
189+
return %res : $()
190+
}
191+
192+
// CHECK-LABEL: sil [ossa] @test_redundantguaranteedphiarg2 :
193+
// CHECK: bb3([[ARG1:%.*]] : @owned $FakeOptional<Klass>, [[ARG2:%.*]] : @guaranteed $FakeOptional<Klass>):
194+
// CHECK-LABEL: } // end sil function 'test_redundantguaranteedphiarg2'
195+
sil [ossa] @test_redundantguaranteedphiarg2 : $@convention(thin) () -> () {
196+
bb0:
197+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
198+
cond_br undef, bb1, bb2
199+
200+
bb1:
201+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
202+
203+
bb2:
204+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
205+
206+
bb3(%1 : @owned $FakeOptional<Klass>, %2 : @guaranteed $FakeOptional<Klass>):
207+
destroy_value %1 : $FakeOptional<Klass>
208+
end_borrow %2 : $FakeOptional<Klass>
209+
%res = tuple ()
210+
return %res : $()
211+
}
212+
213+
// CHECK-LABEL: sil [ossa] @test_redundantguaranteedphiarg3 :
214+
// CHECK: bb3([[ARG3:%.*]] : @guaranteed $FakeOptional<Klass>, [[ARG2:%.*]] : @guaranteed $FakeOptional<Klass>):
215+
// CHECK-LABEL: } // end sil function 'test_redundantguaranteedphiarg3'
216+
sil [ossa] @test_redundantguaranteedphiarg3 : $@convention(thin) () -> () {
217+
bb0:
218+
%0 = enum $FakeOptional<Klass>, #FakeOptional.none!enumelt
219+
cond_br undef, bb1, bb2
220+
221+
bb1:
222+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
223+
224+
bb2:
225+
br bb3(%0 : $FakeOptional<Klass>, %0 : $FakeOptional<Klass>)
226+
227+
bb3(%1 : @guaranteed $FakeOptional<Klass>, %2 : @guaranteed $FakeOptional<Klass>):
228+
cond_br undef, bb4, bb5
229+
230+
bb4:
231+
end_borrow %1 : $FakeOptional<Klass>
232+
end_borrow %2 : $FakeOptional<Klass>
233+
br bb6
234+
235+
bb5:
236+
end_borrow %2 : $FakeOptional<Klass>
237+
end_borrow %1 : $FakeOptional<Klass>
238+
br bb6
239+
240+
bb6:
241+
%res = tuple ()
242+
return %res : $()
243+
}
244+

0 commit comments

Comments
 (0)