Skip to content

Commit 8ab1e2d

Browse files
committed
Unify debug scope and location handling in SILInstruction and SILBuilder.
The drivers for this change are providing a simpler API to SIL pass authors, having a more efficient of the in-memory representation, and ruling out an entire class of common bugs that usually result in hard-to-debug backend crashes. Summary ------- SILInstruction Old New +---------------+ +------------------+ +-----------------+ |SILInstruction | |SILInstruction | |SILDebugLocation | +---------------+ +------------------+ +-----------------+ | ... | | ... | | ... | |SILLocation | |SILDebugLocation *| -> |SILLocation | |SILDebugScope *| +------------------+ |SILDebugScope * | +---------------+ +-----------------+ We’re introducing a new class SILDebugLocation which represents the combination of a SILLocation and a SILDebugScope. Instead of storing an inline SILLocation and a SILDebugScope pointer, SILInstruction now only has one SILDebugLocation pointer. The APIs of SILBuilder and SILDebugLocation guarantees that every SILInstruction has a nonempty SILDebugScope. Developer-visible changes include: SILBuilder ---------- In the old design SILBuilder populated the InsertedInstrs list to allow setting the debug scopes of all built instructions in bulk at the very end (as the responsibility of the user). In the new design, SILBuilder now carries a "current debug scope" state and immediately sets the debug scope when an instruction is inserted. This fixes a use-after-free issue with with SIL passes that delete instructions before destroying the SILBuilder that created them. Because of this, SILBuilderWithScopes no longer needs to be a template, which simplifies its call sites. SILInstruction -------------- It is neither possible or necessary to manually call setDebugScope() on a SILInstruction any more. The function still exists as a private method, but is only used when splicing instructions from one function to another. Efficiency ---------- In addition to dropping 20 bytes from each SILInstruction, SILDebugLocations are now allocated in the SILModule's bump pointer allocator and are uniqued by SILBuilder. Unfortunately repeat compiles of the standard library already vary by about 5% so I couldn’t yet produce reliable numbers for how much this saves overall. rdar://problem/22017421
1 parent 23c2579 commit 8ab1e2d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

67 files changed

+2163
-2074
lines changed

include/swift/Basic/SourceLoc.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ class SourceLoc {
5959
return SourceLoc();
6060
}
6161

62+
const void *getOpaquePointerValue() const { return Value.getPointer(); }
63+
6264
/// Print out the SourceLoc. If this location is in the same buffer
6365
/// as specified by \c LastBufferID, then we don't print the filename. If
6466
/// not, we do print the filename, and then update \c LastBufferID with the

include/swift/SIL/SILBuilder.h

Lines changed: 622 additions & 579 deletions
Large diffs are not rendered by default.

include/swift/SIL/SILCloner.h

Lines changed: 226 additions & 100 deletions
Large diffs are not rendered by default.

include/swift/SIL/SILDebugScope.h

Lines changed: 64 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,48 +24,45 @@
2424

