Skip to content

Commit f009cf3

Browse files
committed
EscapeAnalysis cleanup and add utilities [nearly NFC]
This is the first in a series of patches that reworks EscapeAnalysis. For this patch, I extracted every change that does not introduce new features, rewrite logic, or appear to change functionality. These cleanups were done in preparation for: - adding a graph representation for reference counted objects - rewriting parts to the query logic - ...which then allows the analysis to safely assume that all exclusive arguments are unique - ...which then allows more aggressive optimization of local variables that don't escape There are two possible functional changes: 1. getUnderlyingAddressRoot in InstructionUtils now sees through OSSA instructions: begin_borrow and copy_value 2. The getPointerBase helper in EscapeAnalysis now sees through all of these reference and pointer casts: + case ValueKind::UncheckedRefCastInst: + case ValueKind::ConvertFunctionInst: + case ValueKind::UpcastInst: + case ValueKind::InitExistentialRefInst: + case ValueKind::OpenExistentialRefInst: + case ValueKind::RawPointerToRefInst: + case ValueKind::RefToRawPointerInst: + case ValueKind::RefToBridgeObjectInst: + case ValueKind::BridgeObjectToRefInst: + case ValueKind::UncheckedAddrCastInst: + case ValueKind::UnconditionalCheckedCastInst: + case ValueKind::RefTo##Name##Inst: + case ValueKind::Name##ToRefInst: This coalesces a whole bunch of nodes together that were just there because of casts. The existing code was already doing this for one level of casts, but there was a bug that prevented it from happening transitively. So, in theory, anything that breaks with this fix could also break without this fix, but may not have been exposed. The fact that this analysis coalesces address-to-reference casts at all is what caused me to spent vast amounts of time debugging any time I tried to force some structure on the graph via assertions. If it is done at all, it should be done everywhere consistently to expose issues as early as possible. Here is a description of the changes in diff order. If something in the diff is not listed here, then the code probably just moved around in the file: Rename isNotAliasedIndirectParameter to isExclusiveIndirectParameter. The argument may be aliased in the caller's scope and it's contents may have already escaped. Add comments to SILType APIs (isTrivial, isReferenceCounted) that give answers about the AST type which don't really make sense for address SILTypes. Add comments about CGNode's 'Value' field. I spent lots of time attempting to tighten this down with asserts, but it's only possible for non-content nodes. For content nodes, the node's value is highly unpredictable and basically nonsense but needed for debugging. Add comments about not assuming that the content nodes pointsTo edge represents physical indirection. This matters when reasoning about aliasing and it's a tempting assumption to make. Add a CGNode::mergeProperties placeholder for adding node properties. Factor out a CGNode::canAddDeferred helper for use later. Rename `setPointsTo` to `setPointsToEdge` because it actually creates an edge rather than just setting `pointsTo`. Add CGNode::getValue() and related helpers to help maintain invariants. Factor out a `markEscaping` helper. Clean up the `escapesInsideFunction` helper. Add node visitor helpers: visitSuccessors, visitDefers. This made is much easier to prototype utilities. Add comments to clarify the `pointsTo` invariant. In particular, an entire defer web may have a null pointsTo for a while. Add an `activeWorklist` to avoid nasty bugs when composing multiple helpers that each use the worklist. Remove the `EA` argument from `getNode`. I ended up needing access to the `EA` context from the ConnectionGraph many times during prototyping and passing `this` was all the `getNode` calls was very silly. Add graph visitor helpers: backwardTraverse, forwardTraverseDefer, forwardTraversePointsToEdges, and mayReach for ease in developing new logic and utilities. Add isExclusiveArgument helper and distinguish exclusive arguments from local objects. Confusing these properties could lead to scary bugs. For example, unlike a local object, an exclusive argument's contents may still escape even when the content's connection graph node is non-escaping! Add isUniquelyIdentified helper when we want to treat exclusive arguments and local objects uniformly. getUnderlyingAddressRoot now looks through OSSA instructions. Rename `getAccessedMemory` to `getDirectlyAccessedMemory` with comments. This is another dangerous API because it assumes the memory access to a given address won't touch memory represented by different graph nodes, but graph edges don't necessarily represent physical indirection. Further clarify this issue in comments in AliasAnalysis.cpp. Factor out a 'findRecursiveRefType' helper from the old 'mayContainReference' for checking whether types may or must contain references. Support both kinds of queries so the analysis can be certain when a pointer's content is a physical heap object. Factor out 'getPointerBase' and 'getPointerRoot' helpers that follow address projections within what EscapeAnalysis will consider a single node. Create a CGNodeWorklist abstraction to safely formalize the worklist mechanism that's used all over the place. In one place, there were even two separate independent lists used independently (nodes added to one list could appear to be in the other list). The CGNodeMap abstraction did not significantly change, it was just moved. Added 'dumpCG' for dumping .dot files making it possible to remote debug. Added '-escapes-enable-graphwriter' option to dump .dot files, since they are so much more useful than the textual dump of the connection graph, which lacks node identity!
1 parent ea950df commit f009cf3

File tree

12 files changed

+750
-342
lines changed

12 files changed

+750
-342
lines changed

include/swift/SIL/SILArgumentConvention.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,8 +129,8 @@ struct SILArgumentConvention {
129129
llvm_unreachable("covered switch isn't covered?!");
130130
}
131131

132-
/// Returns true if \p Value is a not-aliasing indirect parameter.
133-
bool isNotAliasedIndirectParameter() {
132+
/// Returns true if \p Value is a non-aliasing indirect parameter.
133+
bool isExclusiveIndirectParameter() {
134134
switch (Value) {
135135
case SILArgumentConvention::Indirect_In:
136136
case SILArgumentConvention::Indirect_In_Constant:

include/swift/SIL/SILType.h

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,19 +281,22 @@ class SILType {
281281
/// address-only. This is the opposite of isLoadable.
282282
bool isAddressOnly(const SILFunction &F) const;
283283

284-
/// True if the type, or the referenced type of an address type, is trivial,
285-
/// meaning it is loadable and can be trivially copied, moved or detroyed.
284+
/// True if the underlying AST type is trivial, meaning it is loadable and can
285+
/// be trivially copied, moved or detroyed. Returns false for address types
286+
/// even though they are technically trivial.
286287
bool isTrivial(const SILFunction &F) const;
287288

288289
/// True if the type, or the referenced type of an address type, is known to
289-
/// be a scalar reference-counted type.
290+
/// be a scalar reference-counted type such as a class, box, or thick function
291+
/// type. Returns false for non-trivial aggregates.
290292
bool isReferenceCounted(SILModule &M) const;
291293

292294
/// Returns true if the referenced type is a function type that never
293295
/// returns.
294296
bool isNoReturnFunction(SILModule &M) const;
295297

296-
/// Returns true if the referenced type has reference semantics.
298+
/// Returns true if the referenced AST type has reference semantics, even if
299+
/// the lowered SIL type is known to be trivial.
297300
bool hasReferenceSemantics() const {
298301
return getASTType().hasReferenceSemantics();
299302
}

0 commit comments

Comments
 (0)