Skip to content

Commit 58e832a

Browse files
committed
Emit updated debug info when inout parameters are consumed and reinitialized.
Change SILGen to emit the `debug_value` instruction using the original inout parameter address, instead of the `mark_must_check` inserted for move-only parameters, because code in the MoveOnlyAddressChecker did not expect to find the debug_value anywhere but on the original address. Update move-only diagnostics so that they pick up the declaration name for a memory location from any debug_value instruction if there are more than one. rdar://109740281
1 parent a63c8d5 commit 58e832a

File tree

5 files changed

+62
-16
lines changed

5 files changed

+62
-16
lines changed

include/swift/SIL/DebugUtils.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,16 @@ inline Operand *getSingleDebugUse(SILValue value) {
194194
return *ii;
195195
}
196196

197+
/// If \p value has any debug user(s), return the operand associated with some
198+
/// use. Otherwise, returns nullptr.
199+
inline Operand *getAnyDebugUse(SILValue value) {
200+
auto range = getDebugUses(value);
201+
auto ii = range.begin(), ie = range.end();
202+
if (ii == ie)
203+
return nullptr;
204+
return *ii;
205+
}
206+
197207
/// Erases the instruction \p I from it's parent block and deletes it, including
198208
/// all debug instructions which use \p I.
199209
/// Precondition: The instruction may only have debug instructions as uses.

lib/SILGen/SILGenProlog.cpp

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -784,6 +784,7 @@ class ArgumentInitHelper {
784784
return;
785785
}
786786

