Skip to content

Commit d7fc12e

Browse files
committed
[AddressLowering] Allow use-projection into phi.
Resolves numerous regressions when enabling AddressLowering early in the pipeline. Previously, if a value incoming into a phi had storage which itself was a use-projection out of some other storage, PhiStorageOptimizer bailed out. The result was unnecessary "moves" (i.e. `copy_addr [take] [init]` instructions). Here, this bailout is removed. In order to do this, it is necessary to find (1) all values whose storage recursively project out of an incoming value (such a value may have storage which is either a use _or_ a def projection) and (2) the block which dominates the defs of all these values. Together, these values are used to compute liveness to determine interference. Previously, the live region was that between the uses of an incoming value and its defining block. Now, it is that between the uses of any of the values found in (1) and the dominating block found in (2).
1 parent 9e66a94 commit d7fc12e

File tree

5 files changed

+629
-72
lines changed

5 files changed

+629
-72
lines changed

lib/SILOptimizer/Mandatory/AddressLowering.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1446,7 +1446,7 @@ void OpaqueStorageAllocation::allocatePhi(PhiValue phi) {
14461446
// The phi operand projections are computed first to give them priority. Then
14471447
// we determine if the phi itself can share storage with one of its users.
14481448
CoalescedPhi coalescedPhi;
1449-
coalescedPhi.coalesce(phi, pass.valueStorageMap);
1449+
coalescedPhi.coalesce(phi, pass.valueStorageMap, pass.domInfo);
14501450

14511451
SmallVector<SILValue, 4> coalescedValues;
14521452
coalescedValues.reserve(coalescedPhi.getCoalescedOperands().size());

lib/SILOptimizer/Mandatory/PhiStorageOptimizer.cpp

Lines changed: 152 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@
8787

8888
#include "PhiStorageOptimizer.h"
8989
#include "swift/SIL/BasicBlockDatastructures.h"
90+
#include "swift/SIL/Dominance.h"
91+
#include "swift/SIL/NodeDatastructures.h"
9092
#include "swift/SIL/SILBasicBlock.h"
9193
#include "swift/SIL/SILInstruction.h"
9294

@@ -101,35 +103,46 @@ namespace swift {
101103
class PhiStorageOptimizer {
102104
PhiValue phi;
103105
const ValueStorageMap &valueStorageMap;
106+
DominanceInfo *domInfo;
104107

105108
CoalescedPhi &coalescedPhi;
106109

107110
BasicBlockSet occupiedBlocks;
108111

109112
public:
110113
PhiStorageOptimizer(PhiValue phi, const ValueStorageMap &valueStorageMap,
111-
CoalescedPhi &coalescedPhi)
112-
: phi(phi), valueStorageMap(valueStorageMap), coalescedPhi(coalescedPhi),
113-
occupiedBlocks(getFunction()) {}
114+
CoalescedPhi &coalescedPhi, DominanceInfo *domInfo)
115+
: phi(phi), valueStorageMap(valueStorageMap), domInfo(domInfo),
116+
coalescedPhi(coalescedPhi), occupiedBlocks(getFunction()) {}
114117

115118
SILFunction *getFunction() const { return phi.phiBlock->getParent(); }
116119

117120
void optimize();
118121

119122
protected:
120-
bool hasUseProjection(SILInstruction *defInst);
121-
bool canCoalesceValue(SILValue incomingVal);
123+
using ProjectedValues = StackList<SILValue>;
124+
bool findUseProjections(SILInstruction *defInst,
125+
SmallVectorImpl<Operand *> &operands);
126+
bool findDefProjections(SILValue value,
127+
SmallVectorImpl<SILValue> *projecteds);
128+
SILBasicBlock *canCoalesceValue(SILValue incomingVal,
129+
ProjectedValues *projectedValues);
122130
void tryCoalesceOperand(SILBasicBlock *incomingPred);
123-
bool recordUseLiveness(SILValue incomingVal, BasicBlockSetVector &liveBlocks);
131+
bool recordUseLiveness(ProjectedValues &incomingVals,
132+
SILBasicBlock *dominatingBlock,
133+
BasicBlockSetVector &liveBlocks);
134+
SILBasicBlock *getDominatingBlock(SILValue incomingVal,
135+
StackList<SILValue> *projectedValues);
124136
};
125137

126138
} // namespace swift
127139

