Skip to content

Commit 76e19d3

Browse files
aslasl
committed
Properly transfer location info from debug_value into alloc_stack: we need to drop op_deref expression
1 parent a802492 commit 76e19d3

File tree

3 files changed

+154
-5
lines changed

3 files changed

+154
-5
lines changed

‎lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,9 +1315,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
13151315
case SILInstructionKind::DebugValueInst:
13161316
DebugVarTy = inst->getOperand(0)->getType();
13171317
if (DebugVarTy.isAddress()) {
1318-
auto Expr = varInfo->DIExpr.operands();
1319-
if (!Expr.empty() &&
1320-
Expr.begin()->getOperator() == SILDIExprOperator::Dereference)
1318+
// FIXME: op_deref could be applied to address types only.
1319+
// FIXME: Add this check
1320+
if (varInfo->DIExpr.startsWithDeref())
13211321
DebugVarTy = DebugVarTy.getObjectType();
13221322
}
13231323
break;
@@ -1343,9 +1343,16 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
13431343
require(DebugVars[argNum].first == varInfo->Name,
13441344
"Scope contains conflicting debug variables for one function "
13451345
"argument");
1346-
// Check for type
1347-
require(DebugVars[argNum].second == DebugVarTy,
1346+
// The source variable might change its location (e.g. due to
1347+
// optimizations). Check for most common transformations (e.g. loading
1348+
// to SSA value and vice versa) as well
1349+
require(DebugVars[argNum].second == DebugVarTy ||
1350+
(DebugVars[argNum].second.isAddress() &&
1351+
DebugVars[argNum].second.getObjectType() == DebugVarTy) ||
1352+
(DebugVarTy.isAddress() &&
1353+
DebugVars[argNum].second == DebugVarTy.getObjectType()),
13481354
"conflicting debug variable type!");
1355+
DebugVars[argNum].second = DebugVarTy;
13491356
} else {
13501357
// Reserve enough space.
13511358
while (DebugVars.size() <= argNum) {

‎lib/SILOptimizer/Differentiation/Common.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,11 @@ findDebugLocationAndVariable(SILValue originalValue) {
257257
for (auto *use : originalValue->getUses()) {
258258
if (auto *dvi = dyn_cast<DebugValueInst>(use->getUser()))
259259
return dvi->getVarInfo().map([&](SILDebugVariable var) {
260+
// We need to drop `op_deref` here as we're transferring debug info
261+
// location from debug_value instruction (which describes how to get value)
262+
// into alloc_stack (which describes the location)
263+
if (var.DIExpr.startsWithDeref())
264+
var.DIExpr.eraseElement(var.DIExpr.element_begin());
260265
return std::make_pair(dvi->getDebugLocation(), var);
261266
});
262267
}

‎test/AutoDiff/compiler_crashers_fixed/sr15849-invalid-debug-info.swift

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
// SR-15849: Mutating functions with control flow can cause assertion failure
2+
// for conflicting debug variable type.
3+
4+
// RUN: %target-swift-frontend -emit-ir -O -g %s | %FileCheck %s
5+
// CHECK-LABEL: define internal swiftcc float @"$s4main8TestTypeV24doDifferentiableTimeStep04timeG0ySf_tFTJpSSpSrTA"
6+
// CHECK: [[SELF:%.*]] = alloca %T4main8TestTypeV06ManualB7TangentV, align 4
7+
// CHECK: call void @llvm.dbg.declare(metadata %T4main8TestTypeV06ManualB7TangentV* [[SELF]]
8+
9+
import _Differentiation
10+
11+
public protocol TestInterface {
12+
mutating func doDifferentiableTimeStep(timeStep: Float)
13+
14+
var zeroTangentVectorInitializer: () -> TestInterfaceTangentVector { get }
15+
mutating func move(by offset: TestInterfaceTangentVector)
16+
}
17+
18+
public protocol TestInterfaceTangentVector {
19+
static var zero: Self { get }
20+
static func add(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> TestInterfaceTangentVector
21+
static func subtract(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> TestInterfaceTangentVector
22+
static func equals(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> Bool
23+
}
24+
25+
public extension TestInterface {
26+
var zeroTangentVector: TestInterfaceTangentVector { zeroTangentVectorInitializer() }
27+
}
28+
29+
private typealias InitFunction = @convention(c) () -> UnsafeMutableRawPointer
30+
31+
public protocol HasZeroTangentVectorDuplicate: Differentiable {
32+
var duplicateZeroTangentVectorInitializer: () -> TangentVector { get }
33+
}
34+
35+
public extension HasZeroTangentVectorDuplicate {
36+
var zeroTangentVector: TangentVector { duplicateZeroTangentVectorInitializer() }
37+
}
38+
39+
public extension HasZeroTangentVectorDuplicate {
40+
var duplicateZeroTangentVectorInitializer: () -> TangentVector {
41+
{ Self.TangentVector.zero }
42+
}
43+
}
44+
45+
struct TestType: TestInterface {
46+
struct TestState: Differentiable {
47+
public struct TangentVector: Differentiable, AdditiveArithmetic {
48+
public typealias TangentVector = TestState.TangentVector
49+
public var property0: Float.TangentVector
50+
public var time: Float.TangentVector
51+
public var property1: Float.TangentVector
52+
}
53+
54+
public mutating func move(by offset: TangentVector) {
55+
self.property0.move(by: offset.property0)
56+
self.time.move(by: offset.time)
57+
self.property1.move(by: offset.property1)
58+
}
59+
60+
@noDerivative
61+
var needUpdate: Bool
62+
@noDerivative
63+
var initialConditionsAreStale: Bool
64+
var property0: Float
65+
var time: Float
66+
var property1: Float
67+
68+
init() {
69+
self.needUpdate = true
70+
self.initialConditionsAreStale = true
71+
self.property0 = 0.01
72+
self.time = 0.01
73+
self.property1 = 0.01
74+
}
75+
}
76+
77+
var state = TestState()
78+
79+
@differentiable(reverse)
80+
mutating func doDifferentiableTimeStep(timeStep: Float) {
81+
if state.needUpdate {
82+
differentiableDoFlow()
83+
}
84+
if state.initialConditionsAreStale {
85+
doInit()
86+
}
87+
}
88+
89+
@differentiable(reverse)
90+
mutating func differentiableDoFlow() {
91+
state.property1 = 1.2
92+
state.property0 = 2.3
93+
state.needUpdate = false
94+
}
95+
mutating func doInit() {
96+
state.initialConditionsAreStale = false
97+
}
98+
99+
}
100+
101+
extension TestType: Differentiable {
102+
struct ManualTestTangent: Differentiable & AdditiveArithmetic {
103+
var state: TestState.TangentVector
104+
}
105+
typealias TangentVector = ManualTestTangent
106+
107+
mutating func move(by offset: ManualTestTangent) {
108+
self.state.move(by: offset.state)
109+
}
110+
}
111+
extension TestType: HasZeroTangentVectorDuplicate {}
112+
113+
114+
extension TestType {
115+
mutating func move(by offset: TestInterfaceTangentVector) {
116+
self.move(by: offset as! Self.TangentVector)
117+
}
118+
119+
var zeroTangentVectorInitializer: () -> TestInterfaceTangentVector {
120+
let initializer: () -> TangentVector = self.duplicateZeroTangentVectorInitializer
121+
return initializer
122+
}
123+
}
124+
125+
extension TestType.TangentVector: TestInterfaceTangentVector {
126+
static func add(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> TestInterfaceTangentVector {
127+
return (lhs as! Self) + (rhs as! Self)
128+
}
129+
130+
static func subtract(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> TestInterfaceTangentVector {
131+
return (lhs as! Self) - (rhs as! Self)
132+
}
133+
134+
static func equals(lhs: TestInterfaceTangentVector, rhs: TestInterfaceTangentVector) -> Bool {
135+
return (lhs as! Self) == (rhs as! Self)
136+
}
137+
}

0 commit comments

Comments
 (0)