Skip to content

Commit 06b9ed7

Browse files
committed
[Exclusivity] Switch static checking to use IndexTrie instead of ProjectionPath
IndexTrie is a more light-weight representation and it works well in this case. This requires recovering the represented sequence from an IndexTrieNode, so also add a getParent() method.
1 parent 32082f6 commit 06b9ed7

File tree

4 files changed

+126
-51
lines changed

4 files changed

+126
-51
lines changed

include/swift/SILOptimizer/Analysis/AccessSummaryAnalysis.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "swift/SIL/SILFunction.h"
2323
#include "swift/SIL/SILInstruction.h"
2424
#include "swift/SILOptimizer/Analysis/BottomUpIPAnalysis.h"
25+
#include "swift/SILOptimizer/Utils/IndexTrie.h"
2526
#include "llvm/ADT/DenseMap.h"
2627
#include "llvm/ADT/SmallVector.h"
2728

@@ -129,12 +130,27 @@ class AccessSummaryAnalysis : public BottomUpIPAnalysis {
129130
llvm::DenseMap<SILFunction *, FunctionInfo *> FunctionInfos;
130131

131132
llvm::SpecificBumpPtrAllocator<FunctionInfo> Allocator;
133+
134+
/// A trie of integer indices that gives pointer identity to a path of
135+
/// projections. This is shared between all functions in the module.
136+
IndexTrieNode *SubPathTrie;
137+
132138
public:
133-
AccessSummaryAnalysis() : BottomUpIPAnalysis(AnalysisKind::AccessSummary) {}
139+
AccessSummaryAnalysis() : BottomUpIPAnalysis(AnalysisKind::AccessSummary) {
140+
SubPathTrie = new IndexTrieNode();
141+
}
142+
143+
~AccessSummaryAnalysis() {
144+
delete SubPathTrie;
145+
}
134146

135147
/// Returns a summary of the accesses performed by the given function.
136148
const FunctionSummary &getOrCreateSummary(SILFunction *Fn);
137149

150+
IndexTrieNode *getSubPathTrieRoot() {
151+
return SubPathTrie;
152+
}
153+
138154
virtual void initialize(SILPassManager *PM) override {}
139155
virtual void invalidate() override;
140156
virtual void invalidate(SILFunction *F, InvalidationKind K) override;

include/swift/SILOptimizer/Utils/IndexTrie.h

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,12 @@ class IndexTrieNode {
2424
static const unsigned RootIdx = ~0U;
2525
unsigned Index;
2626
llvm::SmallVector<IndexTrieNode*, 8> Children;
27+
IndexTrieNode *Parent;
2728

2829
public:
29-
IndexTrieNode(): Index(RootIdx) {}
30+
IndexTrieNode(): Index(RootIdx), Parent(nullptr) {}
3031

31-
explicit IndexTrieNode(unsigned V): Index(V) {}
32+
explicit IndexTrieNode(unsigned V, IndexTrieNode *P): Index(V), Parent(P) {}
3233

3334
IndexTrieNode(IndexTrieNode &) =delete;
3435
IndexTrieNode &operator=(const IndexTrieNode&) =delete;
@@ -53,12 +54,29 @@ class IndexTrieNode {
5354
});
5455
if (I != Children.end() && (*I)->Index == Idx)
5556
return *I;
56-
auto *N = new IndexTrieNode(Idx);
57+
auto *N = new IndexTrieNode(Idx, this);
5758
Children.insert(I, N);
5859
return N;
5960
}
6061

6162
ArrayRef<IndexTrieNode*> getChildren() const { return Children; }
63+
64+
IndexTrieNode *getParent() const { return Parent; }
65+
66+
/// Returns true when the sequence of indices represented by this
67+
/// node is a prefix of the sequence represented by the passed-in node.
68+
bool isPrefixOf(const IndexTrieNode *Other) const {
69+
const IndexTrieNode *I = Other;
70+
71+
do {
72+
if (this == I)
73+
return true;
74+
75+
I = I->getParent();
76+
} while (I);
77+
78+
return false;
79+
}
6280
};
6381

