Skip to content

Commit ad33b5d

Browse files
committed
[OSSALifetimeCompletion] Handle available boundary
Not every block in a region which begins with the non-lifetime-ending boundary of a value and ending with unreachable-terminated blocks has the value available. If the unreachable-terminated blocks in this boundary are not available, it is incorrect to insert destroys of the value in them: it is an overconsume on some paths. Previously, however, destroys were simply being inserted at the unreachable. Here, this is fixed by finding the boundary of availability within that region and inserting destroys before the terminators of the blocks on that boundary. rdar://116255254
1 parent 2824bf3 commit ad33b5d

File tree

5 files changed

+388
-20
lines changed

5 files changed

+388
-20
lines changed

include/swift/SIL/OSSALifetimeCompletion.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class OSSALifetimeCompletion {
8787

8888
static void visitUnreachableLifetimeEnds(
8989
SILValue value, const SSAPrunedLiveness &liveness,
90-
llvm::function_ref<void(UnreachableInst *)> visit);
90+
llvm::function_ref<void(SILInstruction *)> visit);
9191

9292
protected:
9393
bool analyzeAndUpdateLifetime(SILValue value, bool forceBoundaryCompletion);

include/swift/SILOptimizer/Utils/CanonicalizeOSSALifetime.h

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -247,9 +247,12 @@ class CanonicalizeOSSALifetime final {
247247
SILValue currentDef;
248248

249249
/// Original points in the CFG where the current value's lifetime is consumed
250-
/// or destroyed. For guaranteed values it remains empty. A backward walk from
251-
/// these blocks must discover all uses on paths that lead to a return or
252-
/// throw.
250+
/// or destroyed. Each block either contains a consuming instruction (e.g.
251+
/// `destroy_value`) or is on the availability boundary of the value in a
252+
/// dead-end region (e.g. `unreachable`).
253+
///
254+
/// For guaranteed values it remains empty. A backward walk from these blocks
255+
/// must discover all uses on paths that lead to a return or throw.
253256
///
254257
/// These blocks are not necessarily in the pruned live blocks since
255258
/// pruned liveness does not consider destroy_values.
@@ -418,6 +421,11 @@ class CanonicalizeOSSALifetime final {
418421
void recordDebugValue(DebugValueInst *dvi) { debugValues.insert(dvi); }
419422

420423
void recordConsumingUse(Operand *use) { recordConsumingUser(use->getUser()); }
424+
/// Record that the value is consumed at `user`.
425+
///
426+
/// Either `user` is a consuming use (e.g. `destroy_value`) or it is the
427+
/// terminator of a block on the availability boundary of the value in a
428+
/// dead-end region (e.g. `unreachable`).
421429
void recordConsumingUser(SILInstruction *user) {
422430
consumingBlocks.insert(user->getParent());
423431
}

lib/SIL/Utils/OSSALifetimeCompletion.cpp

Lines changed: 192 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,10 @@
5151

5252
#include "swift/SIL/OSSALifetimeCompletion.h"
5353
#include "swift/SIL/SILBuilder.h"
54+
#include "swift/SIL/SILFunction.h"
5455
#include "swift/SIL/SILInstruction.h"
5556
#include "swift/SIL/Test.h"
57+
#include "llvm/ADT/STLExtras.h"
5658

5759
using namespace swift;
5860

@@ -99,40 +101,217 @@ static bool endLifetimeAtBoundary(SILValue value,
99101
return changed;
100102
}
101103

102-
void OSSALifetimeCompletion::visitUnreachableLifetimeEnds(
103-
SILValue value, const SSAPrunedLiveness &liveness,
104-
llvm::function_ref<void(UnreachableInst *)> visit) {
104+
namespace {
105+
/// Implements OSSALifetimeCompletion::visitUnreachableLifetimeEnds. Finds
106+
/// positions as near as possible to unreachables at which `value`'s lifetime
107+
/// is available.
108+
///
109+
/// Finding these positions is a three step process:
110+
/// 1) computeRegion: Forward CFG walk from non-lifetime-ending boundary to find
111+
/// the dead-end region in which the value might be available.
112+
/// 2) propagateAvailability: Forward iterative dataflow within the region to
113+
/// determine which blocks the value is available in.
114+
/// 3) visitAvailabilityBoundary: Visits the final blocks in the region where
115+
/// the value is available--these are the blocks
116+
/// without successors or with at least one
117+
/// unavailable successor.
118+
class VisitUnreachableLifetimeEnds {
119+
/// The value whose dead-end block lifetime ends are to be visited.
120+
SILValue value;
121+
122+
/// The non-lifetime-ending boundary of `value`.
123+
BasicBlockSet starts;
124+
/// The region between (inclusive) the `starts` and the unreachable blocks.
125+
BasicBlockSetVector region;
126+
127+
public:
128+
VisitUnreachableLifetimeEnds(SILValue value)
129+
: value(value), starts(value->getFunction()),
130+
region(value->getFunction()) {}
131+
132+
/// Region discovery.
133+
///
134+
/// Forward CFG walk from non-lifetime-ending boundary to unreachable
135+
/// instructions.
136+
void computeRegion(const SSAPrunedLiveness &liveness);
137+
138+
struct Result;
139+
140+
/// Iterative dataflow to determine availability for each block in `region`.
141+
void propagateAvailablity(Result &result);
142+
143+
/// Visit the terminators of blocks on the boundary of availability.
144+
void
145+
visitAvailabilityBoundary(Result const &result,
146+
llvm::function_ref<void(SILInstruction *)> visit);
147+
148+
struct State {
149+
enum class Value : uint8_t {
150+
Unavailable = 0,
151+
Available,
152+
Unknown,
153+
};
154+
Value value;
155+
156+
State(Value value) : value(value){};
157+
operator Value() const { return value; }
158+
State meet(State const other) const {
159+
return *this < other ? *this : other;
160+
}
161+
162+
static State Unavailable() { return {Value::Unavailable}; }
163+
static State Available() { return {Value::Available}; }
164+
static State Unknown() { return {Value::Unknown}; }
165+
};
166+
167+
struct Result {
168+
BasicBlockBitfield states;
169+
170+
Result(SILFunction *function) : states(function, 2) {}
171+
172+
State getState(SILBasicBlock *block) const {
173+
return {(State::Value)states.get(block)};
174+
}
175+
176+
void setState(SILBasicBlock *block, State newState) {
177+
states.set(block, (unsigned)newState.value);
178+
}
179+
180+
/// Propagate predecessors' state into `block`.
181+
///
182+
/// states[block] ∧= state[predecessor_1] ∧ ... ∧ state[predecessor_n]
183+
bool updateState(SILBasicBlock *block) {
184+
auto oldState = getState(block);
185+
auto state = oldState;
186+
for (auto *predecessor : block->getPredecessorBlocks()) {
187+
state = state.meet(getState(predecessor));
188+
}
189+
setState(block, state);
190+
return state != oldState;
191+
}
192+
};
193+
};
194+
195+
void VisitUnreachableLifetimeEnds::computeRegion(
196+
const SSAPrunedLiveness &liveness) {
197+
// Find the non-lifetime-ending boundary of `value`.
105198
PrunedLivenessBoundary boundary;
106199
liveness.computeBoundary(boundary);
107200

108-
BasicBlockWorklist deadEndBlocks(value->getFunction());
109201
for (SILInstruction *lastUser : boundary.lastUsers) {
110202
if (liveness.isInterestingUser(lastUser)
111203
!= PrunedLiveness::LifetimeEndingUse) {
112-
deadEndBlocks.push(lastUser->getParent());
204+
region.insert(lastUser->getParent());
205+
starts.insert(lastUser->getParent());
113206
}
114207
}
115208
for (SILBasicBlock *edge : boundary.boundaryEdges) {
116-
deadEndBlocks.push(edge);
209+
region.insert(edge);
210+
starts.insert(edge);
117211
}
118212
for (SILNode *deadDef : boundary.deadDefs) {
119-
deadEndBlocks.push(deadDef->getParentBlock());
213+
region.insert(deadDef->getParentBlock());
214+
starts.insert(deadDef->getParentBlock());
215+
}
216+
217+
// Forward walk to find the region in which `value` might be available.
218+
BasicBlockWorklist regionWorklist(value->getFunction());
219+
// Start the forward walk from the non-lifetime-ending boundary.
220+
for (auto *start : region) {
221+
regionWorklist.push(start);
120222
}
121-
// Forward CFG walk from the non-lifetime-ending boundary to the unreachable
122-
// instructions.
123-
while (auto *block = deadEndBlocks.pop()) {
223+
while (auto *block = regionWorklist.pop()) {
124224
if (block->succ_empty()) {
125225
// This assert will fail unless there are already lifetime-ending
126226
// instruction on all paths to normal function exits.
127-
auto *unreachable = cast<UnreachableInst>(block->getTerminator());
128-
visit(unreachable);
227+
assert(isa<UnreachableInst>(block->getTerminator()));
129228
}
130229
for (auto *successor : block->getSuccessorBlocks()) {
131-
deadEndBlocks.pushIfNotVisited(successor);
230+
regionWorklist.pushIfNotVisited(successor);
231+
region.insert(successor);
132232
}
133233
}
134234
}
135235

236+
void VisitUnreachableLifetimeEnds::propagateAvailablity(Result &result) {
237+
// Initialize per-block state.
238+
// - all blocks outside of the region are ::Unavailable (automatically
239+
// initialized)
240+
// - non-initial in-region blocks are Unknown
241+
// - start blocks are ::Available
242+
for (auto *block : region) {
243+
if (starts.contains(block))
244+
result.setState(block, State::Available());
245+
else
246+
result.setState(block, State::Unknown());
247+
}
248+
249+
BasicBlockWorklist worklist(value->getFunction());
250+
251+
// Initialize worklist with all participating blocks.
252+
//
253+
// Only perform dataflow in the non-initial region. Every initial block is
254+
// by definition ::Available.
255+
for (auto *block : region) {
256+
if (starts.contains(block))
257+
continue;
258+
worklist.push(block);
259+
}
260+
261+
// Iterate over blocks which are successors of blocks whose state changed.
262+
while (auto *block = worklist.popAndForget()) {
263+
// Only propagate availability in non-initial, in-region blocks.
264+
if (!region.contains(block) || starts.contains(block))
265+
continue;
266+
auto changed = result.updateState(block);
267+
if (!changed) {
268+
continue;
269+
}
270+
// The state has changed. Propagate the new state into successors.
271+
for (auto *successor : block->getSuccessorBlocks()) {
272+
worklist.pushIfNotVisited(successor);
273+
}
274+
}
275+
}
276+
277+
void VisitUnreachableLifetimeEnds::visitAvailabilityBoundary(
278+
Result const &result, llvm::function_ref<void(SILInstruction *)> visit) {
279+
for (auto *block : region) {
280+
auto available = result.getState(block) == State::Available();
281+
if (!available) {
282+
continue;
283+
}
284+
auto hasUnreachableSuccessor = [&]() {
285+
// Use a lambda to avoid checking if possible.
286+
return llvm::any_of(block->getSuccessorBlocks(), [&result](auto *block) {
287+
return result.getState(block) == State::Unavailable();
288+
});
289+
};
290+
if (!block->succ_empty() && !hasUnreachableSuccessor()) {
291+
continue;
292+
}
293+
assert(hasUnreachableSuccessor() ||
294+
isa<UnreachableInst>(block->getTerminator()));
295+
visit(block->getTerminator());
296+
}
297+
}
298+
} // end anonymous namespace
299+
300+
void OSSALifetimeCompletion::visitUnreachableLifetimeEnds(
301+
SILValue value, const SSAPrunedLiveness &liveness,
302+
llvm::function_ref<void(SILInstruction *)> visit) {
303+
304+
VisitUnreachableLifetimeEnds visitor(value);
305+
306+
visitor.computeRegion(liveness);
307+
308+
VisitUnreachableLifetimeEnds::Result result(value->getFunction());
309+
310+
visitor.propagateAvailablity(result);
311+
312+
visitor.visitAvailabilityBoundary(result, visit);
313+
}
314+
136315
static bool endLifetimeAtUnreachableBlocks(SILValue value,
137316
const SSAPrunedLiveness &liveness) {
138317
bool changed = false;
@@ -274,4 +453,3 @@ bool UnreachableLifetimeCompletion::completeLifetimes() {
274453
}
275454
return changed;
276455
}
277-

test/SILOptimizer/mem2reg_lifetime.sil

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1858,8 +1858,7 @@ right:
18581858
// CHECK: {{bb[^,]+}}([[INSTANCE:%[^,]+]] : @owned $C):
18591859
// CHECK: cond_br undef, [[BB1:bb[0-9]+]], [[BB4:bb[0-9]+]]
18601860
// CHECK: [[BB1]]:
1861-
// CHECK: [[COPY:%[^,]+]] = copy_value [[INSTANCE]]
1862-
// CHECK: [[LIFETIME_OWNED:%[^,]+]] = move_value [lexical] [[COPY]]
1861+
// CHECK: [[LIFETIME_OWNED:%[^,]+]] = move_value [lexical] [[INSTANCE]]
18631862
// CHECK: cond_br undef, [[BB2:bb[0-9]+]], [[BB3:bb[0-9]+]]
18641863
// CHECK: [[BB2]]:
18651864
// CHECK: destroy_value [[LIFETIME_OWNED]]

0 commit comments

Comments
 (0)