Skip to content

Commit ee451f0

Browse files
committed
[move-only] If we have a noncopyable type on an alloc_stack/alloc_box/debug_value, set the [moveable_value_debuginfo] flag.
This still does not have the complete behavior that we want since we are not yet inserting the debug_value undef to invalidate after performing moves. NOTE: In printed SIL, I decided to make the flag implicit if we have a noncopyable type. This is just to reduce the bloat in SIL. If one needs this property for a copyable type though, it is mandatory.
1 parent 8d12f89 commit ee451f0

File tree

5 files changed

+47
-27
lines changed

5 files changed

+47
-27
lines changed

docs/SIL.rst

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3770,10 +3770,11 @@ This is the case, e.g. for conditionally initialized objects.
37703770
The optional ``lexical`` attribute specifies that the storage corresponds to a
37713771
local variable in the Swift source.
37723772

3773-
The optional ``moveable_value_debuginfo`` attribute specifies that when
3774-
emitting debug info, the code generator can not assume that the value in the
3775-
alloc_stack can be semantically valid over the entire function frame when
3776-
emitting debug info.
3773+
The optional ``moveable_value_debuginfo`` attribute specifies that when emitting
3774+
debug info, the code generator can not assume that the value in the alloc_stack
3775+
can be semantically valid over the entire function frame when emitting debug
3776+
info. NOTE: This is implicitly set to true if the alloc_stack's type is
3777+
noncopyable. This is just done to make SIL less verbose.
37773778

37783779
The memory is not retainable. To allocate a retainable box for a value
37793780
type, use ``alloc_box``.
@@ -3886,10 +3887,11 @@ Releasing a box is undefined behavior if the box's value is uninitialized.
38863887
To deallocate a box whose value has not been initialized, ``dealloc_box``
38873888
should be used.
38883889

3889-
The optional ``moveable_value_debuginfo`` attribute specifies that when
3890-
emitting debug info, the code generator can not assume that the value in the
3891-
alloc_stack can be semantically valid over the entire function frame when
3892-
emitting debug info.
3890+
The optional ``moveable_value_debuginfo`` attribute specifies that when emitting
3891+
debug info, the code generator can not assume that the value in the alloc_stack
3892+
can be semantically valid over the entire function frame when emitting debug
3893+
info. NOTE: This is implicitly set to true if the alloc_stack's type is
3894+
noncopyable. This is just done to make SIL less verbose.
38933895

38943896
alloc_global
38953897
````````````
@@ -4163,7 +4165,10 @@ debug_value
41634165

41644166
::
41654167

4166-
sil-instruction ::= debug_value '[poison]'? '[moveable_value_debuginfo]'? '[trace]'? sil-operand (',' debug-var-attr)* advanced-debug-var-attr* (',' 'expr' debug-info-expr)?
4168+
sil-instruction ::= debug_value sil-debug-value-option* sil-operand (',' debug-var-attr)* advanced-debug-var-attr* (',' 'expr' debug-info-expr)?
4169+
sil-debug-value-option ::= [poison]
4170+
sil-debug-value-option ::= [moveable_value_debuginfo]
4171+
sil-debug-value-option ::= [trace]
41674172

41684173
debug_value %1 : $Int
41694174

@@ -4172,10 +4177,12 @@ specified operand. The declaration in question is identified by either the
41724177
SILLocation attached to the debug_value instruction or the SILLocation specified
41734178
in the advanced debug variable attributes.
41744179

4175-
If the '[moveable_value_debuginfo]' flag is set, then one knows that the debug_value's operand is
4176-
moved at some point of the program, so one can not model the debug_value using
4177-
constructs that assume that the value is live for the entire function (e.x.:
4178-
llvm.dbg.declare).
4180+
If the ``moveable_value_debuginfo`` flag is set, then one knows that the
4181+
debug_value's operand is moved at some point of the program, so one can not
4182+
model the debug_value using constructs that assume that the value is live for
4183+
the entire function (e.x.: llvm.dbg.declare). NOTE: This is implicitly set to
4184+
true if the alloc_stack's type is noncopyable. This is just done to make SIL
4185+
less verbose.
41794186

41804187
::
41814188

