Skip to content

Commit bebbe37

Browse files
committed
Fix EscapeAnalysis verification assert at unreachable blocks
If EscapeAnalysis verification runs on unreachable code, it asserts with "Missing escape connection graph mapping" because the connection graph builder only runs on reachable blocks. Add a ReachableBlocks utility and use it during verification. Fixes <rdar://problem/60373501> EscapeAnalysis crashes with CFG with unreachable blocks
1 parent 05f92f1 commit bebbe37

File tree

4 files changed

+98
-38
lines changed

4 files changed

+98
-38
lines changed

include/swift/SILOptimizer/Utils/BasicBlockOptUtils.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,24 @@ class BasicBlockCloner;
3333
class SILLoop;
3434
class SILLoopInfo;
3535

36+
/// Compute the set of reachable blocks.
37+
class ReachableBlocks {
38+
SmallPtrSet<SILBasicBlock *, 32> visited;
39+
40+
public:
41+
/// Invoke \p visitor for each reachable block in \p f in worklist order (at
42+
/// least one predecessor has been visited--defs are always visited before
43+
/// uses except for phi-type block args). The \p visitor takes a block
44+
/// argument, which is already marked visited, and must return true to
45+
/// continue visiting blocks.
46+
///
47+
/// Returns true if all reachable blocks were visited.
48+
bool visit(SILFunction *f, function_ref<bool(SILBasicBlock *)> visitor);
49+
50+
/// Return true if \p bb has been visited.
51+
bool isVisited(SILBasicBlock *bb) const { return visited.count(bb); }
52+
};
53+
3654
/// Remove all instructions in the body of \p bb in safe manner by using
3755
/// undef.
3856
void clearBlockBody(SILBasicBlock *bb);