2525
namespace swift {
2626

27-
class SILFunction;
27+
class SILDebugLocation;
28+
class SILDebugScope;
2829

2930
/// SILDebugScope - This class stores a lexical scope as it is
3031
/// represented in the debug info.
3132
class SILDebugScope : public SILAllocated<SILDebugScope> {
3233
public:
3334
SILLocation Loc;
3435
/// Always points to the parent lexical scope.
35-
SILDebugScope *Parent;
36+
const SILDebugScope *Parent;
3637
/// If this scope is inlined, this points to a special "scope" that
3738
/// holds only the location of the call site. The parent scope will be
3839
/// the scope of the inlined call site.
39-
SILDebugScope *InlinedCallSite;
40+
const SILDebugScope *InlinedCallSite;
4041
/// The SILFunction that the scope belongs to. Inlined functions may
4142
/// be elided, so keep track of their type here.
4243
/// FIXME: Storing this for every scope is wasteful. We only need
4344
/// this once per function.
4445
SILFunction *SILFn;
4546

46-
SILDebugScope(SILLocation Loc,
47-
SILFunction &SILFn,
48-
SILDebugScope *Parent = nullptr)
49-
: Loc(Loc), Parent(Parent), InlinedCallSite(nullptr),
50-
SILFn(&SILFn)
51-
{ }
47+
SILDebugScope(SILLocation Loc, SILFunction &SILFn,
48+
const SILDebugScope *Parent = nullptr,
49+
const SILDebugScope *InlinedCallSite = nullptr)
50+
: Loc(Loc), Parent(Parent), InlinedCallSite(InlinedCallSite),
51+
SILFn(&SILFn) {}
5252

5353
/// Create a scope for an artificial function.
5454
SILDebugScope(SILLocation Loc)
55-
: Loc(Loc), Parent(nullptr), InlinedCallSite(nullptr), SILFn(nullptr)
56-
{ }
55+
: Loc(Loc), Parent(nullptr), InlinedCallSite(nullptr), SILFn(nullptr) {}
5756

5857
/// Create an inlined version of CalleeScope.
59-
SILDebugScope(SILDebugScope *CallSiteScope, SILDebugScope *CalleeScope,
60-
SILFunction *InlinedFn)
61-
: Loc(CalleeScope->Loc), Parent(CalleeScope->Parent),
62-
InlinedCallSite(CallSiteScope), SILFn(InlinedFn) {
58+
SILDebugScope(const SILDebugScope *CallSiteScope,
59+
const SILDebugScope *CalleeScope, SILFunction *InlinedFn)
60+
: Loc(CalleeScope->Loc), Parent(CalleeScope->Parent),
61+
InlinedCallSite(CallSiteScope), SILFn(InlinedFn) {
6362
assert(CallSiteScope && CalleeScope);
6463
assert(InlinedFn->isInlined() &&
6564
"function of inlined debug scope is not inlined");
6665
}
67-
68-
void setParent(SILDebugScope *P) { Parent = P; }
6966
};
7067

7168
#ifndef NDEBUG
@@ -75,36 +72,72 @@ bool maybeScopeless(SILInstruction &I);
7572

7673
/// Knows how to make a deep copy of a debug scope.
7774
class ScopeCloner {
78-
llvm::SmallDenseMap<SILDebugScope *, SILDebugScope *> ClonedScopeCache;
75+
llvm::SmallDenseMap<const SILDebugScope *,
76+
const SILDebugScope *> ClonedScopeCache;
7977
SILFunction &NewFn;
8078
public:
81-
ScopeCloner(SILFunction &Fn) : NewFn(Fn) {
82-
auto *OrigScope = Fn.getDebugScope();
83-
Fn.setDebugScope(getOrCreateClonedScope(OrigScope));
84-
OrigScope->SILFn->setInlined();
79+
/// ScopeCloner expects NewFn to be a clone of the original
80+
/// function, with all debug scopes and locations still pointing to
81+
/// the original function.
82+
ScopeCloner(SILFunction &NewFn) : NewFn(NewFn) {
83+
// Some clients of SILCloner copy over the original function's
84+
// debug scope. Create a new one here.
85+
// FIXME: Audit all call sites and make them create the function
86+
// debug scope.
87+
if (NewFn.getDebugScope()->SILFn != &NewFn) {
88+
NewFn.getDebugScope()->SILFn->setInlined();
89+
NewFn.setDebugScope(getOrCreateClonedScope(NewFn.getDebugScope()));
90+
}
8591
}
86-
SILDebugScope *getOrCreateClonedScope(SILDebugScope *OrigScope) {
92+
93+
/// Return a (cached) deep copy of a scope.
94+
const SILDebugScope *getOrCreateClonedScope(const SILDebugScope *OrigScope) {
8795
if (!OrigScope)
8896
return nullptr;
8997

9098
auto it = ClonedScopeCache.find(OrigScope);
9199
if (it != ClonedScopeCache.end())
92100
return it->second;
93101

94-
auto CloneScope = new (NewFn.getModule()) SILDebugScope(*OrigScope);
102+
auto ClonedScope = new (NewFn.getModule()) SILDebugScope(*OrigScope);
95103
if (OrigScope->InlinedCallSite) {
96104
// For inlined functions, we need to rewrite the inlined call site.
97-
CloneScope->InlinedCallSite =
98-
getOrCreateClonedScope(OrigScope->InlinedCallSite);
105+
ClonedScope->InlinedCallSite =
106+
getOrCreateClonedScope(OrigScope->InlinedCallSite);
99107
} else {
100-
CloneScope->SILFn = &NewFn;
101-
CloneScope->Parent = getOrCreateClonedScope(OrigScope->Parent);
102-
} // Create an inline scope for the cloned instruction.
103-
ClonedScopeCache.insert({OrigScope, CloneScope});
104-
return CloneScope;
108+
ClonedScope->SILFn = &NewFn;
109+
ClonedScope->Parent = getOrCreateClonedScope(OrigScope->Parent);
110+
}
111+
// Create an inline scope for the cloned instruction.
112+
assert(ClonedScopeCache.find(OrigScope) == ClonedScopeCache.end());
113+
ClonedScopeCache.insert({OrigScope, ClonedScope});
114+
return ClonedScope;
115+
}
116+
};
117+
118+
/// A SILLocation together with a SILDebugScope.
119+
class SILDebugLocation : public SILAllocated<SILDebugLocation> {
120+
SILLocation Location;
121+
const SILDebugScope *Scope = nullptr;
122+
123+
public:
124+
SILDebugLocation(SILLocation Loc, const SILDebugScope *DS)
125+
: Location(Loc), Scope(DS) {}
126+
SILLocation getLocation() const { return Location; }
127+
const SILDebugScope *getScope() const { return Scope; }
128+
bool operator==(const SILDebugLocation &other) const {
129+
return Location == other.getLocation() && Scope == other.getScope();
105130
}
106131
};
107132

133+
/// For use in a DenseMap.
134+
typedef std::pair<std::pair<const void *, unsigned>, const void *> DebugLocKey;
135+
struct SILDebugLocationID : public DebugLocKey {
136+
SILDebugLocationID(const SILDebugLocation &L)
137+
: DebugLocKey({L.getLocation().getOpaquePointerValue(),
138+
L.getLocation().getOpaqueKind()},
139+
L.getScope()) {}
140+
};
108141
}
109142

110143
#endif

include/swift/SIL/SILFunction.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ class SILFunction
8686
DeclContext *DeclCtx;
8787

8888
/// The source location and scope of the function.
89-
SILDebugScope *DebugScope;
89+
const SILDebugScope *DebugScope;
9090

9191
/// The function's bare attribute. Bare means that the function is SIL-only
9292
/// and does not require debug info.
@@ -162,7 +162,7 @@ class SILFunction
162162
ClassVisibility_t classVisibility,
163163
Inline_t inlineStrategy, EffectsKind E,
164164
SILFunction *insertBefore,
165-
SILDebugScope *debugScope,
165+
const SILDebugScope *debugScope,
166166
DeclContext *DC);
167167

168168
public:
@@ -178,7 +178,7 @@ class SILFunction
178178
Inline_t inlineStrategy = InlineDefault,
179179
EffectsKind EK = EffectsKind::Unspecified,
180180
SILFunction *InsertBefore = nullptr,
181-
SILDebugScope *DebugScope = nullptr,
181+
const SILDebugScope *DebugScope = nullptr,
182182
DeclContext *DC = nullptr);
183183
~SILFunction();
184184

@@ -380,10 +380,10 @@ class SILFunction
380380
}
381381

382382
/// Initialize the debug scope of the function.
383-
void setDebugScope(SILDebugScope *DS) { DebugScope = DS; }
383+
void setDebugScope(const SILDebugScope *DS) { DebugScope = DS; }
384384

385385
/// Get the source location of the function.
386-
SILDebugScope *getDebugScope() const { return DebugScope; }
386+
const SILDebugScope *getDebugScope() const { return DebugScope; }
387387

388388
/// Get this function's bare attribute.
389389
IsBare_t isBare() const { return IsBare_t(Bare); }

0 commit comments

Comments
 (0)