787+
SILValue debugOperand = value;
787788
if (value->getType().isMoveOnly()) {
788789
switch (pd->getValueOwnership()) {
789790
case ValueOwnership::Default:
@@ -799,11 +800,14 @@ class ArgumentInitHelper {
799800
"Should have inserted mark must check inst in EmitBBArgs");
800801
}
801802
break;
802-
case ValueOwnership::InOut:
803-
assert(isa<MarkMustCheckInst>(value) &&
804-
"Expected mark must check inst with inout to be handled in "
805-
"emitBBArgs earlier");
803+
case ValueOwnership::InOut: {
804+
assert(isa<MarkMustCheckInst>(value)
805+
&& "Expected mark must check inst with inout to be handled in "
806+
"emitBBArgs earlier");
807+
auto mark = cast<MarkMustCheckInst>(value);
808+
debugOperand = mark->getOperand();
806809
break;
810+
}
807811
case ValueOwnership::Owned:
808812
value = SGF.B.createMarkMustCheckInst(
809813
loc, value, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
@@ -815,7 +819,17 @@ class ArgumentInitHelper {
815819
}
816820
}
817821

818-
SGF.B.createDebugValueAddr(loc, value, varinfo);
822+
DebugValueInst *debugInst
823+
= SGF.B.createDebugValueAddr(loc, debugOperand, varinfo);
824+
825+
if (value != debugOperand) {
826+
if (auto valueInst = dyn_cast<MarkMustCheckInst>(value)) {
827+
// Move the debug instruction outside of any marker instruction that might
828+
// have been applied to the value, so that analysis doesn't move the
829+
// debug_value anywhere it shouldn't be.
830+
debugInst->moveBefore(valueInst);
831+
}
832+
}
819833
SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(value);
820834
}
821835

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ static bool isReinitToInitConvertibleInst(SILInstruction *memInst) {
385385

386386
static void insertDebugValueBefore(SILInstruction *insertPt,
387387
DebugVarCarryingInst debugVar,
388-
SILValue operand) {
388+
llvm::function_ref<SILValue ()> operand) {
389389
if (!debugVar) {
390390
return;
391391
}
@@ -395,7 +395,7 @@ static void insertDebugValueBefore(SILInstruction *insertPt,
395395
}
396396
SILBuilderWithScope debugInfoBuilder(insertPt);
397397
debugInfoBuilder.setCurrentDebugScope(debugVar->getDebugScope());
398-
debugInfoBuilder.createDebugValue(debugVar->getLoc(), operand,
398+
debugInfoBuilder.createDebugValue(debugVar->getLoc(), operand(),
399399
*varInfo, false, true);
400400
}
401401

@@ -423,7 +423,7 @@ static void convertMemoryReinitToInitForm(SILInstruction *memInst,
423423
// Insert a new debug_value instruction after the reinitialization, so that
424424
// the debugger knows that the variable is in a usable form again.
425425
insertDebugValueBefore(memInst->getNextInstruction(), debugVar,
426-
stripAccessMarkers(dest));
426+
[&]{ return debugVar.getOperandForDebugValueClone(); });
427427
}
428428

429429
static bool memInstMustConsume(Operand *memOper) {
@@ -2333,12 +2333,10 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
23332333
// referring to the same debug variable as the original definition, we have to
23342334
// use the same debug scope and location as the original debug var.
23352335
auto insertUndefDebugValue = [&debugVar](SILInstruction *insertPt) {
2336-
if (!debugVar) {
2337-
return;
2338-
}
2339-
insertDebugValueBefore(insertPt, debugVar,
2340-
SILUndef::get(debugVar.getOperandForDebugValueClone()->getType(),
2341-
insertPt->getModule()));
2336+
insertDebugValueBefore(insertPt, debugVar, [&]{
2337+
return SILUndef::get(debugVar.getOperandForDebugValueClone()->getType(),
2338+
insertPt->getModule());
2339+
});
23422340
};
23432341

23442342
for (auto &pair : boundary.getLastUsers()) {

lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ static void getVariableNameForValue(SILValue value2,
7777
SmallString<64> &resultingString) {
7878
// Before we do anything, lets see if we have an exact debug_value on our
7979
// mmci. In such a case, we can end early and are done.
80-
if (auto *use = getSingleDebugUse(value2)) {
80+
if (auto *use = getAnyDebugUse(value2)) {
8181
if (auto debugVar = DebugVarCarryingInst(use->getUser())) {
8282
assert(debugVar.getKind() == DebugVarCarryingInst::Kind::DebugValue);
8383
resultingString += debugVar.getName();
@@ -112,7 +112,7 @@ static void getVariableNameForValue(SILValue value2,
112112

113113
// If we do not do an exact match, see if we can find a debug_var inst. If
114114
// we do, we always break since we have a root value.
115-
if (auto *use = getSingleDebugUse(searchValue)) {
115+
if (auto *use = getAnyDebugUse(searchValue)) {
116116
if (auto debugVar = DebugVarCarryingInst(use->getUser())) {
117117
assert(debugVar.getKind() == DebugVarCarryingInst::Kind::DebugValue);
118118
variableNamePath.push_back(use->getUser());

test/SILOptimizer/moveonly_debug_info_reinit.swift

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,27 @@ func bar(_ x: consuming Foo, y: consuming Foo, z: consuming Foo) {
3939
// CHECK: debug_value undef : $*Foo, var, name "x"
4040
use(&x)
4141
}
42+
43+
// CHECK-LABEL: sil {{.*}} @${{.*}}10inoutParam
44+
func inoutParam(_ x: inout Foo, y: consuming Foo, z: consuming Foo) {
45+
// CHECK: debug_value [[X:%[0-9]+]] : $*Foo, var, name "x"
46+
47+
// CHECK: [[USE:%.*]] = function_ref @use
48+
// CHECK: apply [[USE]]
49+
// CHECK: debug_value undef : $*Foo, var, name "x"
50+
use(&x)
51+
let _ = x
52+
53+
// CHECK: debug_value undef : $*Foo, var, name "y"
54+
// CHECK: debug_value [[X]] : $*Foo, var, name "x"
55+
x = y
56+
// CHECK: [[USE:%.*]] = function_ref @use
57+
// CHECK: apply [[USE]]
58+
// CHECK: debug_value undef : $*Foo, var, name "x"
59+
use(&x)
60+
let _ = x
61+
62+
// CHECK: debug_value undef : $*Foo, var, name "z"
63+
// CHECK: debug_value [[X]] : $*Foo, var, name "x"
64+
x = z
65+
}

0 commit comments

Comments
 (0)