Skip to content

[6.0] FieldSensitivePrunedLiveness: Handle conditionality of try_apply defs. #72453

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 1 commit into from
Mar 21, 2024
Merged

[6.0] FieldSensitivePrunedLiveness: Handle conditionality of try_apply defs. #72453

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
78 changes: 46 additions & 32 deletions include/swift/SIL/FieldSensitivePrunedLiveness.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,17 +493,21 @@ class FieldSensitivePrunedLiveBlocks {
/// LiveWithin blocks have at least one use and/or def within the block, but
/// are not (yet) LiveOut.
///
/// DeadToLiveEdge blocks are not live within the block itself, but the value
/// becomes live on one or more of the edges out.
///
/// LiveOut blocks are live on at least one successor path. LiveOut blocks may
/// or may not contain defs or uses.
///
/// NOTE: The values below for Dead, LiveWithin, LiveOut were picked to ensure
/// that given a 2 bit representation of the value, a value is Dead if the
/// first bit is 0 and is LiveOut if the second bit is set.
enum IsLive {
Dead = 0,
LiveWithin = 1,
DeadToLiveEdge = 2,
LiveOut = 3,
};

static bool isDead(IsLive liveness) {
return liveness == Dead || liveness == DeadToLiveEdge;
}

/// A bit vector that stores information about liveness. This is composed
/// with SmallBitVector since it contains two bits per liveness so that it
Expand All @@ -524,37 +528,27 @@ class FieldSensitivePrunedLiveBlocks {
unsigned size() const { return bits.size() / 2; }

IsLive getLiveness(unsigned bitNo) const {
if (!bits[bitNo * 2])
return IsLive::Dead;
return bits[bitNo * 2 + 1] ? LiveOut : LiveWithin;
return IsLive((bits[bitNo * 2 + 1] << 1) | bits[bitNo * 2]);
}

/// Returns the liveness in \p resultingFoundLiveness. We only return the
/// bits for endBitNo - startBitNo.
void getLiveness(unsigned startBitNo, unsigned endBitNo,
SmallVectorImpl<IsLive> &resultingFoundLiveness) const {
unsigned actualStartBitNo = startBitNo * 2;
unsigned actualEndBitNo = endBitNo * 2;

for (unsigned i = actualStartBitNo, e = actualEndBitNo; i != e; i += 2) {
if (!bits[i]) {
resultingFoundLiveness.push_back(Dead);
continue;
}

resultingFoundLiveness.push_back(bits[i + 1] ? LiveOut : LiveWithin);
for (unsigned i = startBitNo, e = endBitNo; i != e; ++i) {
resultingFoundLiveness.push_back(getLiveness(i));
}
}

void setLiveness(unsigned startBitNo, unsigned endBitNo, IsLive isLive) {
for (unsigned i = startBitNo * 2, e = endBitNo * 2; i != e; i += 2) {
bits[i] = isLive & 1;
bits[i + 1] = isLive & 2;
}
void setLiveness(unsigned bitNo, IsLive isLive) {
bits[bitNo * 2] = isLive & 1;
bits[bitNo * 2 + 1] = bool(isLive & 2);
}

void setLiveness(unsigned bitNo, IsLive isLive) {
setLiveness(bitNo, bitNo + 1, isLive);
void setLiveness(unsigned startBitNo, unsigned endBitNo, IsLive isLive) {
for (unsigned i = startBitNo, e = endBitNo; i != e; ++i) {
setLiveness(i, isLive);
}
}
};

Expand Down Expand Up @@ -620,9 +614,10 @@ class FieldSensitivePrunedLiveBlocks {
}

void initializeDefBlock(SILBasicBlock *defBB, unsigned startBitNo,
unsigned endBitNo) {
unsigned endBitNo,
IsLive isLive = LiveWithin) {
assert(isInitialized());
markBlockLive(defBB, startBitNo, endBitNo, LiveWithin);
markBlockLive(defBB, startBitNo, endBitNo, isLive);
}

/// Update this liveness result for a single use.
Expand All @@ -632,7 +627,7 @@ class FieldSensitivePrunedLiveBlocks {
auto *block = user->getParent();
if (!isUserBeforeDef) {
auto liveness = getBlockLiveness(block, bitNo);
if (liveness != Dead)
if (!isDead(liveness))
return liveness;
}
computeScalarUseBlockLiveness(block, bitNo);
Expand Down Expand Up @@ -696,6 +691,7 @@ class FieldSensitivePrunedLiveBlocks {
// If we are dead, always update to the new liveness.
switch (iterAndInserted.first->getSecond().getLiveness(bitNo)) {
case Dead:
case DeadToLiveEdge:
iterAndInserted.first->getSecond().setLiveness(bitNo, bitNo + 1,
isLive);
break;
Expand Down Expand Up @@ -935,10 +931,12 @@ class FieldSensitivePrunedLiveness {
return UserBlockRange(getAllUsers(), op);
}

void initializeDefBlock(SILBasicBlock *defBB, TypeTreeLeafTypeRange span) {
void initializeDefBlock(SILBasicBlock *defBB, TypeTreeLeafTypeRange span,
FieldSensitivePrunedLiveBlocks::IsLive isLive
= FieldSensitivePrunedLiveBlocks::LiveWithin) {
assert(isInitialized());
liveBlocks.initializeDefBlock(defBB, span.startEltOffset,
span.endEltOffset);
span.endEltOffset, isLive);
}

/// For flexibility, \p lifetimeEnding is provided by the
Expand Down Expand Up @@ -1303,6 +1301,14 @@ class FieldSensitiveSSAPrunedLiveRange
FieldSensitivePrunedLivenessBoundary &boundary) const;
};

static inline SILBasicBlock *getDefinedInBlock(SILNode *node) {
// try_apply defines the value only on the success edge.
if (auto ta = dyn_cast<TryApplyInst>(node)) {
return ta->getNormalBB();
}
return node->getParentBlock();
}

/// MultiDefPrunedLiveness is computed incrementally by calling updateForUse.
///
/// Defs should be initialized before calling updatingForUse on any def
Expand Down Expand Up @@ -1351,9 +1357,17 @@ class FieldSensitiveMultiDefPrunedLiveRange
void initializeDef(SILNode *node, TypeTreeLeafTypeRange span) {
assert(Super::isInitialized());
defs.insert(node, span);
auto *block = node->getParentBlock();
defBlocks.insert(block, span);
initializeDefBlock(block, span);
auto defBlock = getDefinedInBlock(node);
defBlocks.insert(defBlock, span);
initializeDefBlock(defBlock, span);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine fixing this on main in a forthcoming commit.

I find the logic here hides what is actually happening. First getDefinedInBlock(node) has a special case for try_apply and then we have here another special case below for try_apply. Why not just do something conditional on try_apply here. Then it is clear where all of the special case code is. That being said, I see you used getDefinedInBlock later in the patch, so maybe it makes sense to do this... IDK. Just reads weird to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I went back and forth on whether to use getDefinedInBlock here. Maybe it's premature, but I was also trying to think about how to systematically handle these sorts of def-on-edge cases if any more come up; if it does then having getDefinedInBlock already be here and used in more than one place seems like a good thing.


if (auto ta = dyn_cast<TryApplyInst>(node)) {
// The value becomes live on the success edge.
// Mark the basic block the try_apply terminates as a dead-to-live
// edge.
initializeDefBlock(ta->getParent(), span,
FieldSensitivePrunedLiveBlocks::DeadToLiveEdge);
}
}

void initializeDef(SILInstruction *def, TypeTreeLeafTypeRange span) {
Expand Down
34 changes: 28 additions & 6 deletions lib/SIL/Utils/FieldSensitivePrunedLiveness.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,7 @@ void FieldSensitivePrunedLiveBlocks::computeScalarUseBlockLiveness(
case LiveWithin:
markBlockLive(predBlock, bitNo, LiveOut);
break;
case DeadToLiveEdge:
case LiveOut:
break;
}
Expand Down Expand Up @@ -615,6 +616,7 @@ void FieldSensitivePrunedLiveBlocks::updateForUse(
} else {
LLVM_FALLTHROUGH;
}
case DeadToLiveEdge:
case Dead: {
// This use block has not yet been marked live. Mark it and its
// predecessor blocks live.
Expand All @@ -634,6 +636,8 @@ FieldSensitivePrunedLiveBlocks::getStringRef(IsLive isLive) const {
return "Dead";
case LiveWithin:
return "LiveWithin";
case DeadToLiveEdge:
return "DeadToLiveEdge";
case LiveOut:
return "LiveOut";
}
Expand Down Expand Up @@ -853,6 +857,7 @@ bool FieldSensitivePrunedLiveRange<LivenessWithDefs>::isWithinBoundary(
PRUNED_LIVENESS_LOG(llvm::dbgs() << " Visiting bit: " << bit << '\n');
bool isLive = false;
switch (pair.value()) {
case FieldSensitivePrunedLiveBlocks::DeadToLiveEdge:
case FieldSensitivePrunedLiveBlocks::Dead:
PRUNED_LIVENESS_LOG(llvm::dbgs() << " Dead... continuing!\n");
// We are only not within the boundary if all of our bits are dead. We
Expand Down Expand Up @@ -942,6 +947,8 @@ static StringRef getStringRef(FieldSensitivePrunedLiveBlocks::IsLive isLive) {
return "Dead";
case FieldSensitivePrunedLiveBlocks::LiveWithin:
return "LiveWithin";
case FieldSensitivePrunedLiveBlocks::DeadToLiveEdge:
return "DeadToLiveEdge";
case FieldSensitivePrunedLiveBlocks::LiveOut:
return "LiveOut";
}
Expand Down Expand Up @@ -972,8 +979,8 @@ void FieldSensitivePrunedLiveRange<LivenessWithDefs>::computeBoundary(
switch (pair.value()) {
case FieldSensitivePrunedLiveBlocks::LiveOut:
for (SILBasicBlock *succBB : block->getSuccessors()) {
if (getBlockLiveness(succBB, index) ==
FieldSensitivePrunedLiveBlocks::Dead) {
if (FieldSensitivePrunedLiveBlocks::isDead(
getBlockLiveness(succBB, index))) {
PRUNED_LIVENESS_LOG(llvm::dbgs() << "Marking succBB as boundary edge: bb"
<< succBB->getDebugID() << '\n');
boundary.getBoundaryEdgeBits(succBB).set(index);
Expand All @@ -989,6 +996,9 @@ void FieldSensitivePrunedLiveRange<LivenessWithDefs>::computeBoundary(
foundAnyNonDead = true;
break;
}
case FieldSensitivePrunedLiveBlocks::DeadToLiveEdge:
foundAnyNonDead = true;
LLVM_FALLTHROUGH;
case FieldSensitivePrunedLiveBlocks::Dead:
// We do not assert here like in the normal pruned liveness
// implementation since we can have dead on some bits and liveness along
Expand Down Expand Up @@ -1178,7 +1188,7 @@ void findBoundaryInSSADefBlock(SILNode *ssaDef, unsigned bitNo,
// defInst is null for argument defs.
PRUNED_LIVENESS_LOG(llvm::dbgs() << "Searching using findBoundaryInSSADefBlock.\n");
SILInstruction *defInst = dyn_cast<SILInstruction>(ssaDef);
for (SILInstruction &inst : llvm::reverse(*ssaDef->getParentBlock())) {
for (SILInstruction &inst : llvm::reverse(*getDefinedInBlock(ssaDef))) {
PRUNED_LIVENESS_LOG(llvm::dbgs() << "Visiting: " << inst);
if (&inst == defInst) {
PRUNED_LIVENESS_LOG(llvm::dbgs() << " Found dead def: " << *defInst);
Expand All @@ -1192,9 +1202,21 @@ void findBoundaryInSSADefBlock(SILNode *ssaDef, unsigned bitNo,
}
}

auto *deadArg = cast<SILArgument>(ssaDef);
PRUNED_LIVENESS_LOG(llvm::dbgs() << " Found dead arg: " << *deadArg);
boundary.getDeadDefsBits(deadArg).set(bitNo);
if (auto *deadArg = dyn_cast<SILArgument>(ssaDef)) {
PRUNED_LIVENESS_LOG(llvm::dbgs() << " Found dead arg: " << *deadArg);
boundary.getDeadDefsBits(deadArg).set(bitNo);
return;
}

// If we searched the success branch of a try_apply and found no uses, then
// the try_apply itself is a dead def.
if (isa<TryApplyInst>(ssaDef)) {
PRUNED_LIVENESS_LOG(llvm::dbgs() << " Found dead try_apply: " << *ssaDef);
boundary.getDeadDefsBits(ssaDef).set(bitNo);
return;
}

llvm_unreachable("def not found?!");
}

//===----------------------------------------------------------------------===//
Expand Down
1 change: 1 addition & 0 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2816,6 +2816,7 @@ bool GlobalLivenessChecker::testInstVectorLiveness(
for (auto isLive : isLiveArray) {
switch (isLive) {
case IsLive::Dead:
case IsLive::DeadToLiveEdge:
LLVM_DEBUG(llvm::dbgs() << " Dead block!\n");
// Ignore a dead block. Our error use could not be in such a block.
//
Expand Down
8 changes: 3 additions & 5 deletions test/Interpreter/moveonly_resilient_deinit_on_throw_same_module.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@
// RUN: %target-codesign %t/a.out.fragile
// RUN: %target-run %t/a.out.fragile | %FileCheck %s

// FIXME: miscompiles cause extra deinits with library evolution enabled

// R/UN: %target-build-swift -enable-library-evolution -o %t/a.out.resilient %s
// R/UN: %target-codesign %t/a.out.resilient
// R/UN: %target-run %t/a.out.resilient | %FileCheck %s
// RUN: %target-build-swift -enable-library-evolution -o %t/a.out.resilient %s
// RUN: %target-codesign %t/a.out.resilient
// RUN: %target-run %t/a.out.resilient | %FileCheck %s

// REQUIRES: executable_test

Expand Down