Skip to content

Commit 504fe52

Browse files
committed
[move-function] When emitting debug_value undef, use the loc of the original DbgVarCarryingInst we found.
LLVM wants the debug_value undef to be /exactly/ the same as the original debug_value including the loc. If we don't match, LLVM won't invalidate the value. Also now that things actually work (at least for copyable lets), lets add some basic tests that debug info works as expected. NOTE: With this patch it seems that address only move is still broken. I am looking into it and will fix it in a forthcoming PR.
1 parent 58e2caf commit 504fe52

File tree

3 files changed

+111
-5
lines changed

3 files changed

+111
-5
lines changed

lib/SILOptimizer/Mandatory/MoveKillsCopyableAddressesChecker.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,7 +1767,7 @@ bool DataflowState::process(
17671767
debug.markAsMoved();
17681768
if (auto varInfo = debug.getVarInfo()) {
17691769
builder.createDebugValue(
1770-
mvi->getLoc(),
1770+
debug.inst->getLoc(),
17711771
SILUndef::get(address->getType(), builder.getModule()), *varInfo,
17721772
false /*poison*/, true /*was moved*/);
17731773
}
@@ -1913,7 +1913,7 @@ static bool performSingleBasicBlockAnalysis(DataflowState &dataflowState,
19131913
if (auto debug = DebugVarCarryingInst::getFromValue(address)) {
19141914
if (auto varInfo = debug.getVarInfo()) {
19151915
builder.createDebugValue(
1916-
mvi->getLoc(),
1916+
debug.inst->getLoc(),
19171917
SILUndef::get(address->getType(), builder.getModule()), *varInfo,
19181918
false,
19191919
/*was moved*/ true);
@@ -2021,7 +2021,7 @@ static bool performSingleBasicBlockAnalysis(DataflowState &dataflowState,
20212021
if (auto debug = DebugVarCarryingInst::getFromValue(address)) {
20222022
if (auto varInfo = debug.getVarInfo()) {
20232023
builder.createDebugValue(
2024-
mvi->getLoc(),
2024+
debug.inst->getLoc(),
20252025
SILUndef::get(address->getType(), builder.getModule()), *varInfo,
20262026
false,
20272027
/*was moved*/ true);
@@ -2060,7 +2060,7 @@ static bool performSingleBasicBlockAnalysis(DataflowState &dataflowState,
20602060
if (auto debug = DebugVarCarryingInst::getFromValue(address)) {
20612061
if (auto varInfo = debug.getVarInfo()) {
20622062
builder.createDebugValue(
2063-
mvi->getLoc(),
2063+
debug.inst->getLoc(),
20642064
SILUndef::get(address->getType(), builder.getModule()), *varInfo,
20652065
false,
20662066
/*was moved*/ true);

lib/SILOptimizer/Mandatory/MoveKillsCopyableValuesChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,7 @@ bool MoveKillsCopyableValuesChecker::check() {
400400
// Use an autogenerated location to ensure that if we are next to a
401401
// terminator, we don't assert.
402402
builder.createDebugValue(
403-
RegularLocation::getAutoGeneratedLocation(),
403+
dbgVarInfo.inst->getLoc(),
404404
SILUndef::get(mvi->getOperand()->getType(), mod), *varInfo,
405405
false /*poison*/, true /*moved*/);
406406
}
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %target-swift-frontend -parse-as-library -g -emit-ir -o - %s | %FileCheck %s
3+
// RUN: %target-swift-frontend -parse-as-library -g -c %s -o %t/out.o
4+
// RUN: %llvm-dwarfdump --show-children %t/out.o | %FileCheck -check-prefix=DWARF %s
5+
6+
// This test checks that:
7+
//
8+
// 1. At the IR level, we insert the appropriate llvm.dbg.addr, llvm.dbg.value.
9+
//
10+
// 2. At the Dwarf that we have proper locations with PC validity ranges where
11+
// the value isn't valid.
12+
13+
// We only run this on macOS right now since we would need to pattern match
14+
// slightly differently on other platforms.
15+
// REQUIRES: OS=macosx
16+
17+
18+
//////////////////
19+
// Declarations //
20+
//////////////////
21+
22+
public class Klass {
23+
public func doSomething() {}
24+
}
25+
26+
public protocol P {
27+
static var value: P { get }
28+
func doSomething()
29+
}
30+
31+
///////////
32+
// Tests //
33+
///////////
34+
35+
// CHECK-LABEL: define swiftcc void @"$s21move_function_dbginfo17copyableValueTestyyF"()
36+
//
37+
// We should have a llvm.dbg.addr for k since we moved it.
38+
// CHECK: call void @llvm.dbg.addr(metadata {{.*}}** %k.debug, metadata ![[K_COPYABLE_VALUE_TEST:[0-9]*]],
39+
//
40+
// In contrast, we should have a dbg.declare for m since we aren't
41+
// CHECK: call void @llvm.dbg.declare(metadata {{.*}}** %m.debug, metadata ![[M_COPYABLE_VALUE_TEST:[0-9]*]],
42+
//
43+
// Our undef should be an llvm.dbg.value. Counter-intuitively this works for
44+
// both llvm.dbg.addr /and/ llvm.dbg.value. Importantly though its metadata
45+
// should be for k since that is the variable that we are telling the debugger
46+
// is no longer defined.
47+
// CHECK: call void @llvm.dbg.value(metadata %T21move_function_dbginfo5KlassC* undef, metadata ![[K_COPYABLE_VALUE_TEST]],
48+
//
49+
// CHECK: ret void
50+
// CHECK-NEXT: }
51+
// Anchor to the next function in line.
52+
// CHECK: define swiftcc %swift.metadata_response @"$s21move_function_dbginfo5KlassCMa"(i64 %0)
53+
//
54+
// DWARF: DW_AT_linkage_name{{.*}}("$s3out17copyableValueTestyyF")
55+
// DWARF-NEXT: DW_AT_name ("copyableValueTest")
56+
// DWARF-NEXT: DW_AT_decl_file
57+
// DWARF-NEXT: DW_AT_decl_line
58+
// DWARF-NEXT: DW_AT_type
59+
// DWARF-NEXT: DW_AT_external (true)
60+
//
61+
// DWARF: DW_TAG_variable
62+
// DWARF-NEXT: DW_AT_location (DW_OP_fbreg
63+
// DWARF-NEXT: DW_AT_name ("m")
64+
// DWARF-NEXT: DW_AT_decl_file (
65+
// DWARF-NEXT: DW_AT_decl_line (
66+
// DWARF-NEXT: DW_AT_type (
67+
//
68+
// DWARF: DW_TAG_variable
69+
// DWARF-NEXT: DW_AT_location (0x{{[a-z0-9]+}}:
70+
// DWARF-NEXT: [0x{{[a-z0-9]+}}, 0x{{[a-z0-9]+}}):
71+
// DWARF-NEXT: DW_AT_name ("k")
72+
// DWARF-NEXT: DW_AT_decl_file (
73+
// DWARF-NEXT: DW_AT_decl_line (
74+
// DWARF-NEXT: DW_AT_type (
75+
public func copyableValueTest() {
76+
let k = Klass()
77+
k.doSomething()
78+
let m = _move(k)
79+
m.doSomething()
80+
}
81+
82+
// TODO: Look at this.
83+
public func copyableVarTest() {
84+
var k = Klass()
85+
k.doSomething()
86+
let m = _move(k)
87+
m.doSomething()
88+
k = Klass()
89+
k.doSomething()
90+
}
91+
92+
public func addressOnlyValueTest<T : P>(_ x: T) {
93+
let k = x
94+
k.doSomething()
95+
let m = _move(k)
96+
m.doSomething()
97+
}
98+
99+
public func addressOnlyVarTest<T : P>(_ x: T) {
100+
var k = x
101+
k.doSomething()
102+
let m = _move(k)
103+
m.doSomething()
104+
k = x
105+
k.doSomething()
106+
}

0 commit comments

Comments
 (0)