Skip to content

[SIL] Key consume checking off var_decl attr. #70054

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.

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 4 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 17 additions & 3 deletions docs/SIL.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4514,8 +4514,15 @@ live. This makes sense semantically since ``%1`` is modeling a new value with a
dependent lifetime on ``%0``.

The optional ``lexical`` attribute specifies that the operand corresponds to a
local variable in the Swift source, so special care must be taken when moving
the end_borrow.
local variable with a lexical lifetime in the Swift source, so special care
must be taken when moving the end_borrow. Compare to the ``var_decl``
attribute.

The optional ``pointer_escape`` attribute specifies that a pointer to the
operand escapes within the borrow scope introduced by this begin_borrow.

The optional ``var_decl`` attribute specifies that the operand corresponds to a
local variable in the Swift source.

This instruction is only valid in functions in Ownership SSA form.

Expand Down Expand Up @@ -6373,7 +6380,14 @@ values'. A move_value instruction is an instruction that introduces (or injects)
a type `T` into the move only value space.

The ``lexical`` attribute specifies that the value corresponds to a local
variable in the Swift source.
variable with a lexical lifetime in the Swift source. Compare to the
``var_decl`` attribute.

The optional ``pointer_escape`` attribute specifies that a pointer to the
operand escapes within the scope introduced by this move_value.

The optional ``var_decl`` attribute specifies that the operand corresponds to a
local variable in the Swift source.


drop_deinit
Expand Down
16 changes: 10 additions & 6 deletions include/swift/SIL/SILBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -804,11 +804,13 @@ class SILBuilder {

BeginBorrowInst *createBeginBorrow(SILLocation Loc, SILValue LV,
bool isLexical = false,
bool hasPointerEscape = false) {
bool hasPointerEscape = false,
bool fromVarDecl = false) {
assert(getFunction().hasOwnership());
assert(!LV->getType().isAddress());
return insert(new (getModule()) BeginBorrowInst(getSILDebugLocation(Loc),
LV, isLexical, hasPointerEscape));
return insert(new (getModule())
BeginBorrowInst(getSILDebugLocation(Loc), LV, isLexical,
hasPointerEscape, fromVarDecl));
}

/// Convenience function for creating a load_borrow on non-trivial values and
Expand Down Expand Up @@ -1402,13 +1404,15 @@ class SILBuilder {

MoveValueInst *createMoveValue(SILLocation loc, SILValue operand,
bool isLexical = false,
bool hasPointerEscape = false) {
bool hasPointerEscape = false,
bool fromVarDecl = false) {
assert(getFunction().hasOwnership());
assert(!operand->getType().isTrivial(getFunction()) &&
"Should not be passing trivial values to this api. Use instead "
"emitMoveValueOperation");
return insert(new (getModule()) MoveValueInst(
getSILDebugLocation(loc), operand, isLexical, hasPointerEscape));
return insert(new (getModule())
MoveValueInst(getSILDebugLocation(loc), operand,
isLexical, hasPointerEscape, fromVarDecl));
}

DropDeinitInst *createDropDeinit(SILLocation loc, SILValue operand) {
Expand Down
15 changes: 8 additions & 7 deletions include/swift/SIL/SILCloner.h
Original file line number Diff line number Diff line change
Expand Up @@ -1217,10 +1217,11 @@ void SILCloner<ImplClass>::visitBeginBorrowInst(BeginBorrowInst *Inst) {
return recordFoldedValue(Inst, getOpValue(Inst->getOperand()));
}

recordClonedInstruction(
Inst, getBuilder().createBeginBorrow(getOpLocation(Inst->getLoc()),
getOpValue(Inst->getOperand()),
Inst->isLexical()));
recordClonedInstruction(Inst,
getBuilder().createBeginBorrow(
getOpLocation(Inst->getLoc()),
getOpValue(Inst->getOperand()), Inst->isLexical(),
Inst->hasPointerEscape(), Inst->isFromVarDecl()));
}

template <typename ImplClass>
Expand Down Expand Up @@ -1930,9 +1931,9 @@ void SILCloner<ImplClass>::visitMoveValueInst(MoveValueInst *Inst) {
if (!getBuilder().hasOwnership()) {
return recordFoldedValue(Inst, getOpValue(Inst->getOperand()));
}
auto *MVI = getBuilder().createMoveValue(getOpLocation(Inst->getLoc()),
getOpValue(Inst->getOperand()),
Inst->isLexical());
auto *MVI = getBuilder().createMoveValue(
getOpLocation(Inst->getLoc()), getOpValue(Inst->getOperand()),
Inst->isLexical(), Inst->hasPointerEscape(), Inst->isFromVarDecl());
MVI->setAllowsDiagnostics(Inst->getAllowDiagnostics());
recordClonedInstruction(Inst, MVI);
}
Expand Down
12 changes: 10 additions & 2 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -4398,11 +4398,12 @@ class BeginBorrowInst
USE_SHARED_UINT8;

BeginBorrowInst(SILDebugLocation DebugLoc, SILValue LValue, bool isLexical,
bool hasPointerEscape)
bool hasPointerEscape, bool fromVarDecl)
: UnaryInstructionBase(DebugLoc, LValue,
LValue->getType().getObjectType()) {
sharedUInt8().BeginBorrowInst.lexical = isLexical;
sharedUInt8().BeginBorrowInst.pointerEscape = hasPointerEscape;
sharedUInt8().BeginBorrowInst.fromVarDecl = fromVarDecl;
}

public:
Expand All @@ -4428,6 +4429,10 @@ class BeginBorrowInst
sharedUInt8().BeginBorrowInst.pointerEscape = pointerEscape;
}

