Skip to content

Commit 166daeb

Browse files
authored
Merge pull request #28153 from atrick/escape-refcount
EscapeAnalysis: add a refcount flag to content nodes.
2 parents 67cd031 + 999b32e commit 166daeb

File tree

3 files changed

+79
-21
lines changed

3 files changed

+79
-21
lines changed

include/swift/SILOptimizer/Analysis/EscapeAnalysis.h

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -305,13 +305,19 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
305305
/// True if the merge is finished (see mergeTo). In this state this node
306306
/// is completely unlinked from the graph,
307307
bool isMerged = false;
308-
308+
309+
/// True if this is a content node that owns a reference count. Such a
310+
/// content node necessarilly keeps alive all content it points to until it
311+
/// is released. This can be conservatively false.
312+
bool hasRC = false;
313+
309314
/// The type of the node (mainly distinguishes between content and value
310315
/// nodes).
311316
NodeType Type;
312317

313318
/// The constructor.
314-
CGNode(ValueBase *V, NodeType Type) : V(V), UsePoints(0), Type(Type) {
319+
CGNode(ValueBase *V, NodeType Type, bool hasRC)
320+
: V(V), UsePoints(0), hasRC(hasRC), Type(Type) {
315321
switch (Type) {
316322
case NodeType::Argument:
317323
case NodeType::Value:
@@ -424,6 +430,12 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
424430

425431
/// Return true if this node represents content.
426432
bool isContent() const { return Type == NodeType::Content; }
433+
434+
/// Return true if this node represents an entire reference counted object.
435+
bool hasRefCount() const { return hasRC; }
436+
437+
void setRefCount() { hasRC = true; }
438+
427439
/// Returns the escape state.
428440
EscapeState getEscapeState() const { return State; }
429441

@@ -565,10 +577,14 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
565577

566578
/// Removes all nodes from the graph.
567579
void clear();
568-
580+
569581
/// Allocates a node of a given type.
570-
CGNode *allocNode(ValueBase *V, NodeType Type) {
571-
CGNode *Node = new (NodeAllocator.Allocate()) CGNode(V, Type);
582+
///
583+
/// hasRC is set for Content nodes based on the type and origin of
584+
/// the pointer.
585+
CGNode *allocNode(ValueBase *V, NodeType Type, bool hasRC = false) {
586+
assert(Type == NodeType::Content || !hasRC);
587+
CGNode *Node = new (NodeAllocator.Allocate()) CGNode(V, Type, hasRC);
572588
Nodes.push_back(Node);
573589
return Node;
574590
}
@@ -631,7 +647,7 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
631647
/// Helper to create a content node and update the pointsTo graph. \p
632648
/// addrNode will point to the new content node. The new content node is
633649
/// directly initialized with the remaining function arguments.
634-
CGNode *createContentNode(CGNode *addrNode, SILValue addrVal);
650+
CGNode *createContentNode(CGNode *addrNode, SILValue addrVal, bool hasRC);
635651

636652
/// Create a new content node based on an existing content node to support
637653
/// graph merging.
@@ -691,7 +707,7 @@ class EscapeAnalysis : public BottomUpIPAnalysis {
691707
void escapeContentsOf(CGNode *Node) {
692708
CGNode *escapedContent = Node->getContentNodeOrNull();
693709
if (!escapedContent) {
694-
escapedContent = createContentNode(Node, Node->V);
710+
escapedContent = createContentNode(Node, Node->V, /*hasRC=*/false);
695711
}
696712
escapedContent->markEscaping();
697713
}

include/swift/SILOptimizer/Analysis/ValueTracking.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ bool pointsToLocalObject(SILValue V);
4747
/// allocated object).
4848
///
4949
/// - an address projection based on an exclusive argument with no levels of
50-
/// indirection.
50+
/// indirection (e.g. ref_element_addr, project_box, etc.).
5151
inline bool isUniquelyIdentified(SILValue V) {
5252
return pointsToLocalObject(V)
5353
|| isExclusiveArgument(getUnderlyingAddressRoot(V));

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,9 @@ static bool mayContainReference(SILType Ty, const SILFunction &F) {
117117

118118
// Returns true if the type \p Ty must be a reference or must transitively
119119
// contain a reference. If \p Ty is itself an address, return false.
120-
// Will be used in a subsequent commit.
121-
// static bool mustContainReference(SILType Ty, const SILFunction &F) {
122-
// return findRecursiveRefType(Ty, F, true);
123-
//}
120+
static bool mustContainReference(SILType Ty, const SILFunction &F) {
121+
return findRecursiveRefType(Ty, F, true);
122+
}
124123

125124
bool EscapeAnalysis::isPointer(ValueBase *V) const {
126125
auto *F = V->getFunction();
@@ -334,6 +333,15 @@ class EscapeAnalysis::CGNodeMap {
334333
void EscapeAnalysis::CGNode::mergeProperties(CGNode *fromNode) {
335334
if (!V)
336335
V = fromNode->V;
336+
337+
// TODO: Optimistically merge hasRC. 'this' node can only be merged with
338+
// `fromNode` if their pointer values are compatible. If `fromNode->hasRC` is
339+
// true, then it is guaranteed to represent the head of a heap object. Thus,
340+
// it can only be merged with 'this' when the pointer values that access
341+
// 'this' are also references.
342+
//
343+
// For now, this is pessimistic until we understand performance implications.
344+
hasRC &= fromNode->hasRC;
337345
}
338346

339347
template <typename Visitor>
@@ -842,8 +850,9 @@ void EscapeAnalysis::ConnectionGraph::computeUsePoints() {
842850
}
843851

844852
CGNode *EscapeAnalysis::ConnectionGraph::createContentNode(CGNode *addrNode,
845-
SILValue addrVal) {
846-
CGNode *newContent = allocNode(addrVal, NodeType::Content);
853+
SILValue addrVal,
854+
bool hasRC) {
855+
CGNode *newContent = allocNode(addrVal, NodeType::Content, hasRC);
847856
initializePointsToEdge(addrNode, newContent);
848857
assert(ToMerge.empty()
849858
&& "Initially setting pointsTo should not require any node merges");
@@ -860,7 +869,7 @@ EscapeAnalysis::ConnectionGraph::createMergedContent(CGNode *destAddrNode,
860869
// on the source content.
861870
//
862871
// TODO: node properties will come from `srcContent` here...
863-
return createContentNode(destAddrNode, destAddrNode->V);
872+
return createContentNode(destAddrNode, destAddrNode->V, srcContent->hasRC);
864873
}
865874

866875
// Get a node representing the field data within the given reference-counted
@@ -872,7 +881,7 @@ CGNode *EscapeAnalysis::ConnectionGraph::getFieldContent(CGNode *rcNode) {
872881
if (rcNode->pointsTo)
873882
return rcNode->pointsTo;
874883

875-
return createContentNode(rcNode, rcNode->V);
884+
return createContentNode(rcNode, rcNode->V, /*hasRC=*/false);
876885
}
877886

878887
bool EscapeAnalysis::ConnectionGraph::mergeFrom(ConnectionGraph *SourceGraph,
@@ -1140,6 +1149,8 @@ std::string CGForDotView::getNodeLabel(const Node *Node) const {
11401149

11411150
switch (Node->OrigNode->Type) {
11421151
case swift::EscapeAnalysis::NodeType::Content:
1152+
if (Node->OrigNode->hasRefCount())
1153+
O << "rc-";
11431154
O << "content";
11441155
break;
11451156
case swift::EscapeAnalysis::NodeType::Return:
@@ -1177,7 +1188,11 @@ std::string CGForDotView::getNodeAttributes(const Node *Node) const {
11771188
std::string attr;
11781189
switch (Orig->Type) {
11791190
case swift::EscapeAnalysis::NodeType::Content:
1180-
attr = "style=\"rounded\"";
1191+
attr = "style=\"rounded";
1192+
if (Orig->hasRefCount()) {
1193+
attr += ",filled";
1194+
}
1195+
attr += "\"";
11811196
break;
11821197
case swift::EscapeAnalysis::NodeType::Argument:
11831198
case swift::EscapeAnalysis::NodeType::Return:
@@ -1296,6 +1311,9 @@ void EscapeAnalysis::ConnectionGraph::dumpCG() const {
12961311

12971312
void EscapeAnalysis::CGNode::dump() const {
12981313
llvm::errs() << getTypeStr();
1314+
if (hasRefCount())
1315+
llvm::errs() << " [rc]";
1316+
12991317
if (V)
13001318
llvm::errs() << ": " << *V;
13011319
else
@@ -1406,6 +1424,9 @@ void EscapeAnalysis::ConnectionGraph::print(llvm::raw_ostream &OS) const {
14061424
OS << Separator << NodeStr(Def);
14071425
Separator = ", ";
14081426
}
1427+
if (Nd->hasRefCount())
1428+
OS << " [rc]";
1429+
14091430
OS << '\n';
14101431
}
14111432
OS << "End\n";
@@ -1439,6 +1460,10 @@ void EscapeAnalysis::ConnectionGraph::verify(bool allowMerge) const {
14391460
// which consist of only defer-edges and a single trailing points-to edge
14401461
// must lead to the same
14411462
assert(Nd->matchPointToOfDefers(allowMerge));
1463+
if (Nd->isContent() && Nd->V) {
1464+
if (Nd->hasRefCount())
1465+
assert(mayContainReference(Nd->V->getType(), *F));
1466+
}
14421467
}
14431468
#endif
14441469
}
@@ -1522,15 +1547,26 @@ EscapeAnalysis::getValueContent(ConnectionGraph *conGraph, SILValue addrVal) {
15221547
assert(isPointer(addrNodeValue));
15231548
assert(addrNodeValue == getPointerRoot(addrVal));
15241549
}
1550+
auto *F = addrVal->getFunction();
1551+
bool hasRC = mustContainReference(baseAddr->getType(), *F)
1552+
|| mustContainReference(addrVal->getType(), *F);
1553+
15251554
// Have we already merged a content node?
15261555
if (CGNode *content = addrNode->getContentNodeOrNull()) {
1527-
// TODO: Merge node properties here.
1556+
// hasRC might not match if one of the values pointing to this content was
1557+
// cast to an unknown type. If any of the types must contain a reference,
1558+
// then the content should contain a reference.
1559+
if (content->hasRefCount())
1560+
assert(mayContainReference(baseAddr->getType(), *F));
1561+
else if (hasRC)
1562+
content->setRefCount();
1563+
15281564
return content;
15291565
}
15301566
if (!isPointer(baseAddr))
15311567
return nullptr;
15321568

1533-
return conGraph->createContentNode(addrNode, baseAddr);
1569+
return conGraph->createContentNode(addrNode, baseAddr, hasRC);
15341570
}
15351571

15361572
void EscapeAnalysis::buildConnectionGraph(FunctionInfo *FInfo,
@@ -1737,8 +1773,11 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I,
17371773
CGNode *ArrayRefNode = ArrayStructValue->getContentNodeOrNull();
17381774
if (!ArrayRefNode) {
17391775
ArrayRefNode = ConGraph->createContentNode(
1740-
ArrayStructValue, ArrayStructValue->getValueOrNull());
1741-
}
1776+
ArrayStructValue, ArrayStructValue->getValueOrNull(),
1777+
/*hasRC=*/true);
1778+
} else
1779+
ArrayRefNode->setRefCount();
1780+
17421781
// Another content node for the element storage.
17431782
CGNode *ArrayElementStorage = ConGraph->getFieldContent(ArrayRefNode);
17441783
ArrayElementStorage->markEscaping();
@@ -1868,6 +1907,9 @@ void EscapeAnalysis::analyzeInstruction(SILInstruction *I,
18681907
if (!rcContent)
18691908
return;
18701909

1910+
// rcContent->hasRefCount() may or may not be true depending on whether
1911+
// the type could be analyzed. Either way, treat it structurally like a
1912+
// refcounted object.
18711913
CGNode *fieldContent = ConGraph->getFieldContent(rcContent);
18721914
if (!deinitIsKnownToNotCapture(OpV)) {
18731915
fieldContent->markEscaping();

0 commit comments

Comments
 (0)