Skip to content

Commit 0faa055

Browse files
committed
[DebugInfo] Add debug info support for recursive SIL SROA
This removes the restriction that only one fragment is allowed at the end of a SIL DIExpression. rdar://100046900
1 parent bd46860 commit 0faa055

File tree

10 files changed

+67
-36
lines changed

10 files changed

+67
-36
lines changed

include/swift/SIL/SILDebugVariable.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@ struct SILDebugVariable {
7676
static std::optional<SILDebugVariable>
7777
createFromAllocation(const AllocationInst *AI);
7878

79+
/// Return the underlying variable declaration that this denotes,
80+
/// or nullptr if we don't have one.
81+
VarDecl *getDecl() const;
82+
7983
// We're not comparing DIExpr here because strictly speaking,
8084
// DIExpr is not part of the debug variable. We simply piggyback
8185
// it in this class so that's it's easier to carry DIExpr around.

lib/IRGen/IRGenDebugInfo.cpp

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2938,21 +2938,19 @@ bool IRGenDebugInfoImpl::buildDebugInfoExpression(
29382938
llvm::DIExpression::FragmentInfo &Fragment) {
29392939
assert(VarInfo.DIExpr && "SIL debug info expression not found");
29402940

2941-
#ifndef NDEBUG
2942-
bool HasFragment = VarInfo.DIExpr.hasFragment();
2943-
#endif
2944-
29452941
const auto &DIExpr = VarInfo.DIExpr;
29462942
for (const SILDIExprOperand &ExprOperand : DIExpr.operands()) {
2943+
llvm::DIExpression::FragmentInfo SubFragment = {0, 0};
29472944
switch (ExprOperand.getOperator()) {
29482945
case SILDIExprOperator::Fragment:
2949-
assert(HasFragment && "Fragment must be the last part of a DIExpr");
2950-
#ifndef NDEBUG
2951-
// Trigger the assert above if we have more than one fragment expression.
2952-
HasFragment = false;
2953-
#endif
2954-
if (!handleFragmentDIExpr(ExprOperand, Fragment))
2946+
if (!handleFragmentDIExpr(ExprOperand, SubFragment))
29552947
return false;
2948+
assert(!Fragment.SizeInBits
2949+
|| (SubFragment.OffsetInBits + SubFragment.SizeInBits
2950+
<= Fragment.SizeInBits)
2951+
&& "Invalid nested fragments");
2952+
Fragment.OffsetInBits += SubFragment.OffsetInBits;
2953+
Fragment.SizeInBits = SubFragment.SizeInBits;
29562954
break;
29572955
case SILDIExprOperator::Dereference:
29582956
Operands.push_back(llvm::dwarf::DW_OP_deref);

lib/IRGen/IRGenSIL.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5463,9 +5463,16 @@ void IRGenSILFunction::visitDebugValueInst(DebugValueInst *i) {
54635463
assert(isa<SILBoxType>(RealTy));
54645464
}
54655465

5466+
VarDecl *VD = i->getDecl();
5467+
if (!VD) {
5468+
// The source location of a DebugValueInst inserted by the SIL optimizer is
5469+
// not necessarily the VarDecl, as it can be the source location of the
5470+
// update point this DebugValueInst represents.
5471+
VD = VarInfo->getDecl();
5472+
}
54665473
// Figure out the debug variable type
5467-
if (VarDecl *Decl = i->getDecl()) {
5468-
DbgTy = DebugTypeInfo::getLocalVariable(Decl, RealTy, getTypeInfo(SILTy),
5474+
if (VD) {
5475+
DbgTy = DebugTypeInfo::getLocalVariable(VD, RealTy, getTypeInfo(SILTy),
54695476
IGM, IsFragmentType);
54705477
} else if (!SILTy.hasArchetype() && !VarInfo->Name.empty()) {
54715478
// Handle the cases that read from a SIL file

lib/SIL/IR/SILInstructions.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,10 +208,6 @@ SILDebugVariable::createFromAllocation(const AllocationInst *AI) {
208208
if (!VarInfo)
209209
return {};
210210

211-
// TODO: Support variables with expressions.
212-
if (VarInfo->DIExpr)
213-
return {};
214-
215211
// Coalesce the debug loc attached on AI into VarInfo
216212
SILType Type = AI->getType();
217213
SILLocation InstLoc = AI->getLoc();
@@ -495,6 +491,12 @@ VarDecl *DebugValueInst::getDecl() const {
495491
return getLoc().getAsASTNode<VarDecl>();
496492
}
497493

494+
VarDecl *SILDebugVariable::getDecl() const {
495+
if (!Loc)
496+
return nullptr;
497+
return Loc->getAsASTNode<VarDecl>();
498+
}
499+
498500
AllocExistentialBoxInst *AllocExistentialBoxInst::create(
499501
SILDebugLocation Loc, SILType ExistentialType, CanType ConcreteType,
500502
ArrayRef<ProtocolConformanceRef> Conformances,

lib/SIL/Verifier/SILVerifier.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,6 +1469,7 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
14691469

14701470
// Check debug info expression
14711471
if (const auto &DIExpr = varInfo->DIExpr) {
1472+
bool HasFragment = false;
14721473
for (auto It = DIExpr.element_begin(), ItEnd = DIExpr.element_end();
14731474
It != ItEnd;) {
14741475
require(It->getKind() == SILDIExprElement::OperatorKind,
@@ -1483,8 +1484,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
14831484
"di-expression operand kind mismatch");
14841485

14851486
if (Op == SILDIExprOperator::Fragment)
1486-
require(It == ItEnd, "op_fragment directive needs to be at the end "
1487-
"of a di-expression");
1487+
HasFragment = true;
1488+
else
1489+
require(!HasFragment, "no directive allowed after op_fragment"
1490+
" in a di-expression");
14881491
}
14891492
}
14901493
}

lib/SILOptimizer/Utils/InstOptUtils.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1933,10 +1933,7 @@ void swift::createDebugFragments(SILValue oldValue, Projection proj,
19331933
if (!debugVal)
19341934
continue;
19351935

1936-
// Can't create a fragment of a fragment.
19371936
auto varInfo = debugVal->getVarInfo();
1938-
if (!varInfo || varInfo->DIExpr.hasFragment())
1939-
continue;
19401937

19411938
SILType baseType = oldValue->getType();
19421939

test/DebugInfo/sil_based_dbg.swift

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,12 @@ struct TheStruct {
2929
}
3030
// CHECK_DBG_SCOPE-LABEL: sil {{.*}}test_debug_scope
3131
public func test_debug_scope(val : Int) -> Int {
32-
// TODO: Repeated SROA of the same variable is currently disabled.
33-
// CHECK_DBG_SCOPE: alloc_stack $Builtin.Int{{[0-9]+}},
34-
// CHECK_DBG_SCOPE-NOT: var,
35-
// DISABLED_DBG_SCOPE-SAME: var, (name "the_struct",
36-
// DISABLED_DBG_SCOPE-SAME: loc
32+
// CHECK_DBG_SCOPE: alloc_stack $Builtin.Int{{[0-9]+}},
33+
// CHECK_DBG_SCOPE-SAME: var, (name "the_struct",
34+
// CHECK_DBG_SCOPE-SAME: loc
35+
// CHECK_DBG_SCOPE-SAME: expr op_fragment:#TheStruct.the_member:op_fragment:#Int._value
3736
// The auxiliary debug scope should be removed
38-
// DISABLED_DBG_SCOPE-NOT: scope {{[0-9]+}})
37+
// CHECK_DBG_SCOPE-NOT: scope {{[0-9]+}})
3938
var the_struct = TheStruct(the_member: 0)
4039
the_struct.the_member = val + 13
4140
return the_struct.the_member

test/DebugInfo/srosroa_mem2reg.sil

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,10 @@ bb0(%0 : $Int64, %1 : $Int64):
3131
debug_value %0 : $Int64, let, name "in_x", argno 1, loc "sroa.swift":7:10, scope 2
3232
debug_value %1 : $Int64, let, name "in_y", argno 2, loc "sroa.swift":7:21, scope 2
3333
%4 = alloc_stack $MyStruct, var, name "my_struct", type $*MyContainer, expr op_fragment:#MyContainer.z, loc "sroa.swift":8:9, scope 2
34-
// Recursive SROA of already SROA-ed aggregates is not yet supported.
3534
// CHECK-SROA: alloc_stack $Int64
36-
// CHECK-SROA-NOT: var,
35+
// CHECK-SROA-SAME: var,
3736
// CHECK-SROA: alloc_stack $Int64
38-
// CHECK-SROA-NOT: var,
37+
// CHECK-SROA-SAME: var,
3938
// CHECK-SROA: metatype
4039
%5 = metatype $@thin MyStruct.Type, loc "sroa.swift":8:21, scope 2
4140
%6 = integer_literal $Builtin.Int64, 0, loc "sroa.swift":8:33, scope 2
Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: not --crash %target-swift-frontend %s -sil-verify-all -g -emit-sil
1+
// RUN: %target-swift-frontend %s -sil-verify-all -g -emit-sil | %FileCheck %s
22
import Builtin
33
import Swift
44

@@ -11,11 +11,12 @@ sil_scope 1 { loc "file.swift":7:6 parent @test_fragment : $@convention(thin) ()
1111

1212
sil hidden @test_fragment : $@convention(thin) () -> () {
1313
bb0:
14-
%2 = alloc_stack $MyStruct, var, name "my_struct", loc "file.swift":8:9, scope 1
15-
%3 = struct_element_addr %2 : $*MyStruct, #MyStruct.x, loc "file.swift":9:17, scope 1
16-
// every op_fragment should be the last di-expression operand
17-
debug_value %3 : $*Builtin.Int64, var, name "my_struct", expr op_deref:op_fragment:#MyStruct.y:op_fragment:#MyStruct.x
18-
dealloc_stack %2 : $*MyStruct
14+
%0 = alloc_stack $MyStruct, var, name "my_struct", loc "file.swift":8:9, scope 1
15+
%1 = struct_element_addr %0 : $*MyStruct, #MyStruct.x, loc "file.swift":9:17, scope 1
16+
// nested op_fragments at the end are allowed
17+
// CHECK: debug_value %1 : $*Builtin.Int64, var, name "my_struct", expr op_deref:op_fragment:#MyStruct.y:op_fragment:#MyStruct.x
18+
debug_value %1 : $*Builtin.Int64, var, name "my_struct", expr op_deref:op_fragment:#MyStruct.y:op_fragment:#MyStruct.x
19+
dealloc_stack %0 : $*MyStruct
1920
%r = tuple()
2021
return %r : $()
2122
}
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// RUN: not --crash %target-swift-frontend %s -sil-verify-all -g -emit-sil
2+
import Builtin
3+
import Swift
4+
5+
struct MyStruct {
6+
var x: Builtin.Int64
7+
var y: Builtin.Int64
8+
}
9+
10+
sil_scope 1 { loc "file.swift":7:6 parent @test_fragment : $@convention(thin) () -> () }
11+
12+
sil hidden @test_fragment : $@convention(thin) () -> () {
13+
bb0:
14+
%0 = alloc_stack $MyStruct, var, name "my_struct", loc "file.swift":8:9, scope 1
15+
%1 = struct_element_addr %0 : $*MyStruct, #MyStruct.x, loc "file.swift":9:17, scope 1
16+
// every op_fragment should be the last di-expression operand
17+
debug_value %1 : $*Builtin.Int64, var, name "my_struct", expr op_deref:op_fragment:#MyStruct.y:op_constu:42:op_plus
18+
dealloc_stack %0 : $*MyStruct
19+
%r = tuple()
20+
return %r : $()
21+
}

0 commit comments

Comments
 (0)