bool isFromVarDecl() const {
return sharedUInt8().BeginBorrowInst.fromVarDecl;
}

/// Return a range over all EndBorrow instructions for this BeginBorrow.
EndBorrowRange getEndBorrows() const;

Expand Down Expand Up @@ -8374,10 +8379,11 @@ class MoveValueInst
USE_SHARED_UINT8;

MoveValueInst(SILDebugLocation DebugLoc, SILValue operand, bool isLexical,
bool hasPointerEscape)
bool hasPointerEscape, bool fromVarDecl)
: UnaryInstructionBase(DebugLoc, operand, operand->getType()) {
sharedUInt8().MoveValueInst.lexical = isLexical;
sharedUInt8().MoveValueInst.pointerEscape = hasPointerEscape;
sharedUInt8().MoveValueInst.fromVarDecl = fromVarDecl;
}

public:
Expand All @@ -8400,6 +8406,8 @@ class MoveValueInst
void setHasPointerEscape(bool pointerEscape) {
sharedUInt8().MoveValueInst.pointerEscape = pointerEscape;
}

bool isFromVarDecl() const { return sharedUInt8().MoveValueInst.fromVarDecl; }
};

class DropDeinitInst
Expand Down
6 changes: 4 additions & 2 deletions include/swift/SIL/SILNode.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,8 @@ class alignas(8) SILNode :

SHARED_FIELD(BeginBorrowInst, uint8_t
lexical : 1,
pointerEscape : 1);
pointerEscape : 1,
fromVarDecl : 1);

SHARED_FIELD(CopyAddrInst, uint8_t
isTakeOfSrc : 1,
Expand Down Expand Up @@ -272,7 +273,8 @@ class alignas(8) SILNode :
SHARED_FIELD(MoveValueInst, uint8_t
allowDiagnostics : 1,
lexical : 1,
pointerEscape : 1);
pointerEscape : 1,
fromVarDecl : 1);

// Do not use `_sharedUInt8_private` outside of SILNode.
} _sharedUInt8_private;
Expand Down
5 changes: 5 additions & 0 deletions lib/SIL/IR/SILPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,9 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
if (BBI->hasPointerEscape()) {
*this << "[pointer_escape] ";
}
if (BBI->isFromVarDecl()) {
*this << "[var_decl] ";
}
*this << getIDAndType(BBI->getOperand());
}

Expand Down Expand Up @@ -2074,6 +2077,8 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
*this << "[lexical] ";
if (I->hasPointerEscape())
*this << "[pointer_escape] ";
if (I->isFromVarDecl())
*this << "[var_decl] ";
*this << getIDAndType(I->getOperand());
}

Expand Down
12 changes: 10 additions & 2 deletions lib/SIL/Parser/ParseSIL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3846,6 +3846,7 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
bool allowsDiagnostics = false;
bool isLexical = false;
bool hasPointerEscape = false;
bool fromVarDecl = false;