lib/SILOptimizer/Analysis/EscapeAnalysis.cpp

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "swift/SILOptimizer/Analysis/ArraySemantic.h"
1919
#include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h"
2020
#include "swift/SILOptimizer/PassManager/PassManager.h"
21+
#include "swift/SILOptimizer/Utils/BasicBlockOptUtils.h"
2122
#include "swift/SILOptimizer/Utils/InstOptUtils.h"
2223
#include "llvm/Support/CommandLine.h"
2324
#include "llvm/Support/GraphWriter.h"
@@ -1585,17 +1586,19 @@ void EscapeAnalysis::ConnectionGraph::verify() const {
15851586
verifyStructure();
15861587

15871588
// Verify that all pointer nodes are still mapped, otherwise the process of
1588-
// merging nodes may have lost information.
1589-
for (SILBasicBlock &BB : *F) {
1590-
for (auto &I : BB) {
1591-
if (isNonWritableMemoryAddress(&I))
1589+
// merging nodes may have lost information. Only visit reachable blocks,
1590+
// because the graph builder only mapped values from reachable blocks.
1591+
ReachableBlocks reachable;
1592+
reachable.visit(F, [this](SILBasicBlock *bb) {
1593+
for (auto &i : *bb) {
1594+
if (isNonWritableMemoryAddress(&i))
15921595
continue;
15931596

1594-
if (auto ai = dyn_cast<ApplyInst>(&I)) {
1597+
if (auto ai = dyn_cast<ApplyInst>(&i)) {
15951598
if (EA->canOptimizeArrayUninitializedCall(ai, this).isValid())
15961599
continue;
15971600
}
1598-
for (auto result : I.getResults()) {
1601+
for (auto result : i.getResults()) {
15991602
if (EA->getPointerBase(result))
16001603
continue;
16011604

@@ -1611,7 +1614,8 @@ void EscapeAnalysis::ConnectionGraph::verify() const {
16111614
}
16121615
}
16131616
}
1614-
}
1617+
return true;
1618+
});
16151619
#endif
16161620
}
16171621

@@ -1705,27 +1709,21 @@ void EscapeAnalysis::buildConnectionGraph(FunctionInfo *FInfo,
17051709
ConnectionGraph *ConGraph = &FInfo->Graph;
17061710
assert(ConGraph->isEmpty());
17071711

1708-
// We use a worklist for iteration to visit the blocks in dominance order.
1709-
llvm::SmallPtrSet<SILBasicBlock*, 32> VisitedBlocks;
1710-
llvm::SmallVector<SILBasicBlock *, 16> WorkList;
1711-
VisitedBlocks.insert(&*ConGraph->F->begin());
1712-
WorkList.push_back(&*ConGraph->F->begin());
1713-
1714-
while (!WorkList.empty()) {
1715-
SILBasicBlock *BB = WorkList.pop_back_val();
1716-
1712+
// Visit the blocks in dominance order.
1713+
ReachableBlocks reachable;
1714+
reachable.visit(ConGraph->F, [&](SILBasicBlock *bb) {
17171715
// Create edges for the instructions.
1718-
for (auto &I : *BB) {
1719-
analyzeInstruction(&I, FInfo, BottomUpOrder, RecursionDepth);
1716+
for (auto &i : *bb) {
1717+
analyzeInstruction(&i, FInfo, BottomUpOrder, RecursionDepth);
17201718
}
1721-
for (auto &Succ : BB->getSuccessors()) {
1722-
if (VisitedBlocks.insert(Succ.getBB()).second)
1723-
WorkList.push_back(Succ.getBB());
1724-
}
1725-
}
1719+
return true;
1720+
});
17261721

17271722
// Second step: create defer-edges for block arguments.
17281723
for (SILBasicBlock &BB : *ConGraph->F) {
1724+
if (!reachable.isVisited(&BB))
1725+
continue;
1726+
17291727
if (!linkBBArgs(&BB))
17301728
continue;
17311729

lib/SILOptimizer/Utils/BasicBlockOptUtils.cpp

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,30 @@
1717

1818
using namespace swift;
1919

20+
/// Invoke \p visitor for each reachable block in \p f in worklist order (at
21+
/// least one predecessor has been visited).
22+
bool ReachableBlocks::visit(SILFunction *f,
23+
function_ref<bool(SILBasicBlock *)> visitor) {
24+
assert(visited.empty() && "blocks already visited");
25+
26+
// Walk over the CFG, starting at the entry block, until all reachable blocks
27+
// are visited.
28+
SILBasicBlock *entryBB = f->getEntryBlock();
29+
SmallVector<SILBasicBlock *, 8> worklist = {entryBB};
30+
visited.insert(entryBB);
31+
while (!worklist.empty()) {
32+
SILBasicBlock *bb = worklist.pop_back_val();
33+
if (!visitor(bb))
34+
return false;
35+
36+
for (auto &succ : bb->getSuccessors()) {
37+
if (visited.insert(succ).second)
38+
worklist.push_back(succ);
39+
}
40+
}
41+
return true;
42+
}
43+
2044
/// Remove all instructions in the body of \p bb in safe manner by using
2145
/// undef.
2246
void swift::clearBlockBody(SILBasicBlock *bb) {
@@ -42,25 +66,16 @@ void swift::removeDeadBlock(SILBasicBlock *bb) {
4266
}
4367

4468
bool swift::removeUnreachableBlocks(SILFunction &f) {
45-
// All reachable blocks, but does not include the entry block.
46-
llvm::SmallPtrSet<SILBasicBlock *, 8> visited;
47-
48-
// Walk over the CFG, starting at the entry block, until all reachable blocks are visited.
49-
llvm::SmallVector<SILBasicBlock *, 8> worklist(1, f.getEntryBlock());
50-
while (!worklist.empty()) {
51-
SILBasicBlock *bb = worklist.pop_back_val();
52-
for (auto &Succ : bb->getSuccessors()) {
53-
if (visited.insert(Succ).second)
54-
worklist.push_back(Succ);
55-
}
56-
}
69+
ReachableBlocks reachable;
70+
// Visit all the blocks without doing any extra work.
71+
reachable.visit(&f, [](SILBasicBlock *) { return true; });
5772

58-
// Remove the blocks we never reached. Exclude the entry block from the iteration because it's
59-
// not included in the Visited set.
73+
// Remove the blocks we never reached. Assume the entry block is visited.
74+
// Reachable's visited set contains dangling pointers during this loop.
6075
bool changed = false;
6176
for (auto ii = std::next(f.begin()), end = f.end(); ii != end;) {
6277
auto *bb = &*ii++;
63-
if (!visited.count(bb)) {
78+
if (!reachable.isVisited(bb)) {
6479
removeDeadBlock(bb);
6580
changed = true;
6681
}

test/SILOptimizer/escape_analysis.sil

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1909,3 +1909,32 @@ bb0(%0 : $*SomeData):
19091909
%5 = tuple ()
19101910
return %5 : $()
19111911
}
1912+
1913+
// -----------------------------------------------------------------------------
1914+
// Unreachable blocks are not mapped to the connection graph. Test
1915+
// that verification does not assert with "Missing escape connection
1916+
// graph mapping". <rdar://problem/60373501> EscapeAnalysis crashes
1917+
// with CFG with unreachable
1918+
sil_global hidden @globalInt: $Int
1919+
1920+
// CHECK-LABEL: CG of testUnreachable
1921+
// CHECK-LABEL: End
1922+
sil hidden [noinline] @testUnreachable : $@convention(thin) () -> () {
1923+
bb0:
1924+
%g1 = global_addr @globalInt : $*Int
1925+
%a1 = begin_access [read] [dynamic] [no_nested_conflict] %g1 : $*Int // users: %18, %17
1926+
%v1 = load %a1 : $*Int
1927+
end_access %a1 : $*Int
1928+
br bb4
1929+
1930+
bb2:
1931+
%g2 = global_addr @globalInt : $*Int
1932+
%a2 = begin_access [read] [dynamic] [no_nested_conflict] %g2 : $*Int // users: %18, %17
1933+
%v2 = load %a2 : $*Int
1934+
end_access %a2 : $*Int
1935+
br bb4
1936+
1937+
bb4:
1938+
%z = tuple ()
1939+
return %z : $()
1940+
}

0 commit comments

Comments
 (0)