Skip to content

Commit b2d1ac1

Browse files
committed
Add AccessPathVerification pass and run it in the pipeline.
1 parent 7e435fa commit b2d1ac1

File tree

6 files changed

+151
-1
lines changed

6 files changed

+151
-1
lines changed

include/swift/SIL/MemAccessUtils.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -844,6 +844,8 @@ class AccessPath {
844844

845845
int getOffset() const { return offset; }
846846

847+
bool hasUnknownOffset() const { return offset == UnknownOffset; }
848+
847849
/// Return true if this path contains \p subPath.
848850
bool contains(AccessPath subPath) const;
849851

include/swift/SILOptimizer/PassManager/Passes.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ PASS(AccessSummaryDumper, "access-summary-dump",
7070
"Dump Address Parameter Access Summary")
7171
PASS(AccessedStorageAnalysisDumper, "accessed-storage-analysis-dump",
7272
"Dump Accessed Storage Analysis Summaries")
73+
PASS(AccessPathVerification, "access-path-verification",
74+
"Verify Access Paths (and Accessed Storage)")
7375
PASS(AccessedStorageDumper, "accessed-storage-dump",
7476
"Dump Accessed Storage Information")
7577
PASS(AccessMarkerElimination, "access-marker-elim",

lib/SIL/Utils/MemAccessUtils.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1274,7 +1274,7 @@ void AccessPath::Index::print(raw_ostream &os) const {
12741274
os << '#' << getSubObjectIndex();
12751275
else {
12761276
os << '@';
1277-
if (getOffset() == AccessPath::UnknownOffset)
1277+
if (isUnknownOffset())
12781278
os << "Unknown";
12791279
else
12801280
os << getOffset();

lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,11 @@ static void addPerfDebugSerializationPipeline(SILPassPipelinePlan &P) {
423423
static void addPrepareOptimizationsPipeline(SILPassPipelinePlan &P) {
424424
P.startPipeline("PrepareOptimizationPasses");
425425

426+
// Verify AccessedStorage once in OSSA before optimizing.
427+
#ifndef NDEBUG
428+
P.addAccessPathVerification();
429+
#endif
430+
426431
P.addForEachLoopUnroll();
427432
P.addMandatoryCombine();
428433
P.addAccessMarkerElimination();
@@ -670,6 +675,11 @@ static void addLastChanceOptPassPipeline(SILPassPipelinePlan &P) {
670675
// A loop might have only one dynamic access now, i.e. hoistable
671676
P.addLICM();
672677

678+
// Verify AccessedStorage once again after optimizing and lowering OSSA.
679+
#ifndef NDEBUG
680+
P.addAccessPathVerification();
681+
#endif
682+
673683
// Only has an effect if the -assume-single-thread option is specified.
674684
P.addAssumeSingleThreaded();
675685

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
//===--- AccessPathVerification.cpp - verify access paths and storage -----===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
///
13+
/// Verify AccessPath computation. For the address of every memory operation in
14+
/// the module, compute the access path, compute all the users of that path,
15+
/// then verify that all users have the same access path.
16+
///
17+
/// This is potentially expensive, so there is a fast mode that limits the
18+
/// number of uses visited per path.
19+
///
20+
/// During full verification, also check that all addresses that share an
21+
/// AccessPath are covered when computed the use list of that AccessPath. This
22+
/// is important because optimizations may assume that the use list is
23+
/// complete.
24+
///
25+
//===----------------------------------------------------------------------===//
26+
27+
#define DEBUG_TYPE "access-path-verification"
28+
#include "swift/SIL/MemAccessUtils.h"
29+
#include "swift/SIL/PrettyStackTrace.h"
30+
#include "swift/SIL/SILFunction.h"
31+
#include "swift/SIL/SILInstruction.h"
32+
#include "swift/SIL/SILValue.h"
33+
#include "swift/SILOptimizer/PassManager/Passes.h"
34+
#include "swift/SILOptimizer/PassManager/Transforms.h"
35+
#include "llvm/Support/Debug.h"
36+
37+
using namespace swift;
38+
39+
namespace {
40+
41+
/// Verify access path and uses of each access.
42+
class AccessPathVerification : public SILModuleTransform {
43+
llvm::DenseMap<Operand *, AccessPath> useToPathMap;
44+
45+
// Transient uses
46+
llvm::SmallVector<Operand *, 64> uses;
47+
48+
public:
49+
void verifyAccessPath(Operand *operand) {
50+
auto pathAndBase = AccessPathWithBase::compute(operand->get());
51+
auto accessPath = pathAndBase.accessPath;
52+
if (!accessPath.isValid())
53+
return;
54+
55+
auto iterAndInserted = useToPathMap.try_emplace(operand, accessPath);
56+
// If this use was already computed from a previously visited path, make
57+
// sure the path we just computed matches.
58+
if (!iterAndInserted.second) {
59+
auto collectedFromPath = iterAndInserted.first->second;
60+
if (collectedFromPath != accessPath) {
61+
llvm::errs() << "Address use: " << *operand->getUser()
62+
<< " collected from path\n ";
63+
collectedFromPath.dump();
64+
llvm::errs() << " has different path\n ";
65+
accessPath.dump();
66+
operand->getUser()->getFunction()->dump();
67+
assert(false && "computed path does not match collected path");
68+
}
69+
return;
70+
}
71+
// This is a new path, so map all its uses.
72+
assert(uses.empty());
73+
pathAndBase.collectUses(uses, /*collectContainingUses*/ false);
74+
bool foundOperandUse = false;
75+
for (Operand *use : uses) {
76+
if (use == operand) {
77+
foundOperandUse = true;
78+
continue;
79+
}
80+
// (live) subobject projections within an access will be mapped later as a
81+
// separate path.
82+
switch (use->getUser()->getKind()) {
83+
default:
84+
break;
85+
case SILInstructionKind::StructElementAddrInst:
86+
case SILInstructionKind::TupleElementAddrInst:
87+
case SILInstructionKind::IndexAddrInst:
88+
continue;
89+
}
90+
auto iterAndInserted = useToPathMap.try_emplace(use, accessPath);
91+
if (!iterAndInserted.second) {
92+
llvm::errs() << "Address use: " << *operand->getUser()
93+
<< " with path...\n";
94+
pathAndBase.dump();
95+
llvm::errs() << " was not collected for: " << *use->getUser();
96+
llvm::errs() << " with path...\n";
97+
auto computedPath = iterAndInserted.first->second;
98+
computedPath.dump();
99+
use->getUser()->getFunction()->dump();
100+
assert(false && "missing collected use");
101+
}
102+
}
103+
if (!foundOperandUse && !accessPath.hasUnknownOffset()) {
104+
llvm::errs() << "Address use: " << *operand->getUser()
105+
<< " is not a use of path\n ";
106+
accessPath.dump();
107+
assert(false && "not a user of its own computed path ");
108+
}
109+
uses.clear();
110+
}
111+
112+
void run() override {
113+
for (auto &fn : *getModule()) {
114+
if (fn.empty())
115+
continue;
116+
117+
PrettyStackTraceSILFunction functionDumper("...", &fn);
118+
for (auto &bb : fn) {
119+
for (auto &inst : bb) {
120+
if (inst.mayReadOrWriteMemory())
121+
visitAccessedAddress(&inst, [this](Operand *operand) {
122+
return verifyAccessPath(operand);
123+
});
124+
}
125+
}
126+
useToPathMap.clear();
127+
}
128+
}
129+
};
130+
131+
} // end anonymous namespace
132+
133+
SILTransform *swift::createAccessPathVerification() {
134+
return new AccessPathVerification();
135+
}

lib/SILOptimizer/UtilityPasses/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
target_sources(swiftSILOptimizer PRIVATE
22
AADumper.cpp
33
AccessSummaryDumper.cpp
4+
AccessPathVerification.cpp
45
AccessedStorageAnalysisDumper.cpp
56
AccessedStorageDumper.cpp
67
BasicCalleePrinter.cpp

0 commit comments

Comments
 (0)