Skip to content

[5.9] Emit updated debug info when inout parameters are consumed and reinitialized. #66143

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
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
10 changes: 10 additions & 0 deletions include/swift/SIL/DebugUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,16 @@ inline Operand *getSingleDebugUse(SILValue value) {
return *ii;
}

/// If \p value has any debug user(s), return the operand associated with some
/// use. Otherwise, returns nullptr.
inline Operand *getAnyDebugUse(SILValue value) {
auto range = getDebugUses(value);
auto ii = range.begin(), ie = range.end();
if (ii == ie)
return nullptr;
return *ii;
}

/// Erases the instruction \p I from it's parent block and deletes it, including
/// all debug instructions which use \p I.
/// Precondition: The instruction may only have debug instructions as uses.
Expand Down
24 changes: 19 additions & 5 deletions lib/SILGen/SILGenProlog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -784,6 +784,7 @@ class ArgumentInitHelper {
return;
}

SILValue debugOperand = value;
if (value->getType().isMoveOnly()) {
switch (pd->getValueOwnership()) {
case ValueOwnership::Default:
Expand All @@ -799,11 +800,14 @@ class ArgumentInitHelper {
"Should have inserted mark must check inst in EmitBBArgs");
}
break;
case ValueOwnership::InOut:
assert(isa<MarkMustCheckInst>(value) &&
"Expected mark must check inst with inout to be handled in "
"emitBBArgs earlier");
case ValueOwnership::InOut: {
assert(isa<MarkMustCheckInst>(value)
&& "Expected mark must check inst with inout to be handled in "
"emitBBArgs earlier");
auto mark = cast<MarkMustCheckInst>(value);
debugOperand = mark->getOperand();
break;
}
case ValueOwnership::Owned:
value = SGF.B.createMarkMustCheckInst(
loc, value, MarkMustCheckInst::CheckKind::ConsumableAndAssignable);
Expand All @@ -815,7 +819,17 @@ class ArgumentInitHelper {
}
}

SGF.B.createDebugValueAddr(loc, value, varinfo);
DebugValueInst *debugInst
= SGF.B.createDebugValueAddr(loc, debugOperand, varinfo);

if (value != debugOperand) {
if (auto valueInst = dyn_cast<MarkMustCheckInst>(value)) {
// Move the debug instruction outside of any marker instruction that might
// have been applied to the value, so that analysis doesn't move the
// debug_value anywhere it shouldn't be.
debugInst->moveBefore(valueInst);
}
}
SGF.VarLocs[pd] = SILGenFunction::VarLoc::get(value);
}

Expand Down
16 changes: 7 additions & 9 deletions lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ static bool isReinitToInitConvertibleInst(SILInstruction *memInst) {

static void insertDebugValueBefore(SILInstruction *insertPt,
DebugVarCarryingInst debugVar,
SILValue operand) {
llvm::function_ref<SILValue ()> operand) {
if (!debugVar) {
return;
}
Expand All @@ -395,7 +395,7 @@ static void insertDebugValueBefore(SILInstruction *insertPt,
}
SILBuilderWithScope debugInfoBuilder(insertPt);
debugInfoBuilder.setCurrentDebugScope(debugVar->getDebugScope());
debugInfoBuilder.createDebugValue(debugVar->getLoc(), operand,
debugInfoBuilder.createDebugValue(debugVar->getLoc(), operand(),
*varInfo, false, true);
}

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

static bool memInstMustConsume(Operand *memOper) {
Expand Down Expand Up @@ -2333,12 +2333,10 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
// referring to the same debug variable as the original definition, we have to
// use the same debug scope and location as the original debug var.
auto insertUndefDebugValue = [&debugVar](SILInstruction *insertPt) {
if (!debugVar) {
return;
}
insertDebugValueBefore(insertPt, debugVar,
SILUndef::get(debugVar.getOperandForDebugValueClone()->getType(),
insertPt->getModule()));
insertDebugValueBefore(insertPt, debugVar, [&]{
return SILUndef::get(debugVar.getOperandForDebugValueClone()->getType(),
insertPt->getModule());
});
};

for (auto &pair : boundary.getLastUsers()) {
Expand Down
4 changes: 2 additions & 2 deletions lib/SILOptimizer/Mandatory/MoveOnlyDiagnostics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ static void getVariableNameForValue(SILValue value2,
SmallString<64> &resultingString) {
// Before we do anything, lets see if we have an exact debug_value on our
// mmci. In such a case, we can end early and are done.
if (auto *use = getSingleDebugUse(value2)) {
if (auto *use = getAnyDebugUse(value2)) {
if (auto debugVar = DebugVarCarryingInst(use->getUser())) {
assert(debugVar.getKind() == DebugVarCarryingInst::Kind::DebugValue);
resultingString += debugVar.getName();
Expand Down Expand Up @@ -112,7 +112,7 @@ static void getVariableNameForValue(SILValue value2,

// If we do not do an exact match, see if we can find a debug_var inst. If
// we do, we always break since we have a root value.
if (auto *use = getSingleDebugUse(searchValue)) {
if (auto *use = getAnyDebugUse(searchValue)) {
if (auto debugVar = DebugVarCarryingInst(use->getUser())) {
assert(debugVar.getKind() == DebugVarCarryingInst::Kind::DebugValue);
variableNamePath.push_back(use->getUser());
Expand Down
24 changes: 24 additions & 0 deletions test/SILOptimizer/moveonly_debug_info_reinit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,27 @@ func bar(_ x: consuming Foo, y: consuming Foo, z: consuming Foo) {
// CHECK: debug_value undef : $*Foo, var, name "x"
use(&x)
}

// CHECK-LABEL: sil {{.*}} @${{.*}}10inoutParam
func inoutParam(_ x: inout Foo, y: consuming Foo, z: consuming Foo) {
// CHECK: debug_value [[X:%[0-9]+]] : $*Foo, var, name "x"

// CHECK: [[USE:%.*]] = function_ref @use
// CHECK: apply [[USE]]
// CHECK: debug_value undef : $*Foo, var, name "x"
use(&x)
let _ = x

// CHECK: debug_value undef : $*Foo, var, name "y"
// CHECK: debug_value [[X]] : $*Foo, var, name "x"
x = y
// CHECK: [[USE:%.*]] = function_ref @use
// CHECK: apply [[USE]]
// CHECK: debug_value undef : $*Foo, var, name "x"
use(&x)
let _ = x

// CHECK: debug_value undef : $*Foo, var, name "z"
// CHECK: debug_value [[X]] : $*Foo, var, name "x"
x = z
}