128140
void CoalescedPhi::coalesce(PhiValue phi,
129-
const ValueStorageMap &valueStorageMap) {
141+
const ValueStorageMap &valueStorageMap,
142+
DominanceInfo *domInfo) {
130143
assert(empty() && "attempt to recoalesce the same phi");
131144

132-
PhiStorageOptimizer(phi, valueStorageMap, *this).optimize();
145+
PhiStorageOptimizer(phi, valueStorageMap, *this, domInfo).optimize();
133146
}
134147

135148
/// Optimize phi storage by coalescing phi operands.
@@ -168,7 +181,8 @@ void CoalescedPhi::coalesce(PhiValue phi,
168181
void PhiStorageOptimizer::optimize() {
169182
// The single incoming value case always projects storage.
170183
if (auto *predecessor = phi.phiBlock->getSinglePredecessorBlock()) {
171-
if (canCoalesceValue(phi.getValue()->getIncomingPhiValue(predecessor))) {
184+
if (canCoalesceValue(phi.getValue()->getIncomingPhiValue(predecessor),
185+
/*projectedValues=*/nullptr)) {
172186
// Storage will always be allocated for the phi. The optimization
173187
// attempts to let incoming values reuse the phi's storage. This isn't
174188
// always possible, even in the single incoming value case. For example,
@@ -183,21 +197,47 @@ void PhiStorageOptimizer::optimize() {
183197
}
184198
}
185199

186-
// Return true if any of \p defInst's operands are composing use projections
187-
// into \p defInst's storage.
188-
bool PhiStorageOptimizer::hasUseProjection(SILInstruction *defInst) {
200+
// Return \p defInst's operand which is a composing use projection into \p
201+
// defInst's storage, or nullptr if none exists.
202+
bool PhiStorageOptimizer::findUseProjections(
203+
SILInstruction *defInst, SmallVectorImpl<Operand *> &operands) {
204+
bool found = false;
189205
for (Operand &oper : defInst->getAllOperands()) {
190-
if (valueStorageMap.isComposingUseProjection(&oper))
191-
return true;
206+
if (valueStorageMap.isComposingUseProjection(&oper)) {
207+
found = true;
208+
operands.push_back(&oper);
209+
}
210+
}
211+
return found;
212+
}
213+
214+
bool PhiStorageOptimizer::findDefProjections(
215+
SILValue value, SmallVectorImpl<SILValue> *projecteds) {
216+
bool found = false;
217+
for (auto user : value->getUsers()) {
218+
for (auto result : user->getResults()) {
219+
auto *storage = valueStorageMap.getStorageOrNull(result);
220+
if (!storage)
221+
continue;
222+
if (storage->isDefProjection) {
223+
assert(storage->projectedStorageID ==
224+
valueStorageMap.getOrdinal(value));
225+
projecteds->push_back(result);
226+
found = true;
227+
}
228+
}
192229
}
193-
return false;
230+
return found;
194231
}
195232

196-
// Return true in \p incomingVal can be coalesced with this phi ignoring
197-
// possible interference. Simply determine whether storage reuse is possible.
233+
// Determine whether storage reuse is possible with \p incomingVal. Ignore
234+
// possible interference. If reuse is possible, return the block which
235+
// dominates all defs whose storage is projected out of incomingVal.
198236
//
199237
// Precondition: \p incomingVal is an operand of this phi.
200-
bool PhiStorageOptimizer::canCoalesceValue(SILValue incomingVal) {
238+
SILBasicBlock *
239+
PhiStorageOptimizer::canCoalesceValue(SILValue incomingVal,
240+
ProjectedValues *projectedValues) {
201241
// A Phi must not project from storage that was initialized on a path that
202242
// reaches the phi because other uses of the storage may interfere with the
203243
// phi. A phi may, however, be a composing use projection.
@@ -215,31 +255,20 @@ bool PhiStorageOptimizer::canCoalesceValue(SILValue incomingVal) {
215255
// could be handled, but would require by recursively following uses across
216256
// projections when computing liveness.
217257
if (!incomingStorage.isAllocated() || incomingStorage.isProjection())
218-
return false;
258+
return nullptr;
219259

220-
auto *defInst = incomingVal->getDefiningInstruction();
221-
if (!defInst) {
222-
// Indirect function arguments were replaced by loads.
223-
assert(!isa<SILFunctionArgument>(incomingVal));
260+
if (PhiValue(incomingVal)) {
224261
// Do not coalesce a phi with other phis. This would require liveness
225262
// analysis of the whole phi web before coalescing phi operands.
226-
return false;
263+
return nullptr;
227264
}
228265

229266
// Don't coalesce an incoming value unless it's storage is from a stack
230267
// allocation, which can be replaced with another alloc_stack.
231268
if (!isa<AllocStackInst>(incomingStorage.storageAddress))
232-
return false;
269+
return nullptr;
233270

234-
// Make sure that the incomingVal is not coalesced with any of its operands.
235-
//
236-
// Handling incomingValues whose operands project into them would require by
237-
// recursively finding the set of value definitions and their dominating defBB
238-
// instead of simply incomingVal->getParentBlock().
239-
if (hasUseProjection(defInst))
240-
return false;
241-
242-
return true;
271+
return getDominatingBlock(incomingVal, projectedValues);
243272
}
244273

245274
// Process a single incoming phi operand. Compute the value's liveness while
@@ -248,11 +277,13 @@ void PhiStorageOptimizer::tryCoalesceOperand(SILBasicBlock *incomingPred) {
248277
Operand *incomingOper = phi.getOperand(incomingPred);
249278
SILValue incomingVal = incomingOper->get();
250279

251-
if (!canCoalesceValue(incomingVal))
280+
ProjectedValues projectedValues(incomingPred->getFunction());
281+
auto *dominatingBlock = canCoalesceValue(incomingVal, &projectedValues);
282+
if (!dominatingBlock)
252283
return;
253284

254285
BasicBlockSetVector liveBlocks(getFunction());
255-
if (!recordUseLiveness(incomingVal, liveBlocks))
286+
if (!recordUseLiveness(projectedValues, dominatingBlock, liveBlocks))
256287
return;
257288

258289
for (auto *block : liveBlocks) {
@@ -262,39 +293,100 @@ void PhiStorageOptimizer::tryCoalesceOperand(SILBasicBlock *incomingPred) {
262293
coalescedPhi.coalescedOperands.push_back(incomingOper);
263294
}
264295

265-
// Record liveness generated by uses of \p incomingVal.
296+
/// The block which dominates all the defs whose storage is projected out of the
297+
/// storage for \p incomingVal.
298+
///
299+
/// Populates \p projectedValues with the values whose storage is recursively
300+
/// projected out of the storage for incomingVal.
301+
SILBasicBlock *
302+
PhiStorageOptimizer::getDominatingBlock(SILValue incomingVal,
303+
StackList<SILValue> *projectedValues) {
304+
assert(domInfo);
305+
306+
// Recursively find the set of value definitions and the dominating LCA.
307+
ValueWorklist values(incomingVal->getFunction());
308+
309+
values.push(incomingVal);
310+
311+
SILBasicBlock *lca = incomingVal->getParentBlock();
312+
auto updateLCA = [&](SILBasicBlock *other) {
313+
lca = domInfo->findNearestCommonDominator(lca, other);
314+
};
315+
316+
while (auto value = values.pop()) {
317+
if (projectedValues)
318+
projectedValues->push_back(value);
319+
auto *defInst = value->getDefiningInstruction();
320+
if (!defInst) {
321+
assert(!PhiValue(value));
322+
updateLCA(value->getParentBlock());
323+
continue;
324+
}
325+
SmallVector<Operand *, 2> operands;
326+
if (findUseProjections(defInst, operands)) {
327+
// Any operand whose storage is a use-projection out of `value`'s storage
328+
// dominates `value` (i.e. `defInst`), so skip updating the LCA here.
329+
for (auto *operand : operands) {
330+
values.pushIfNotVisited(operand->get());
331+
}
332+
continue;
333+
}
334+
SmallVector<SILValue, 2> projecteds;
335+
if (findDefProjections(value, &projecteds)) {
336+
// Walking up the projection hain from this point, every subsequent
337+
// projection must be a def projection [projection_chain_structure].
338+
// Every such projection is dominated by `value` (i.e. defInst).
339+
//
340+
// If the walk were only updating the LCA, it could stop here.
341+
//
342+
// It is also collecting values whose storage is projected out of the
343+
// phi, however, so the walk must continue.
344+
updateLCA(defInst->getParent());
345+
for (auto projected : projecteds) {
346+
values.pushIfNotVisited(projected);
347+
}
348+
continue;
349+
}
350+
updateLCA(defInst->getParent());
351+
}
352+
return lca;
353+
}
354+
355+
// Record liveness generated by uses of \p projectedVals.
266356
//
267357
// Return true if no interference was detected along the way.
268-
bool PhiStorageOptimizer::recordUseLiveness(SILValue incomingVal,
358+
bool PhiStorageOptimizer::recordUseLiveness(ProjectedValues &projectedVals,
359+
SILBasicBlock *dominatingBlock,
269360
BasicBlockSetVector &liveBlocks) {
270361
assert(liveBlocks.empty());
271362

272-
// Stop liveness traversal at defBB.
273-
SILBasicBlock *defBB = incomingVal->getParentBlock();
274-
for (auto *use : incomingVal->getUses()) {
275-
StackList<SILBasicBlock *> liveBBWorklist(getFunction());
363+
for (auto projectedVal : projectedVals) {
364+
for (auto *use : projectedVal->getUses()) {
365+
StackList<SILBasicBlock *> liveBBWorklist(getFunction());
366+
367+
// If \p liveBB is already occupied by another value, return
368+
// false. Otherwise, mark \p liveBB live and push it onto liveBBWorklist.
369+
auto visitLiveBlock = [&](SILBasicBlock *liveBB) {
370+
assert(liveBB != phi.phiBlock && "phi operands are consumed");
276371

277-
// If \p liveBB is already occupied by another value, return
278-
// false. Otherwise, mark \p liveBB live and push it onto liveBBWorklist.
279-
auto visitLiveBlock = [&](SILBasicBlock *liveBB) {
280-
assert(liveBB != phi.phiBlock && "phi operands are consumed");
372+
if (occupiedBlocks.contains(liveBB))
373+
return false;
281374

282-
if (occupiedBlocks.contains(liveBB))
375+
// Stop liveness traversal at dominatingBlock.
376+
if (liveBlocks.insert(liveBB) && liveBB != dominatingBlock) {
377+
liveBBWorklist.push_back(liveBB);
378+
}
379+
return true;
380+
};
381+
if (!visitLiveBlock(use->getUser()->getParent()))
283382
return false;
284383

285-
if (liveBlocks.insert(liveBB) && liveBB != defBB) {
286-
liveBBWorklist.push_back(liveBB);
287-
}
288-
return true;
289-
};
290-
if (!visitLiveBlock(use->getUser()->getParent()))
291-
return false;
292-
293-
while (!liveBBWorklist.empty()) {
294-
auto *succBB = liveBBWorklist.pop_back_val();
295-
for (auto *predBB : succBB->getPredecessorBlocks()) {
296-
if (!visitLiveBlock(predBB))
297-
return false;
384+
while (!liveBBWorklist.empty()) {
385+
auto *succBB = liveBBWorklist.pop_back_val();
386+
for (auto *predBB : succBB->getPredecessorBlocks()) {
387+
if (!visitLiveBlock(predBB))
388+
return false;
389+
}
298390
}
299391
}
300392
}

lib/SILOptimizer/Mandatory/PhiStorageOptimizer.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@
2424

2525
namespace swift {
2626

27+
class DominanceInfo;
28+
2729
class CoalescedPhi {
2830
friend class PhiStorageOptimizer;
2931

@@ -37,7 +39,8 @@ class CoalescedPhi {
3739
CoalescedPhi(CoalescedPhi &&) = default;
3840
CoalescedPhi &operator=(CoalescedPhi &&) = default;
3941

40-
void coalesce(PhiValue phi, const ValueStorageMap &valueStorageMap);
42+
void coalesce(PhiValue phi, const ValueStorageMap &valueStorageMap,
43+
DominanceInfo *domInfo);
4144

4245
bool empty() const { return coalescedOperands.empty(); }
4346

test/SILOptimizer/address_lowering.sil

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1431,10 +1431,8 @@ nopers(%error : @owned $any Error):
14311431

14321432
// CHECK-LABEL: sil [ossa] @f262_testTryApplyIntoPhi : {{.*}} {
14331433
// CHECK: {{bb[0-9]+}}([[ADDR:%[^,]+]] :
1434-
// CHECK: [[STACK:%[^,]+]] = alloc_stack
1435-
// CHECK: try_apply undef<R>([[STACK]]) {{.*}}, normal [[NORMAL:bb[0-9]+]]
1434+
// CHECK: try_apply undef<R>([[ADDR]]) {{.*}}, normal [[NORMAL:bb[0-9]+]]
14361435
// CHECK: [[NORMAL]]({{%[^,]+}} : $()):
1437-
// CHECK: copy_addr [take] [[STACK]] to [init] [[ADDR]]
14381436
// CHECK: br [[EXIT:bb[0-9]+]]
14391437
// CHECK: [[EXIT]]:
14401438
// CHECK: return {{%[^,]+}} : $()
@@ -2268,15 +2266,13 @@ bb3:
22682266
// Test the result being stored into an @out enum.
22692267
// CHECK-LABEL: sil [ossa] @test_checked_cast_br4 : {{.*}} {
22702268
// CHECK: {{bb[0-9]+}}([[OUT_ADDR:%[^,]+]] : $*Optional<T>, [[INSTANCE:%[^,]+]] : @owned $TestGeneric<Element>):
2271-
// CHECK: [[TEMP:%[^,]+]] = alloc_stack $Optional<T>
22722269
// CHECK: [[CAST_SOURCE_ADDR:%[^,]+]] = alloc_stack $TestGeneric<Element>
22732270
// CHECK: store [[INSTANCE]] to [init] [[CAST_SOURCE_ADDR]]
2274-
// CHECK: [[CAST_DEST_ADDR:%[^,]+]] = init_enum_data_addr [[TEMP]] : $*Optional<T>, #Optional.some!enumelt
2271+
// CHECK: [[CAST_DEST_ADDR:%[^,]+]] = init_enum_data_addr [[OUT_ADDR]] : $*Optional<T>, #Optional.some!enumelt
22752272
// CHECK: checked_cast_addr_br take_on_success TestGeneric<Element> in [[CAST_SOURCE_ADDR]] : $*TestGeneric<Element> to T in [[CAST_DEST_ADDR]] : $*T, [[SUCCESS:bb[0-9]+]]
22762273
// CHECK: [[SUCCESS]]:
22772274
// CHECK: dealloc_stack [[CAST_SOURCE_ADDR]]
2278-
// CHECK: inject_enum_addr [[TEMP]] : $*Optional<T>, #Optional.some!enumelt
2279-
// CHECK: copy_addr [take] [[TEMP]] to [init] [[OUT_ADDR]]
2275+
// CHECK: inject_enum_addr [[OUT_ADDR]] : $*Optional<T>, #Optional.some!enumelt
22802276
// CHECK-LABEL: } // end sil function 'test_checked_cast_br4'
22812277
sil [ossa] @test_checked_cast_br4 : $@convention(method) <Element><T> (@owned TestGeneric<Element>) -> @out Optional<T> {
22822278
bb0(%3 : @owned $TestGeneric<Element>):

0 commit comments

Comments
 (0)