include/swift/SIL/DebugUtils.h

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -492,10 +492,7 @@ struct DebugVarCarryingInst : VarDeclCarryingInst {
492492
///
493493
/// For a debug_value, we just return the actual operand, otherwise we return
494494
/// the pointer address.
495-
///
496-
/// If we have an alloc_box, we return a new project_box. This is the only
497-
/// case where a SILBuilder is required.
498-
SILValue getOperandForDebugValueClone(SILBuilder *builder = nullptr) const {
495+
SILValue getOperandForDebugValueClone() const {
499496
switch (getKind()) {
500497
case Kind::Invalid:
501498
llvm_unreachable("Invalid?!");
@@ -504,10 +501,7 @@ struct DebugVarCarryingInst : VarDeclCarryingInst {
504501
case Kind::AllocStack:
505502
return cast<AllocStackInst>(**this);
506503
case Kind::AllocBox:
507-
assert(builder);
508-
return builder->createProjectBox(
509-
RegularLocation::getAutoGeneratedLocation(),
510-
cast<AllocBoxInst>(**this), 0);
504+
return cast<AllocBoxInst>(**this);
511505
}
512506
}
513507

lib/SIL/IR/SILInstructions.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ AllocStackInst::AllocStackInst(SILDebugLocation Loc, SILType elementType,
241241
sharedUInt8().AllocStackInst.dynamicLifetime = hasDynamicLifetime;
242242
sharedUInt8().AllocStackInst.lexical = isLexical;
243243
sharedUInt8().AllocStackInst.usesMoveableValueDebugInfo =
244-
usesMoveableValueDebugInfo;
244+
usesMoveableValueDebugInfo || elementType.isMoveOnly();
245245
sharedUInt32().AllocStackInst.numOperands = TypeDependentOperands.size();
246246

247247
// VarInfo must be initialized after
@@ -382,6 +382,11 @@ AllocBoxInst::AllocBoxInst(SILDebugLocation Loc, CanSILBoxType BoxType,
382382
VarInfo(Var, getTrailingObjects<char>()) {
383383
sharedUInt8().AllocBoxInst.dynamicLifetime = hasDynamicLifetime;
384384
sharedUInt8().AllocBoxInst.reflection = reflection;
385+
386+
// If we have a noncopyable type, always set uses mvoeable value debug info.
387+
usesMoveableValueDebugInfo |=
388+
BoxType->getLayout()->getFields()[0].getObjectType().isMoveOnly();
389+
385390
sharedUInt8().AllocBoxInst.usesMoveableValueDebugInfo =
386391
usesMoveableValueDebugInfo;
387392
}
@@ -409,7 +414,7 @@ SILType AllocBoxInst::getAddressType() const {
409414

410415
DebugValueInst::DebugValueInst(SILDebugLocation DebugLoc, SILValue Operand,
411416
SILDebugVariable Var, bool poisonRefs,
412-
bool wasMoved, bool trace)
417+
bool usesMoveableValueDebugInfo, bool trace)
413418
: UnaryInstructionBase(DebugLoc, Operand),
414419
SILDebugVariableSupplement(Var.DIExpr.getNumElements(),
415420
Var.Type.has_value(), Var.Loc.has_value(),
@@ -421,7 +426,7 @@ DebugValueInst::DebugValueInst(SILDebugLocation DebugLoc, SILValue Operand,
421426
if (auto *VD = DebugLoc.getLocation().getAsASTNode<VarDecl>())
422427
VarInfo.setImplicit(VD->isImplicit() || VarInfo.isImplicit());
423428
setPoisonRefs(poisonRefs);
424-
if (wasMoved)
429+
if (usesMoveableValueDebugInfo || Operand->getType().isMoveOnly())
425430
setUsesMoveableValueDebugInfo();
426431
setTrace(trace);
427432
}

lib/SIL/IR/SILPrinter.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1393,7 +1393,7 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
13931393
*this << "[dynamic_lifetime] ";
13941394
if (AVI->isLexical())
13951395
*this << "[lexical] ";
1396-
if (AVI->getUsesMoveableValueDebugInfo())
1396+
if (AVI->getUsesMoveableValueDebugInfo() && !AVI->getType().isMoveOnly())
13971397
*this << "[moveable_value_debuginfo] ";
13981398
*this << AVI->getElementType();
13991399
printDebugVar(AVI->getVarInfo(),
@@ -1435,7 +1435,8 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
14351435
*this << "[reflection] ";
14361436
}
14371437

1438-
if (ABI->getUsesMoveableValueDebugInfo()) {
1438+
if (ABI->getUsesMoveableValueDebugInfo() &&
1439+
!ABI->getAddressType().isMoveOnly()) {
14391440
*this << "[moveable_value_debuginfo] ";
14401441
}
14411442

@@ -1769,7 +1770,8 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
17691770
void visitDebugValueInst(DebugValueInst *DVI) {
17701771
if (DVI->poisonRefs())
17711772
*this << "[poison] ";
1772-
if (DVI->getUsesMoveableValueDebugInfo())
1773+
if (DVI->getUsesMoveableValueDebugInfo() &&
1774+
!DVI->getOperand()->getType().isMoveOnly())
17731775
*this << "[moveable_value_debuginfo] ";
17741776
if (DVI->hasTrace())
17751777
*this << "[trace] ";

lib/SIL/Parser/ParseSIL.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2858,6 +2858,10 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
28582858
return true;
28592859
if (parseSILDebugLocation(InstLoc, B))
28602860
return true;
2861+
2862+
if (Ty.isMoveOnly())
2863+
usesMoveableValueDebugInfo = true;
2864+
28612865
ResultVal = B.createAllocBox(InstLoc, Ty.castTo<SILBoxType>(), VarInfo,
28622866
hasDynamicLifetime, hasReflection,
28632867
usesMoveableValueDebugInfo);
@@ -3612,6 +3616,10 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
36123616
return true;
36133617
if (Val->getType().isAddress())
36143618
assert(!poisonRefs && "debug_value w/ address value does not support poison");
3619+
3620+
if (Val->getType().isMoveOnly())
3621+
usesMoveableValueDebugInfo = true;
3622+
36153623
ResultVal = B.createDebugValue(InstLoc, Val, VarInfo, poisonRefs,
36163624
usesMoveableValueDebugInfo, hasTrace);
36173625
break;
@@ -4715,6 +4723,10 @@ bool SILParser::parseSpecificSILInstruction(SILBuilder &B,
47154723
SILDebugVariable VarInfo;
47164724
if (parseSILDebugVar(VarInfo) || parseSILDebugLocation(InstLoc, B))
47174725
return true;
4726+
4727+
if (Ty.isMoveOnly())
4728+
usesMoveableValueDebugInfo = true;
4729+
47184730
// It doesn't make sense to attach a debug var info if the name is empty
47194731
if (VarInfo.Name.size())
47204732
ResultVal = B.createAllocStack(InstLoc, Ty, VarInfo, hasDynamicLifetime,

0 commit comments

Comments
 (0)