Skip to content

Commit 7281a76

Browse files
author
Joe Shajrawi
committed
[AccessEnforcementOpts] Add mergeAccesses optimization
1 parent 62e43a1 commit 7281a76

File tree

5 files changed

+235
-21
lines changed

5 files changed

+235
-21
lines changed

lib/SILOptimizer/LoopTransforms/LICM.cpp

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "swift/SIL/SILArgument.h"
1919
#include "swift/SIL/SILBuilder.h"
2020
#include "swift/SIL/SILInstruction.h"
21+
#include "swift/SILOptimizer/Analysis/AccessedStorageAnalysis.h"
2122
#include "swift/SILOptimizer/Analysis/AliasAnalysis.h"
2223
#include "swift/SILOptimizer/Analysis/Analysis.h"
2324
#include "swift/SILOptimizer/Analysis/ArraySemantic.h"
@@ -361,6 +362,7 @@ class LoopTreeOptimization {
361362
AliasAnalysis *AA;
362363
SideEffectAnalysis *SEA;
363364
DominanceInfo *DomTree;
365+
AccessedStorageAnalysis *ASA;
364366
bool Changed;
365367

366368
/// True if LICM is done on high-level SIL, i.e. semantic calls are not
@@ -380,8 +382,9 @@ class LoopTreeOptimization {
380382
public:
381383
LoopTreeOptimization(SILLoop *TopLevelLoop, SILLoopInfo *LI,
382384
AliasAnalysis *AA, SideEffectAnalysis *SEA,
383-
DominanceInfo *DT, bool RunsOnHighLevelSil)
384-
: LoopInfo(LI), AA(AA), SEA(SEA), DomTree(DT), Changed(false),
385+
DominanceInfo *DT, AccessedStorageAnalysis *ASA,
386+
bool RunsOnHighLevelSil)
387+
: LoopInfo(LI), AA(AA), SEA(SEA), DomTree(DT), ASA(ASA), Changed(false),
385388
RunsOnHighLevelSIL(RunsOnHighLevelSil) {
386389
// Collect loops for a recursive bottom-up traversal in the loop tree.
387390
BotUpWorkList.push_back(TopLevelLoop);
@@ -528,9 +531,10 @@ static bool handledEndAccesses(BeginAccessInst *BI, SILLoop *Loop) {
528531
return true;
529532
}
530533

531-
static bool
532-
analyzeBeginAccess(BeginAccessInst *BI,
533-
SmallVector<BeginAccessInst *, 8> &BeginAccesses) {
534+
static bool analyzeBeginAccess(BeginAccessInst *BI,
535+
SmallVector<BeginAccessInst *, 8> &BeginAccesses,
536+
SmallVector<FullApplySite, 8> &fullApplies,
537+
AccessedStorageAnalysis *ASA) {
534538
if (BI->getEnforcement() != SILAccessEnforcement::Dynamic) {
535539
return false;
536540
}
@@ -550,8 +554,18 @@ analyzeBeginAccess(BeginAccessInst *BI,
550554
findAccessedStorageNonNested(OtherBI));
551555
};
552556

553-
return (
554-
std::all_of(BeginAccesses.begin(), BeginAccesses.end(), safeBeginPred));
557+
if (!std::all_of(BeginAccesses.begin(), BeginAccesses.end(), safeBeginPred))
558+
return false;
559+
560+
for (auto fullApply : fullApplies) {
561+
FunctionAccessedStorage callSiteAccesses;
562+
ASA->getCallSiteEffects(callSiteAccesses, fullApply);
563+
SILAccessKind accessKind = BI->getAccessKind();
564+
if (callSiteAccesses.mayConflictWith(accessKind, storage))
565+
return false;
566+
}
567+
568+
return true;
555569
}
556570

557571
// Analyzes current loop for hosting/sinking potential:
@@ -575,6 +589,8 @@ void LoopTreeOptimization::analyzeCurrentLoop(
575589
SmallVector<FixLifetimeInst *, 8> FixLifetimes;
576590
// Contains begin_access, we might be able to hoist them.
577591
SmallVector<BeginAccessInst *, 8> BeginAccesses;
592+
// Contains all applies - used for begin_access
593+
SmallVector<FullApplySite, 8> fullApplies;
578594

579595
for (auto *BB : Loop->getBlocks()) {
580596
for (auto &Inst : *BB) {
@@ -618,6 +634,9 @@ void LoopTreeOptimization::analyzeCurrentLoop(
618634
LLVM_FALLTHROUGH;
619635
}
620636
default: {
637+
if (auto fullApply = FullApplySite::isa(&Inst)) {
638+
fullApplies.push_back(fullApply);
639+
}
621640
checkSideEffects(Inst, MayWrites);
622641
if (canHoistUpDefault(&Inst, Loop, DomTree, RunsOnHighLevelSIL)) {
623642
HoistUp.insert(&Inst);
@@ -660,7 +679,7 @@ void LoopTreeOptimization::analyzeCurrentLoop(
660679
LLVM_DEBUG(llvm::dbgs() << "Some end accesses can't be handled\n");
661680
continue;
662681
}
663-
if (analyzeBeginAccess(BI, BeginAccesses)) {
682+
if (analyzeBeginAccess(BI, BeginAccesses, fullApplies, ASA)) {
664683
SpecialHoist.insert(BI);
665684
}
666685
}
@@ -711,14 +730,15 @@ class LICM : public SILFunctionTransform {
711730
DominanceAnalysis *DA = PM->getAnalysis<DominanceAnalysis>();
712731
AliasAnalysis *AA = PM->getAnalysis<AliasAnalysis>();
713732
SideEffectAnalysis *SEA = PM->getAnalysis<SideEffectAnalysis>();
733+
AccessedStorageAnalysis *ASA = getAnalysis<AccessedStorageAnalysis>();
714734
DominanceInfo *DomTree = nullptr;
715735

716736
LLVM_DEBUG(llvm::dbgs() << "Processing loops in " << F->getName() << "\n");
717737
bool Changed = false;
718738

719739
for (auto *TopLevelLoop : *LoopInfo) {
720740
if (!DomTree) DomTree = DA->get(F);
721-
LoopTreeOptimization Opt(TopLevelLoop, LoopInfo, AA, SEA, DomTree,
741+
LoopTreeOptimization Opt(TopLevelLoop, LoopInfo, AA, SEA, DomTree, ASA,
722742
RunsOnHighLevelSil);
723743
Changed |= Opt.optimize();
724744
}

lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,10 @@ void addHighLevelLoopOptPasses(SILPassPipelinePlan &P) {
199199
P.addHighLevelLICM();
200200
// Simplify CFG after LICM that creates new exit blocks
201201
P.addSimplifyCFG();
202+
// LICM might have added new merging potential by hoisting
203+
// we don't want to restart the pipeline - ignore the
204+
// potential of merging out of two loops
205+
P.addAccessEnforcementOpts();
202206
// Start of loop unrolling passes.
203207
P.addArrayCountPropagation();
204208
// To simplify induction variable.
@@ -457,6 +461,10 @@ static void addLateLoopOptPassPipeline(SILPassPipelinePlan &P) {
457461
P.addLICM();
458462
// Simplify CFG after LICM that creates new exit blocks
459463
P.addSimplifyCFG();
464+
// LICM might have added new merging potential by hoisting
465+
// we don't want to restart the pipeline - ignore the
466+
// potential of merging out of two loops
467+
P.addAccessEnforcementOpts();
460468

461469
// Optimize overflow checks.
462470
P.addRedundantOverflowCheckRemoval();

lib/SILOptimizer/Transforms/AccessEnforcementOpts.cpp

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@
6565
/// any point in the pipeline. Until then, we could settle for a partially
6666
/// working AccessEnforcementSelection, or expand it somewhat to handle
6767
/// alloc_stack.
68+
///
69+
/// **Access marker merger**
70+
///
71+
/// When a pair of non-overlapping accesses, where the first access dominates
72+
/// the second and there are no conflicts on the same storage in the paths
73+
/// between them, and they are part of the same sub-region
74+
/// be it the same block or the sampe loop, merge those accesses to create
75+
/// a new, larger, scope with a single begin_access for the accesses.
6876
//===----------------------------------------------------------------------===//
6977

7078
#define DEBUG_TYPE "access-enforcement-opts"
@@ -73,10 +81,12 @@
7381
#include "swift/SIL/MemAccessUtils.h"
7482
#include "swift/SIL/SILFunction.h"
7583
#include "swift/SILOptimizer/Analysis/AccessedStorageAnalysis.h"
84+
#include "swift/SILOptimizer/Analysis/DominanceAnalysis.h"
7685
#include "swift/SILOptimizer/Analysis/LoopRegionAnalysis.h"
7786
#include "swift/SILOptimizer/PassManager/Transforms.h"
7887
#include "swift/SILOptimizer/Utils/Local.h"
7988
#include "llvm/ADT/MapVector.h"
89+
#include "llvm/ADT/SCCIterator.h"
8090

8191
using namespace swift;
8292

@@ -871,6 +881,172 @@ removeLocalNonNestedAccess(const AccessConflictAndMergeAnalysis::Result &result,
871881
return changed;
872882
}
873883

884+
// TODO: support multi-end access cases
885+
static EndAccessInst *getSingleEndAccess(BeginAccessInst *inst) {
886+
EndAccessInst *end = nullptr;
887+
for (auto *currEnd : inst->getEndAccesses()) {
888+
if (end == nullptr)
889+
end = currEnd;
890+
else
891+
return nullptr;
892+
}
893+
return end;
894+
}
895+
896+
struct SCCInfo {
897+
unsigned id;
898+
bool hasLoop;
899+
};
900+
901+
static void mergeEndAccesses(BeginAccessInst *parentIns,
902+
BeginAccessInst *childIns) {
903+
auto *endP = getSingleEndAccess(parentIns);
904+
if (!endP)
905+
llvm_unreachable("not supported");
906+
auto *endC = getSingleEndAccess(childIns);
907+
if (!endC)
908+
llvm_unreachable("not supported");
909+
910+
endC->setOperand(parentIns);
911+
endP->eraseFromParent();
912+
}
913+
914+
static bool canMergeEnd(BeginAccessInst *parentIns, BeginAccessInst *childIns) {
915+
auto *endP = getSingleEndAccess(parentIns);
916+
if (!endP)
917+
return false;
918+
919+
auto *endC = getSingleEndAccess(childIns);
920+
if (!endC)
921+
return false;
922+
923+
return true;
924+
}
925+
926+
// TODO: support other merge patterns
927+
static bool
928+
canMergeBegin(PostDominanceInfo *postDomTree,
929+
const llvm::DenseMap<SILBasicBlock *, SCCInfo> &blockToSCCMap,
930+
BeginAccessInst *parentIns, BeginAccessInst *childIns) {
931+
if (!postDomTree->properlyDominates(childIns, parentIns)) {
932+
return false;
933+
}
934+
auto parentSCCIt = blockToSCCMap.find(parentIns->getParent());
935+
assert(parentSCCIt != blockToSCCMap.end() && "Expected block in SCC Map");
936+
auto childSCCIt = blockToSCCMap.find(childIns->getParent());
937+
assert(childSCCIt != blockToSCCMap.end() && "Expected block in SCC Map");
938+
auto parentSCC = parentSCCIt->getSecond();
939+
auto childSCC = childSCCIt->getSecond();
940+
if (parentSCC.id == childSCC.id) {
941+
return true;
942+
}
943+
if (parentSCC.hasLoop) {
944+
return false;
945+
}
946+
if (childSCC.hasLoop) {
947+
return false;
948+
}
949+
return true;
950+
}
951+
952+
static bool
953+
canMerge(PostDominanceInfo *postDomTree,
954+
const llvm::DenseMap<SILBasicBlock *, SCCInfo> &blockToSCCMap,
955+
BeginAccessInst *parentIns, BeginAccessInst *childIns) {
956+
if (!canMergeBegin(postDomTree, blockToSCCMap, parentIns, childIns))
957+
return false;
958+
959+
return canMergeEnd(parentIns, childIns);
960+
}
961+
962+
/// Perform access merging.
963+
static bool mergeAccesses(
964+
SILFunction *F, PostDominanceInfo *postDomTree,
965+
const AccessConflictAndMergeAnalysis::MergeablePairs &mergePairs) {
966+
bool changed = false;
967+
968+
// Compute a map from each block to its SCC -
969+
// For now we can't merge cross SCC boundary
970+
llvm::DenseMap<SILBasicBlock *, SCCInfo> blockToSCCMap;
971+
SCCInfo info;
972+
info.id = 0;
973+
for (auto sccIt = scc_begin(F); !sccIt.isAtEnd(); ++sccIt) {
974+
++info.id;
975+
info.hasLoop = sccIt.hasLoop();
976+
for (auto *bb : *sccIt) {
977+
blockToSCCMap.insert(std::make_pair(bb, info));
978+
}
979+
}
980+
// make a temporary reverse copy to work on:
981+
// It is in reverse order just to make it easier to debug / follow
982+
AccessConflictAndMergeAnalysis::MergeablePairs workPairs;
983+
workPairs.append(mergePairs.rbegin(), mergePairs.rend());
984+
985+
// Assume the result contains two access pairs to be merged:
986+
// (begin_access %1, begin_access %2)
987+
// = merge end_access %1 with begin_access %2
988+
// (begin_access %2, begin_access %3)
989+
// = merge end_access %2 with begin_access %3
990+
// After merging the first pair, begin_access %2 is removed,
991+
// so the second pair in the result list points to a to-be-deleted
992+
// begin_access instruction. We store (begin_access %2 -> begin_access %1)
993+
// to re-map a merged begin_access to it's replaced instruction.
994+
llvm::DenseMap<BeginAccessInst *, BeginAccessInst *> oldToNewMap;
995+
996+
while (!workPairs.empty()) {
997+
auto curr = workPairs.pop_back_val();
998+
auto *parentIns = curr.first;
999+
auto *childIns = curr.second;
1000+
if (oldToNewMap.count(parentIns) != 0) {
1001+
parentIns = oldToNewMap[parentIns];
1002+
}
1003+
assert(oldToNewMap.count(childIns) == 0 &&
1004+
"Can't have same child instruction twice in map");
1005+
1006+
// The optimization might not currently support every mergeable pair
1007+
// If the current pattern is not supported - skip
1008+
if (!canMerge(postDomTree, blockToSCCMap, parentIns, childIns))
1009+
continue;
1010+
1011+
LLVM_DEBUG(llvm::dbgs()
1012+
<< "Merging: " << *childIns << " into " << *parentIns << "\n");
1013+
1014+
// Change the type of access of parent:
1015+
// should be the worse between it and child
1016+
auto childAccess = childIns->getAccessKind();
1017+
if (parentIns->getAccessKind() < childAccess) {
1018+
parentIns->setAccessKind(childAccess);
1019+
}
1020+
1021+
// Change the no nested conflict of parent:
1022+
// should be the worst case scenario: we might merge to non-conflicting
1023+
// scopes to a conflicting one. f the new result does not conflict,
1024+
// a later on pass will remove the flag
1025+
parentIns->setNoNestedConflict(false);
1026+
1027+
// remove end accesses and create new ones that cover bigger scope:
1028+
mergeEndAccesses(parentIns, childIns);
1029+
1030+
// In case the child instruction is at the map,
1031+
// updated the oldToNewMap to reflect that we are getting rid of it:
1032+
oldToNewMap.insert(std::make_pair(childIns, parentIns));
1033+
1034+
// Modify the users of child instruction to use the parent:
1035+
childIns->replaceAllUsesWith(parentIns);
1036+
1037+
changed = true;
1038+
}
1039+
1040+
// Delete all old instructions from parent scopes:
1041+
while (!oldToNewMap.empty()) {
1042+
auto curr = oldToNewMap.begin();
1043+
auto *oldIns = curr->getFirst();
1044+
oldToNewMap.erase(oldIns);
1045+
oldIns->eraseFromParent();
1046+
}
1047+
return changed;
1048+
}
1049+
8741050
namespace {
8751051
struct AccessEnforcementOpts : public SILFunctionTransform {
8761052
void run() override {
@@ -905,6 +1081,14 @@ struct AccessEnforcementOpts : public SILFunctionTransform {
9051081
const FunctionAccessedStorage &functionAccess = ASA->getEffects(F);
9061082
if (removeLocalNonNestedAccess(result, functionAccess))
9071083
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
1084+
1085+
// Perform the access merging
1086+
// The inital version of the optimization requires a postDomTree
1087+
PostDominanceAnalysis *postDomAnalysis =
1088+
getAnalysis<PostDominanceAnalysis>();
1089+
PostDominanceInfo *postDomTree = postDomAnalysis->get(F);
1090+
if (mergeAccesses(F, postDomTree, result.mergePairs))
1091+
invalidateAnalysis(SILAnalysis::InvalidationKind::Instructions);
9081092
}
9091093
};
9101094
} // namespace

test/SILOptimizer/access_enforcement_opts.sil

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -365,8 +365,8 @@ bb0(%0 : $*Int64):
365365
// CHECK-LABEL: sil @testInoutReadEscapeRead : $@convention(thin) () -> () {
366366
// CHECK: [[BOX:%.*]] = alloc_box ${ var Int64 }, var, name "x"
367367
// CHECK: [[BOXADR:%.*]] = project_box [[BOX]] : ${ var Int64 }, 0
368-
// CHECK: begin_access [read] [static] [no_nested_conflict] [[BOXADR]] : $*Int64
369-
// CHECK: begin_access [read] [static] [no_nested_conflict] [[BOXADR]] : $*Int64
368+
// CHECK: begin_access [read] [static] [[BOXADR]] : $*Int64
369+
// CHECK-NOT: begin_access
370370
// CHECK-LABEL: } // end sil function 'testInoutReadEscapeRead'
371371
sil @testInoutReadEscapeRead : $@convention(thin) () -> () {
372372
bb0:
@@ -427,9 +427,11 @@ bb0(%0 : ${ var Int64 }):
427427
// CHECK-LABEL: sil @testInoutReadEscapeWrite : $@convention(thin) () -> () {
428428
// CHECK: [[BOX:%.*]] = alloc_box ${ var Int64 }, var, name "x"
429429
// CHECK: [[BOXADR:%.*]] = project_box [[BOX]] : ${ var Int64 }, 0
430-
// CHECK: begin_access [read] [dynamic] [[BOXADR]] : $*Int64
430+
// CHECK: [[BEGIN:%.*]] = begin_access [read] [dynamic] [[BOXADR]] : $*Int64
431431
// CHECK: apply
432-
// CHECK: begin_access [read] [dynamic] [no_nested_conflict] [[BOXADR]] : $*Int64
432+
// CHECK-NOT: begin_access
433+
// CHECK: end_access [[BEGIN]]
434+
// CHECK-NOT: begin_access
433435
// CHECK-LABEL: } // end sil function 'testInoutReadEscapeWrite'
434436
sil @testInoutReadEscapeWrite : $@convention(thin) () -> () {
435437
bb0:
@@ -490,9 +492,11 @@ bb0(%0 : ${ var Int64 }):
490492
// CHECK-LABEL: sil @$S17enforce_with_opts24testInoutWriteEscapeReadyyF : $@convention(thin) () -> () {
491493
// CHECK: [[BOX:%.*]] = alloc_box ${ var Int64 }, var, name "x"
492494
// CHECK: [[BOXADR:%.*]] = project_box [[BOX]] : ${ var Int64 }, 0
493-
// CHECK: begin_access [modify] [dynamic] [[BOXADR]] : $*Int64
495+
// CHECK: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[BOXADR]] : $*Int64
494496
// CHECK: apply
495-
// CHECK: begin_access [read] [dynamic] [no_nested_conflict] %1 : $*Int64
497+
// CHECK-NOT: begin_access
498+
// CHECK: end_access [[BEGIN]]
499+
// CHECK-NOT: begin_access
496500
// CHECK-LABEL: } // end sil function '$S17enforce_with_opts24testInoutWriteEscapeReadyyF'
497501
sil @$S17enforce_with_opts24testInoutWriteEscapeReadyyF : $@convention(thin) () -> () {
498502
bb0:
@@ -561,9 +565,11 @@ bb0(%0 : ${ var Int64 }):
561565
// CHECK-LABEL: sil @$S17enforce_with_opts020testInoutWriteEscapeF0yyF : $@convention(thin) () -> () {
562566
// CHECK: [[BOX:%.*]] = alloc_box ${ var Int64 }, var, name "x"
563567
// CHECK: [[BOXADR:%.*]] = project_box [[BOX]] : ${ var Int64 }, 0
564-
// CHECK: begin_access [modify] [dynamic] [[BOXADR]] : $*Int64
568+
// CHECK: [[BEGIN:%.*]] = begin_access [modify] [dynamic] [[BOXADR]] : $*Int64
565569
// CHECK: apply
566-
// CHECK: begin_access [read] [dynamic] [no_nested_conflict] [[BOXADR]] : $*Int64
570+
// CHECK-NOT: begin_access
571+
// CHECK: end_access [[BEGIN]]
572+
// CHECK-NOT: begin_access
567573
// CHECK-LABEL: } // end sil function '$S17enforce_with_opts020testInoutWriteEscapeF0yyF'
568574
sil @$S17enforce_with_opts020testInoutWriteEscapeF0yyF : $@convention(thin) () -> () {
569575
bb0:

0 commit comments

Comments
 (0)