StringRef AttrName;
SourceLoc AttrLoc;
Expand All @@ -3856,6 +3857,8 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
isLexical = true;
else if (AttrName == "pointer_escape")
hasPointerEscape = true;
else if (AttrName == "var_decl")
fromVarDecl = true;
else {
P.diagnose(InstLoc.getSourceLoc(),
diag::sil_invalid_attribute_for_instruction, AttrName,
Expand All @@ -3868,7 +3871,8 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
return true;
if (parseSILDebugLocation(InstLoc, B))
return true;
auto *MVI = B.createMoveValue(InstLoc, Val, isLexical, hasPointerEscape);
auto *MVI = B.createMoveValue(InstLoc, Val, isLexical, hasPointerEscape,
fromVarDecl);
MVI->setAllowsDiagnostics(allowsDiagnostics);
ResultVal = MVI;
break;
Expand Down Expand Up @@ -4077,6 +4081,7 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,

bool isLexical = false;
bool hasPointerEscape = false;
bool fromVarDecl = false;

StringRef AttrName;
SourceLoc AttrLoc;
Expand All @@ -4085,6 +4090,8 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
isLexical = true;
else if (AttrName == "pointer_escape")
hasPointerEscape = true;
else if (AttrName == "var_decl")
fromVarDecl = true;
else {
P.diagnose(InstLoc.getSourceLoc(),
diag::sil_invalid_attribute_for_instruction, AttrName,
Expand All @@ -4097,7 +4104,8 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
parseSILDebugLocation(InstLoc, B))
return true;

ResultVal = B.createBeginBorrow(InstLoc, Val, isLexical, hasPointerEscape);
ResultVal = B.createBeginBorrow(InstLoc, Val, isLexical, hasPointerEscape,
fromVarDecl);
break;
}

Expand Down
24 changes: 10 additions & 14 deletions lib/SILGen/SILGenDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -566,9 +566,9 @@ class LocalVariableInitialization : public SingleBufferInitialization {
//
// Only add a lexical lifetime to the box if the the variable it stores
// requires one.
if (lifetime.isLexical()) {
Box = SGF.B.createBeginBorrow(decl, Box, /*isLexical=*/true);
}
Box = SGF.B.createBeginBorrow(
decl, Box, /*isLexical=*/lifetime.isLexical(),
/*hasPointerEscape=*/false, /*fromVarDecl=*/true);
}

Addr = SGF.B.createProjectBox(decl, Box, 0);
Expand Down Expand Up @@ -835,16 +835,17 @@ class LetValueInitialization : public Initialization {
// Otherwise, if we do not have a no implicit copy variable, just follow
// the "normal path": perform a lexical borrow if the lifetime is lexical.
if (!vd->isNoImplicitCopy()) {
if (SGF.F.getLifetime(vd, value->getType()).isLexical())
return SGF.B.createBeginBorrow(PrologueLoc, value, /*isLexical*/ true);
else
return value;
return SGF.B.createBeginBorrow(
PrologueLoc, value,
/*isLexical=*/SGF.F.getLifetime(vd, value->getType()).isLexical(),
/*hasPointerEscape=*/false, /*fromVarDecl=*/true);
}

// If we have a no implicit copy lexical, emit the instruction stream so
// that the move checker knows to check this variable.
value = SGF.B.createBeginBorrow(PrologueLoc, value,
/*isLexical*/ true);
value = SGF.B.createBeginBorrow(
PrologueLoc, value,
/*isLexical*/ true, /*hasPointerEscape=*/false, /*fromVarDecl=*/true);
value = SGF.B.createCopyValue(PrologueLoc, value);
value = SGF.B.createOwnedCopyableToMoveOnlyWrapperValue(PrologueLoc, value);
return SGF.B.createMarkUnresolvedNonCopyableValueInst(
Expand Down Expand Up @@ -2166,11 +2167,6 @@ void SILGenFunction::destroyLocalVariable(SILLocation silLoc, VarDecl *vd) {
return;
}

if (!F.getLifetime(vd, Val->getType()).isLexical()) {
B.emitDestroyValueOperation(silLoc, Val);
return;
}

// This handles any case where we copy + begin_borrow or copyable_to_moveonly
// + begin_borrow. In either case we just need to end the lifetime of the
// begin_borrow's operand.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ bool ConsumeOperatorCopyableValuesChecker::check() {
for (auto &block : *fn) {
for (auto &ii : block) {
if (auto *bbi = dyn_cast<BeginBorrowInst>(&ii)) {
if (bbi->isLexical() && !bbi->getType().isMoveOnly()) {
if (bbi->isFromVarDecl() && !bbi->getType().isMoveOnly()) {
LLVM_DEBUG(llvm::dbgs()
<< "Found lexical lifetime to check: " << *bbi);
valuesToCheck.insert(bbi);
Expand Down
8 changes: 4 additions & 4 deletions lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3028,7 +3028,7 @@ SILValue LifetimeChecker::handleConditionalInitAssign() {
///
/// %box = alloc_box
/// %mark_uninit = mark_uninitialized %box
/// %lifetime = begin_borrow [lexical] %mark_uninit
/// %lifetime = begin_borrow [var_decl] %mark_uninit
/// %proj_box = project_box %lifetime
///
/// We are replacing a
Expand All @@ -3048,7 +3048,7 @@ SILValue LifetimeChecker::handleConditionalInitAssign() {
/// Consequently, it's not sufficient to just replace the destroy_value
/// %mark_uninit with a destroy_addr %proj_box (or to replace it with a diamond
/// where one branch has that destroy_addr) because the destroy_addr is a use
/// of %proj_box which must be within the lexical lifetime of the box.
/// of %proj_box which must be within the var_decl lifetime of the box.
///
/// On the other side, we are hemmed in by the fact that the end_borrow must
/// precede the dealloc_box which will be created in the diamond. So we
Expand Down Expand Up @@ -3084,12 +3084,12 @@ static bool adjustAllocBoxEndBorrow(SILInstruction *previous,
if (!pbi)
return false;

// This fixup only applies if we're destroying a project_box of the lexical
// This fixup only applies if we're destroying a project_box of the var_decl
// lifetime of an alloc_box.
auto *lifetime = dyn_cast<BeginBorrowInst>(pbi->getOperand());
if (!lifetime)
return false;
assert(lifetime->isLexical());
assert(lifetime->isFromVarDecl());
assert(isa<AllocBoxInst>(
cast<MarkUninitializedInst>(lifetime->getOperand())->getOperand()));

Expand Down
Loading