6482
} // end namespace swift

lib/SILOptimizer/Analysis/AccessSummaryAnalysis.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,8 @@ AccessSummaryAnalysis::getOrCreateSummary(SILFunction *fn) {
301301
void AccessSummaryAnalysis::AccessSummaryAnalysis::invalidate() {
302302
FunctionInfos.clear();
303303
Allocator.DestroyAll();
304+
delete SubPathTrie;
305+
SubPathTrie = new IndexTrieNode();
304306
}
305307

306308
void AccessSummaryAnalysis::invalidate(SILFunction *F, InvalidationKind K) {

lib/SILOptimizer/Mandatory/DiagnoseStaticExclusivity.cpp

Lines changed: 86 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "swift/SIL/SILArgument.h"
3636
#include "swift/SIL/SILInstruction.h"
3737
#include "swift/SIL/Projection.h"
38+
#include "swift/SILOptimizer/Analysis/AccessSummaryAnalysis.h"
3839
#include "swift/SILOptimizer/Analysis/PostOrderAnalysis.h"
3940
#include "swift/SILOptimizer/PassManager/Passes.h"
4041
#include "swift/SILOptimizer/PassManager/Transforms.h"
@@ -167,10 +168,10 @@ class AccessedStorage {
167168
class RecordedAccess {
168169
private:
169170
BeginAccessInst *Inst;
170-
ProjectionPath SubPath;
171+
const IndexTrieNode *SubPath;
171172

172173
public:
173-
RecordedAccess(BeginAccessInst *BAI, const ProjectionPath &SubPath)
174+
RecordedAccess(BeginAccessInst *BAI, const IndexTrieNode *SubPath)
174175
: Inst(BAI), SubPath(SubPath) {}
175176

176177
BeginAccessInst *getInstruction() const { return Inst; }
@@ -179,15 +180,15 @@ class RecordedAccess {
179180

180181
SILLocation getAccessLoc() const { return Inst->getLoc(); }
181182

182-
const ProjectionPath &getSubPath() const { return SubPath; }
183+
const IndexTrieNode *getSubPath() const { return SubPath; }
183184
};
184185

185186
/// Records the in-progress accesses to a given sub path.
186187
class SubAccessInfo {
187188
public:
188-
SubAccessInfo(const ProjectionPath &P) : Path(P) {}
189+
SubAccessInfo(const IndexTrieNode *P) : Path(P) {}
189190

190-
ProjectionPath Path;
191+
const IndexTrieNode *Path;
191192

192193
/// The number of in-progress 'read' accesses (that is 'begin_access [read]'
193194
/// instructions that have not yet had the corresponding 'end_access').
@@ -202,7 +203,7 @@ class SubAccessInfo {
202203

203204
public:
204205
/// Increment the count for given access.
205-
void beginAccess(BeginAccessInst *BAI, const ProjectionPath &SubPath) {
206+
void beginAccess(BeginAccessInst *BAI, const IndexTrieNode *SubPath) {
206207
if (!FirstAccess) {
207208
assert(Reads == 0 && NonReads == 0);
208209
FirstAccess = RecordedAccess(BAI, SubPath);
@@ -250,14 +251,17 @@ class SubAccessInfo {
250251
}
251252

252253
bool conflictsWithAccess(SILAccessKind Kind,
253-
const ProjectionPath &SubPath) const {
254+
const IndexTrieNode *SubPath) const {
254255
if (!canConflictWithAccessOfKind(Kind))
255256
return false;
256257

257-
SubSeqRelation_t Relation = Path.computeSubSeqRelation(SubPath);
258-
// If the one path is a subsequence of the other (or they are the same)
259-
// then access the same storage.
260-
return (Relation != SubSeqRelation_t::Unknown);
258+
return pathsConflict(Path, SubPath);
259+
}
260+
261+
/// Returns true when the two subpaths access overlapping memory.
262+
bool pathsConflict(const IndexTrieNode *Path1,
263+
const IndexTrieNode *Path2) const {
264+
return Path1->isPrefixOf(Path2) || Path2->isPrefixOf(Path1);
261265
}
262266
};
263267

@@ -271,7 +275,7 @@ class AccessInfo {
271275
SubAccessVector SubAccesses;
272276

273277
/// Returns the SubAccess info for accessing at the given SubPath.
274-
SubAccessInfo &findOrCreateSubAccessInfo(const ProjectionPath &SubPath) {
278+
SubAccessInfo &findOrCreateSubAccessInfo(const IndexTrieNode *SubPath) {
275279
for (auto &Info : SubAccesses) {
276280
if (Info.Path == SubPath)
277281
return Info;
@@ -283,7 +287,7 @@ class AccessInfo {
283287

284288
SubAccessVector::const_iterator
285289
findFirstSubPathWithConflict(SILAccessKind OtherKind,
286-
const ProjectionPath &OtherSubPath) const {
290+
const IndexTrieNode *OtherSubPath) const {
287291
// Note this iteration requires deterministic ordering for repeatable
288292
// diagnostics.
289293
for (auto I = SubAccesses.begin(), E = SubAccesses.end(); I != E; ++I) {
@@ -299,7 +303,7 @@ class AccessInfo {
299303
// Returns the previous access when beginning an access of the given Kind will
300304
// result in a conflict with a previous access.
301305
Optional<RecordedAccess>
302-
conflictsWithAccess(SILAccessKind Kind, const ProjectionPath &SubPath) const {
306+
conflictsWithAccess(SILAccessKind Kind, const IndexTrieNode *SubPath) const {
303307
auto I = findFirstSubPathWithConflict(Kind, SubPath);
304308
if (I == SubAccesses.end())
305309
return None;
@@ -326,13 +330,13 @@ class AccessInfo {
326330
}
327331

328332
/// Increment the count for given access.
329-
void beginAccess(BeginAccessInst *BAI, const ProjectionPath &SubPath) {
333+
void beginAccess(BeginAccessInst *BAI, const IndexTrieNode *SubPath) {
330334
SubAccessInfo &SubAccess = findOrCreateSubAccessInfo(SubPath);
331335
SubAccess.beginAccess(BAI, SubPath);
332336
}
333337

334338
/// Decrement the count for given access.
335-
void endAccess(EndAccessInst *EAI, const ProjectionPath &SubPath) {
339+
void endAccess(EndAccessInst *EAI, const IndexTrieNode *SubPath) {
336340
SubAccessInfo &SubAccess = findOrCreateSubAccessInfo(SubPath);
337341
SubAccess.endAccess(EAI);
338342
}
@@ -580,36 +584,46 @@ tryFixItWithCallToCollectionSwapAt(const BeginAccessInst *Access1,
580584
/// that stored-property relaxation supports: struct stored properties
581585
/// and tuple elements.
582586
static std::string getPathDescription(DeclName BaseName, SILType BaseType,
583-
ProjectionPath SubPath, SILModule &M) {
587+
const IndexTrieNode *SubPath,
588+
SILModule &M) {
589+
// Walk the trie to the root to collection the sequence (in reverse order).
590+
llvm::SmallVector<unsigned, 4> ReversedIndices;
591+
const IndexTrieNode *I = SubPath;
592+
while (!I->isRoot()) {
593+
ReversedIndices.push_back(I->getIndex());
594+
I = I->getParent();
595+
}
596+
584597
std::string sbuf;
585598
llvm::raw_string_ostream os(sbuf);
586599

587600
os << "'" << BaseName;
588601

589602
SILType ContainingType = BaseType;
590-
for (auto &P : SubPath) {
603+
for (unsigned Index : reversed(ReversedIndices)) {
591604
os << ".";
592-
switch (P.getKind()) {
593-
case ProjectionKind::Struct:
594-
os << P.getVarDecl(ContainingType)->getBaseName();
595-
break;
596-
case ProjectionKind::Tuple: {
597-
// Use the tuple element's name if present, otherwise use its index.
598-
Type SwiftTy = ContainingType.getSwiftRValueType();
599-
TupleType *TupleTy = SwiftTy->getAs<TupleType>();
600-
assert(TupleTy && "Tuple projection on non-tuple type!?");
601605

602-
Identifier ElementName = TupleTy->getElement(P.getIndex()).getName();
606+
if (ContainingType.getAs<StructType>()) {
607+
NominalTypeDecl *D = ContainingType.getNominalOrBoundGenericNominal();
608+
auto Iter = D->getStoredProperties().begin();
609+
std::advance(Iter, Index);
610+
VarDecl *VD = *Iter;
611+
os << VD->getBaseName();
612+
ContainingType = ContainingType.getFieldType(VD, M);
613+
continue;
614+
}
615+
616+
if (auto TupleTy = ContainingType.getAs<TupleType>()) {
617+
Identifier ElementName = TupleTy->getElement(Index).getName();
603618
if (ElementName.empty())
604-
os << P.getIndex();
619+
os << Index;
605620
else
606621
os << ElementName;
607-
break;
608-
}
609-
default:
610-
llvm_unreachable("Unexpected projection kind in SubPath!");
622+
ContainingType = ContainingType.getTupleElementType(Index);
623+
continue;
611624
}
612-
ContainingType = P.getType(ContainingType, M);
625+
626+
llvm_unreachable("Unexpected type in projection SubPath!");
613627
}
614628

615629
os << "'";
@@ -784,8 +798,14 @@ static bool isCallToStandardLibrarySwap(ApplyInst *AI, ASTContext &Ctx) {
784798
return FD == Ctx.getSwap(nullptr);
785799
}
786800

787-
static SILInstruction *getSingleAddressProjectionUser(SILInstruction *I) {
801+
/// If the instruction is a field or tuple projection and it has a single
802+
/// user return a pair of the single user and the projection index.
803+
/// Otherwise, return a pair with the component nullptr and the second
804+
/// unspecified.
805+
static std::pair<SILInstruction *, unsigned>
806+
getSingleAddressProjectionUser(SILInstruction *I) {
788807
SILInstruction *SingleUser = nullptr;
808+
unsigned ProjectionIndex = 0;
789809

790810
for (Operand *Use : I->getUses()) {
791811
SILInstruction *User = Use->getUser();
@@ -794,28 +814,43 @@ static SILInstruction *getSingleAddressProjectionUser(SILInstruction *I) {
794814

795815
// We have more than a single user so bail.
796816
if (SingleUser)
797-
return nullptr;
817+
return std::make_pair(nullptr, 0);
798818

799819
switch (User->getKind()) {
800820
case ValueKind::StructElementAddrInst:
821+
ProjectionIndex = cast<StructElementAddrInst>(User)->getFieldNo();
822+
SingleUser = User;
823+
break;
801824
case ValueKind::TupleElementAddrInst:
825+
ProjectionIndex = cast<TupleElementAddrInst>(User)->getFieldNo();
802826
SingleUser = User;
803827
break;
804828
default:
805-
return nullptr;
829+
return std::make_pair(nullptr, 0);
806830
}
807831
}
808832

809-
return SingleUser;
833+
return std::make_pair(SingleUser, ProjectionIndex);
810834
}
811835

812-
static ProjectionPath findSubPathAccessed(BeginAccessInst *BAI) {
813-
ProjectionPath SubPath(BAI->getType(), BAI->getType());
836+
/// Returns an IndexTrieNode that represents the single subpath accessed from
837+
/// BAI or the root if no such node exists.
838+
static const IndexTrieNode *findSubPathAccessed(BeginAccessInst *BAI,
839+
IndexTrieNode *Root) {
840+
IndexTrieNode *SubPath = Root;
814841

842+
// For each single-user projection of BAI, construct or get a node
843+
// from the trie representing the index of the field or tuple element
844+
// accessed by that projection.
815845
SILInstruction *Iter = BAI;
846+
while (true) {
847+
std::pair<SILInstruction *, unsigned> ProjectionUser =
848+
getSingleAddressProjectionUser(Iter);
849+
if (!ProjectionUser.first)
850+
break;
816851

817-
while ((Iter = getSingleAddressProjectionUser(Iter))) {
818-
SubPath.push_back(Projection(Iter));
852+
SubPath = SubPath->getChild(ProjectionUser.second);
853+
Iter = ProjectionUser.first;
819854
}
820855

821856
return SubPath;
@@ -826,14 +861,15 @@ static ProjectionPath findSubPathAccessed(BeginAccessInst *BAI) {
826861
/// with. Otherwise, returns None.
827862
Optional<RecordedAccess> shouldReportAccess(const AccessInfo &Info,
828863
swift::SILAccessKind Kind,
829-
const ProjectionPath &SubPath) {
864+
const IndexTrieNode *SubPath) {
830865
if (Info.alreadyHadConflict())
831866
return None;
832867

833868
return Info.conflictsWithAccess(Kind, SubPath);
834869
}
835870

836-
static void checkStaticExclusivity(SILFunction &Fn, PostOrderFunctionInfo *PO) {
871+
static void checkStaticExclusivity(SILFunction &Fn, PostOrderFunctionInfo *PO,
872+
AccessSummaryAnalysis *ASA) {
837873
// The implementation relies on the following SIL invariants:
838874
// - All incoming edges to a block must have the same in-progress
839875
// accesses. This enables the analysis to not perform a data flow merge
@@ -899,7 +935,8 @@ static void checkStaticExclusivity(SILFunction &Fn, PostOrderFunctionInfo *PO) {
899935
SILAccessKind Kind = BAI->getAccessKind();
900936
const AccessedStorage &Storage = findAccessedStorage(BAI->getSource());
901937
AccessInfo &Info = Accesses[Storage];
902-
ProjectionPath SubPath = findSubPathAccessed(BAI);
938+
const IndexTrieNode *SubPath =
939+
findSubPathAccessed(BAI, ASA->getSubPathTrieRoot());
903940
if (auto Conflict = shouldReportAccess(Info, Kind, SubPath)) {
904941
ConflictingAccesses.emplace_back(Storage, *Conflict,
905942
RecordedAccess(BAI, SubPath));
@@ -914,7 +951,8 @@ static void checkStaticExclusivity(SILFunction &Fn, PostOrderFunctionInfo *PO) {
914951
AccessInfo &Info = It->getSecond();
915952

916953
BeginAccessInst *BAI = EAI->getBeginAccess();
917-
const ProjectionPath &SubPath = findSubPathAccessed(BAI);
954+
const IndexTrieNode *SubPath =
955+
findSubPathAccessed(BAI, ASA->getSubPathTrieRoot());
918956
Info.endAccess(EAI, SubPath);
919957

920958
// If the storage location has no more in-progress accesses, remove
@@ -958,7 +996,8 @@ class DiagnoseStaticExclusivity : public SILFunctionTransform {
958996
return;
959997

960998
PostOrderFunctionInfo *PO = getAnalysis<PostOrderAnalysis>()->get(Fn);
961-
checkStaticExclusivity(*Fn, PO);
999+
auto *ASA = getAnalysis<AccessSummaryAnalysis>();
1000+
checkStaticExclusivity(*Fn, PO, ASA);
9621001
}
9631002
};
9641003

0 commit comments

Comments
 (0)