Skip to content

Commit edbe4db

Browse files
gottesmmgottesmm
committed
[move-function] Add initial debug info support to move checker passes.
Specifically in this commit we: 1. Add support to the move checkers for marking alloc_stack that are moved or the debug_value of values that are moved with the [moved] flag. In a subsequent PR, I am going to add support in IRGen for emitting llvm.dbg.addr instead of llvm.dbg.declare for such variables. This will ensure that debug info for values that aren't moved get the same codegen. 2. I changed the move checkers to begin emitting debug_value undef at the move site. This ensures that after that point the moved value will be unavailable in the debugger. 3. I also found a small bug that was around terminators that was exposed by some of my tests. What is not in this patch: 1. IRGen part of this patch. 2. DebugInfoCanonicalization that pushes debug_info into coroutine func lets to avoid issues with Swift's coroutine splitting at the LLVM level. rdar://85020571
1 parent 375ce35 commit edbe4db

File tree

6 files changed

+252
-28
lines changed

6 files changed

+252
-28
lines changed

‎include/swift/SIL/DebugUtils.h‎

Lines changed: 48 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -325,34 +325,61 @@ struct DebugVarCarryingInst {
325325
llvm_unreachable("Not implemented");
326326
}
327327
}
328-
};
329328

330-
/// Attempt to discover a StringRef varName for the value \p value. If we fail,
331-
/// we return the name "unknown".
332-
inline StringRef getDebugVarName(SILValue value) {
333-
if (auto *asi = dyn_cast<AllocStackInst>(value)) {
334-
DebugVarCarryingInst debugVar(asi);
335-
if (auto varInfo = debugVar.getVarInfo()) {
336-
return varInfo->Name;
337-
} else {
338-
if (auto *decl = debugVar.getDecl()) {
339-
return decl->getBaseName().userFacingName();
340-
}
329+
void markAsMoved() {
330+
switch (kind) {
331+
case Kind::Invalid:
332+
llvm_unreachable("Invalid?!");
333+
case Kind::DebugValue:
334+
cast<DebugValueInst>(inst)->markAsMoved();
335+
break;
336+
case Kind::AllocStack:
337+
cast<AllocStackInst>(inst)->markAsMoved();
338+
break;
339+
case Kind::AllocBox:
340+
llvm_unreachable("Not implemented");
341341
}
342342
}
343343

344-
StringRef varName = "unknown";
345-
if (auto *use = getSingleDebugUse(value)) {
346-
DebugVarCarryingInst debugVar(use->getUser());
347-
if (auto varInfo = debugVar.getVarInfo()) {
344+
/// If \p value is an alloc_stack, alloc_box use that. Otherwise, see if \p
345+
/// value has a single debug user, return that. Otherwise return the invalid
346+
/// DebugVarCarryingInst.
347+
static DebugVarCarryingInst getFromValue(SILValue value);
348+
349+
/// Take in \p inst, a potentially invalid DebugVarCarryingInst, and returns a
350+
/// name for it. If we have an invalid value or don't find var info or a decl,
351+
/// return "unknown".
352+
///
353+
/// The reason this isn't a method is that in all the other parts of
354+
/// DebugVarCarryingInst, we use Invalid to signal early error.
355+
static StringRef getName(DebugVarCarryingInst inst) {
356+
if (!inst)
357+
return "unknown";
358+
StringRef varName = "unknown";
359+
if (auto varInfo = inst.getVarInfo()) {
348360
varName = varInfo->Name;
349-
} else {
350-
if (auto *decl = debugVar.getDecl()) {
351-
varName = decl->getBaseName().userFacingName();
352-
}
361+
} else if (auto *decl = inst.getDecl()) {
362+
varName = decl->getBaseName().userFacingName();
353363
}
364+
return varName;
354365
}
355-
return varName;
366+
};
367+
368+
inline DebugVarCarryingInst DebugVarCarryingInst::getFromValue(SILValue value) {
369+
if (isa<AllocStackInst>(value) || isa<AllocBoxInst>(value))
370+
return DebugVarCarryingInst(cast<SingleValueInstruction>(value));
371+
372+
if (auto *use = getSingleDebugUse(value))
373+
return DebugVarCarryingInst(use->getUser());
374+
375+
return DebugVarCarryingInst();
376+
}
377+
378+
/// Attempt to discover a StringRef varName for the value \p value. If we fail,
379+
/// we return the name "unknown".
380+
inline StringRef getDebugVarName(SILValue value) {
381+
auto inst = DebugVarCarryingInst::getFromValue(value);
382+
return DebugVarCarryingInst::getName(inst);
356383
}
357384

358385
} // end namespace swift

‎lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp‎

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1494,8 +1494,13 @@ bool DataflowState::cleanupAllDestroyAddr(
14941494
"we processed takes of the given move.\n");
14951495
// Insert a destroy_addr here since the block isn't reachable from any of
14961496
// our moves.
1497-
SILBuilderWithScope builder(
1498-
std::prev(next->getTerminator()->getIterator()));
1497+
SILBasicBlock::iterator iter;
1498+
if (!isa<TermInst>(next->front())) {
1499+
iter = std::prev(next->getTerminator()->getIterator());
1500+
} else {
1501+
iter = next->begin();
1502+
}
1503+
SILBuilderWithScope builder(iter);
14991504
auto *dvi = builder.createDestroyAddr(
15001505
RegularLocation::getAutoGeneratedLocation(), address);
15011506
useState.destroys.insert(dvi);
@@ -1755,12 +1760,26 @@ bool DataflowState::process(
17551760
} else {
17561761
builder.createCopyAddr(mvi->getLoc(), mvi->getSrc(), mvi->getDest(),
17571762
IsTake, IsInitialization);
1763+
1764+
// Now that we have processed all of our mark_moves, eliminate all of the
1765+
// destroy_addr and set our debug value as being moved.
1766+
if (auto debug = DebugVarCarryingInst::getFromValue(address)) {
1767+
debug.markAsMoved();
1768+
if (auto varInfo = debug.getVarInfo()) {
1769+
builder.createDebugValue(
1770+
mvi->getLoc(),
1771+
SILUndef::get(address->getType(), builder.getModule()), *varInfo,
1772+
false /*poison*/, true /*was moved*/);
1773+
}
1774+
}
1775+
17581776
// Flush our SetVector into the visitedByNewMove.
17591777
for (auto *block : visitedBlocks) {
17601778
blocksVisitedWhenProcessingNewTakes.insert(block);
17611779
}
17621780
convertedMarkMoveToTake = true;
17631781
}
1782+
17641783
mvi->eraseFromParent();
17651784
madeChange = true;
17661785
}
@@ -1890,6 +1909,18 @@ static bool performSingleBasicBlockAnalysis(DataflowState &dataflowState,
18901909
SILBuilderWithScope builder(mvi);
18911910
builder.createCopyAddr(mvi->getLoc(), mvi->getSrc(), mvi->getDest(), IsTake,
18921911
IsInitialization);
1912+
// Also, mark the alloc_stack as being moved at some point.
1913+
if (auto debug = DebugVarCarryingInst::getFromValue(address)) {
1914+
if (auto varInfo = debug.getVarInfo()) {
1915+
builder.createDebugValue(
1916+
mvi->getLoc(),
1917+
SILUndef::get(address->getType(), builder.getModule()), *varInfo,
1918+
false,
1919+
/*was moved*/ true);
1920+
}
1921+
debug.markAsMoved();
1922+
}
1923+
18931924
useState.destroys.erase(dvi);
18941925
mvi->eraseFromParent();
18951926
dvi->eraseFromParent();
@@ -1981,11 +2012,22 @@ static bool performSingleBasicBlockAnalysis(DataflowState &dataflowState,
19812012
assert(!interestingUse);
19822013
assert(interestingUser);
19832014

2015+
// If we have a reinit, then we have a successful move.
19842016
convertMemoryReinitToInitForm(interestingUser);
19852017
useState.reinits.erase(interestingUser);
19862018
SILBuilderWithScope builder(mvi);
19872019
builder.createCopyAddr(mvi->getLoc(), mvi->getSrc(), mvi->getDest(), IsTake,
19882020
IsInitialization);
2021+
if (auto debug = DebugVarCarryingInst::getFromValue(address)) {
2022+
if (auto varInfo = debug.getVarInfo()) {
2023+
builder.createDebugValue(
2024+
mvi->getLoc(),
2025+
SILUndef::get(address->getType(), builder.getModule()), *varInfo,
2026+
false,
2027+
/*was moved*/ true);
2028+
}
2029+
debug.markAsMoved();
2030+
}
19892031
mvi->eraseFromParent();
19902032
return false;
19912033
}
@@ -2015,6 +2057,16 @@ static bool performSingleBasicBlockAnalysis(DataflowState &dataflowState,
20152057
LLVM_DEBUG(llvm::dbgs() << "Found apply site to clone: " << **fas);
20162058
LLVM_DEBUG(llvm::dbgs() << "BitVector: ";
20172059
dumpBitVector(llvm::dbgs(), bitVector); llvm::dbgs() << '\n');
2060+
if (auto debug = DebugVarCarryingInst::getFromValue(address)) {
2061+
if (auto varInfo = debug.getVarInfo()) {
2062+
builder.createDebugValue(
2063+
mvi->getLoc(),
2064+
SILUndef::get(address->getType(), builder.getModule()), *varInfo,
2065+
false,
2066+
/*was moved*/ true);
2067+
}
2068+
debug.markAsMoved();
2069+
}
20182070
mvi->eraseFromParent();
20192071
return false;
20202072
}

‎lib/SILOptimizer/Mandatory/MoveKillsCopyableValuesChecker.cpp‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "swift/SIL/InstructionUtils.h"
2121
#include "swift/SIL/OwnershipUtils.h"
2222
#include "swift/SIL/SILArgument.h"
23+
#include "swift/SIL/SILBuilder.h"
2324
#include "swift/SIL/SILFunction.h"
2425
#include "swift/SIL/SILInstruction.h"
2526
#include "swift/SIL/SILUndef.h"
@@ -355,16 +356,12 @@ bool MoveKillsCopyableValuesChecker::check() {
355356
LLVM_DEBUG(llvm::dbgs() << "Visiting Function: " << fn->getName() << "\n");
356357
auto valuesToProcess =
357358
llvm::makeArrayRef(valuesToCheck.begin(), valuesToCheck.end());
358-
359+
auto &mod = fn->getModule();
359360
while (!valuesToProcess.empty()) {
360361
auto lexicalValue = valuesToProcess.front();
361362
valuesToProcess = valuesToProcess.drop_front(1);
362363
LLVM_DEBUG(llvm::dbgs() << "Visiting: " << *lexicalValue);
363364

364-
// Before we do anything, see if we can find a name for our value. We do
365-
// this early since we need this for all of our diagnostics below.
366-
StringRef varName = getDebugVarName(lexicalValue);
367-
368365
// Then compute liveness.
369366
SWIFT_DEFER { livenessInfo.clear(); };
370367
livenessInfo.initDef(lexicalValue);
@@ -377,6 +374,9 @@ bool MoveKillsCopyableValuesChecker::check() {
377374

378375
// Then look at all of our found consuming uses. See if any of these are
379376
// _move that are within the boundary.
377+
bool foundMove = false;
378+
auto dbgVarInfo = DebugVarCarryingInst::getFromValue(lexicalValue);
379+
StringRef varName = DebugVarCarryingInst::getName(dbgVarInfo);
380380
for (auto *use : livenessInfo.consumingUse) {
381381
if (auto *mvi = dyn_cast<MoveValueInst>(use->getUser())) {
382382
// Only emit diagnostics if our move value allows us to.
@@ -394,9 +394,26 @@ bool MoveKillsCopyableValuesChecker::check() {
394394
emitDiagnosticForMove(lexicalValue, varName, mvi);
395395
} else {
396396
LLVM_DEBUG(llvm::dbgs() << " WithinBoundary: No!\n");
397+
if (auto varInfo = dbgVarInfo.getVarInfo()) {
398+
auto *next = mvi->getNextInstruction();
399+
SILBuilderWithScope builder(next);
400+
// Use an autogenerated location to ensure that if we are next to a
401+
// terminator, we don't assert.
402+
builder.createDebugValue(
403+
RegularLocation::getAutoGeneratedLocation(),
404+
SILUndef::get(mvi->getOperand()->getType(), mod), *varInfo,
405+
false /*poison*/, true /*moved*/);
406+
}
397407
}
408+
foundMove = true;
398409
}
399410
}
411+
412+
// If we found a move, mark our debug var inst as having a moved value. This
413+
// ensures we emit llvm.dbg.addr instead of llvm.dbg.declare in IRGen.
414+
if (foundMove) {
415+
dbgVarInfo.markAsMoved();
416+
}
400417
}
401418

402419
return false;

‎test/SILGen/moveonly_builtin.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class Klass {}
3131
// CHECK-SIL-NEXT: debug_value
3232
// CHECK-SIL-NEXT: strong_retain
3333
// CHECK-SIL-NEXT: move_value
34+
// CHECK-SIL-NEXT: debug_value [moved] undef
3435
// CHECK-SIL-NEXT: tuple
3536
// CHECK-SIL-NEXT: tuple
3637
// CHECK-SIL-NEXT: strong_release
@@ -64,6 +65,7 @@ func useMove(_ k: __owned Klass) -> Klass {
6465
// CHECK-SIL-NEXT: debug_value
6566
// CHECK-SIL-NEXT: strong_retain
6667
// CHECK-SIL-NEXT: move_value
68+
// CHECK-SIL-NEXT: debug_value [moved] undef
6769
// CHECK-SIL-NEXT: tuple
6870
// CHECK-SIL-NEXT: tuple
6971
// CHECK-SIL-NEXT: strong_release

‎test/SILOptimizer/move_function_kills_addresses_dbginfo.sil‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
// RUN: %target-sil-opt %s -sil-move-kills-copyable-addresses-checker | %FileCheck %s
2+
3+
// REQUIRES: optimized_stdlib
4+
5+
sil_stage raw
6+
7+
import Builtin
8+
9+
// Make sure that when we process a move_addr on an alloc_stack/debug_value that
10+
// we properly put the [moved] marker on them.
11+
12+
// CHECK-LABEL: sil [ossa] @singleBlock : $@convention(thin) (@owned Builtin.NativeObject) -> () {
13+
// CHECK: [[SRC_ADDR:%.*]] = alloc_stack [lexical] [moved] $Builtin.NativeObject, let, name "[[VAR_NAME:.*]]"
14+
// CHECK: [[DEST_ADDR:%.*]] = alloc_stack $Builtin.NativeObject
15+
// CHECK: copy_addr [take] [[SRC_ADDR]] to [initialization] [[DEST_ADDR]]
16+
// CHECK-NEXT: debug_value [moved] undef
17+
// CHECK: } // end sil function 'singleBlock'
18+
sil [ossa] @singleBlock : $@convention(thin) (@owned Builtin.NativeObject) -> () {
19+
bb0(%0 : @owned $Builtin.NativeObject):
20+
%1 = alloc_stack [lexical] $Builtin.NativeObject, let, name "myName"
21+
%2 = copy_value %0 : $Builtin.NativeObject
22+
store %2 to [init] %1 : $*Builtin.NativeObject
23+
24+
%dest = alloc_stack $Builtin.NativeObject
25+
mark_unresolved_move_addr %1 to %dest : $*Builtin.NativeObject
26+
27+
destroy_addr %dest : $*Builtin.NativeObject
28+
dealloc_stack %dest : $*Builtin.NativeObject
29+
30+
destroy_addr %1 : $*Builtin.NativeObject
31+
dealloc_stack %1 : $*Builtin.NativeObject
32+
33+
destroy_value %0 : $Builtin.NativeObject
34+
%9999 = tuple()
35+
return %9999 : $()
36+
}
37+
38+
// CHECK-LABEL: sil [ossa] @multipleBlock : $@convention(thin) (@owned Builtin.NativeObject) -> () {
39+
// CHECK: bb0(
40+
// CHECK: [[SRC_ADDR:%.*]] = alloc_stack [lexical] [moved] $Builtin.NativeObject, let, name "[[VAR_NAME:.*]]"
41+
// CHECK: [[DEST_ADDR:%.*]] = alloc_stack $Builtin.NativeObject
42+
// CHECK: cond_br undef, bb1, bb2
43+
//
44+
// CHECK: bb1:
45+
// CHECK: copy_addr [take] [[SRC_ADDR]] to [initialization] [[DEST_ADDR]]
46+
// CHECK: debug_value [moved] undef : $*Builtin.NativeObject, let, name "[[VAR_NAME]]"
47+
// CHECK: br bb3
48+
//
49+
// CHECK: } // end sil function 'multipleBlock'
50+
sil [ossa] @multipleBlock : $@convention(thin) (@owned Builtin.NativeObject) -> () {
51+
bb0(%0 : @owned $Builtin.NativeObject):
52+
%1 = alloc_stack [lexical] $Builtin.NativeObject, let, name "myName"
53+
%2 = copy_value %0 : $Builtin.NativeObject
54+
store %2 to [init] %1 : $*Builtin.NativeObject
55+
%dest = alloc_stack $Builtin.NativeObject
56+
cond_br undef, bb1, bb2
57+
58+
bb1:
59+
mark_unresolved_move_addr %1 to %dest : $*Builtin.NativeObject
60+
destroy_addr %dest : $*Builtin.NativeObject
61+
br bb3
62+
63+
bb2:
64+
br bb3
65+
66+
bb3:
67+
dealloc_stack %dest : $*Builtin.NativeObject
68+
destroy_addr %1 : $*Builtin.NativeObject
69+
dealloc_stack %1 : $*Builtin.NativeObject
70+
destroy_value %0 : $Builtin.NativeObject
71+
%9999 = tuple()
72+
return %9999 : $()
73+
}

‎test/SILOptimizer/move_function_kills_values_dbginfo.sil‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// RUN: %target-sil-opt %s -sil-move-kills-copyable-values-checker | %FileCheck %s
2+
3+
// REQUIRES: optimized_stdlib
4+
5+
sil_stage raw
6+
7+
import Builtin
8+
9+
// Make sure that when we process a move_addr on an alloc_stack/debug_value that
10+
// we properly put the [moved] marker on them.
11+
12+
// CHECK-LABEL: sil [ossa] @singleBlock : $@convention(thin) (@owned Builtin.NativeObject) -> () {
13+
// CHECK: bb0([[ARG:%.*]] :
14+
// CHECK: debug_value [moved] [[ARG]] : $Builtin.NativeObject, let, name "myName"
15+
// CHECK: [[MOVED_VAL:%.*]] = move_value [[ARG]]
16+
// CHECK: debug_value [moved] undef : $Builtin.NativeObject, let, name "myName"
17+
// CHECK: } // end sil function 'singleBlock'
18+
sil [ossa] @singleBlock : $@convention(thin) (@owned Builtin.NativeObject) -> () {
19+
bb0(%0 : @owned $Builtin.NativeObject):
20+
debug_value %0 : $Builtin.NativeObject, let, name "myName"
21+
%1 = move_value [allows_diagnostics] %0 : $Builtin.NativeObject
22+
destroy_value %1 : $Builtin.NativeObject
23+
%9999 = tuple()
24+
return %9999 : $()
25+
}
26+
27+
// CHECK-LABEL: sil [ossa] @multipleBlock : $@convention(thin) (@owned Builtin.NativeObject) -> () {
28+
// CHECK: bb0([[ARG:%.*]] :
29+
// CHECK: debug_value [moved] [[ARG]]
30+
// CHECK: cond_br undef, bb1, bb2
31+
//
32+
// CHECK: bb1:
33+
// CHECK: [[MV:%.*]] = move_value [[ARG]]
34+
// CHECK: debug_value [moved] undef : $Builtin.NativeObject, let, name "myName"
35+
// CHECK: } // end sil function 'multipleBlock'
36+
sil [ossa] @multipleBlock : $@convention(thin) (@owned Builtin.NativeObject) -> () {
37+
bb0(%0 : @owned $Builtin.NativeObject):
38+
debug_value %0 : $Builtin.NativeObject, let, name "myName"
39+
cond_br undef, bb1, bb2
40+
41+
bb1:
42+
%1 = move_value [allows_diagnostics] %0 : $Builtin.NativeObject
43+
destroy_value %1 : $Builtin.NativeObject
44+
br bb3
45+
46+
bb2:
47+
destroy_value %0 : $Builtin.NativeObject
48+
br bb3
49+
50+
bb3:
51+
%9999 = tuple()
52+
return %9999 : $()
53+
}

0 commit comments

Comments
 (0)