Skip to content

Commit 19e5256

Browse files
author
Joe Shajrawi
committed
[AccessEnforcementOpts] Fix an iterator invalidation issue
visitSetForConflicts Had a bug wherein we might change the accessSet while iterating over it by recording a conflict. This makes our current iterator invalid. Work-around this issue by restarting the iteration in case we made any changes to the accessSet Similarly, we do that for anywhere where in we recorded a conflict
1 parent 096e6ad commit 19e5256

File tree

1 file changed

+59
-41
lines changed

1 file changed

+59
-41
lines changed

lib/SILOptimizer/Transforms/AccessEnforcementOpts.cpp

Lines changed: 59 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -638,25 +638,31 @@ void AccessConflictAndMergeAnalysis::visitBeginAccess(
638638
}
639639
SILAccessKind beginAccessKind = beginAccess->getAccessKind();
640640
// check the current in-scope accesses for conflicts:
641-
for (auto pair : info.getInScopeAccesses()) {
642-
auto *outerBeginAccess = pair.second;
643-
// If both are reads, keep the mapped access.
644-
if (!accessKindMayConflict(beginAccessKind,
645-
outerBeginAccess->getAccessKind())) {
646-
continue;
647-
}
641+
bool changed = false;
642+
do {
643+
changed = false;
644+
for (auto pair : info.getInScopeAccesses()) {
645+
auto *outerBeginAccess = pair.second;
646+
// If both are reads, keep the mapped access.
647+
if (!accessKindMayConflict(beginAccessKind,
648+
outerBeginAccess->getAccessKind())) {
649+
continue;
650+
}
648651

649-
auto &outerAccessInfo = result.getAccessInfo(outerBeginAccess);
650-
// If there is no potential conflict, leave the outer access mapped.
651-
if (!outerAccessInfo.isDistinctFrom(beginAccessInfo))
652-
continue;
652+
auto &outerAccessInfo = result.getAccessInfo(outerBeginAccess);
653+
// If there is no potential conflict, leave the outer access mapped.
654+
if (!outerAccessInfo.isDistinctFrom(beginAccessInfo))
655+
continue;
653656

654-
LLVM_DEBUG(beginAccessInfo.dump(); llvm::dbgs() << " may conflict with:\n";
655-
outerAccessInfo.dump());
657+
LLVM_DEBUG(beginAccessInfo.dump();
658+
llvm::dbgs() << " may conflict with:\n";
659+
outerAccessInfo.dump());
656660

657-
recordConflict(info, outerAccessInfo);
658-
break;
659-
}
661+
recordConflict(info, outerAccessInfo);
662+
changed = true;
663+
break;
664+
}
665+
} while (changed);
660666

661667
// Record the current access to InScopeAccesses.
662668
// It can potentially be folded
@@ -691,22 +697,27 @@ void AccessConflictAndMergeAnalysis::detectApplyConflicts(
691697
const swift::FunctionAccessedStorage &callSiteAccesses,
692698
const DenseAccessMap &conflictFreeSet,
693699
const swift::FullApplySite &fullApply, RegionInfo &info) {
694-
for (auto pair : conflictFreeSet) {
695-
auto *outerBeginAccess = pair.second;
696-
// If there is no potential conflict, leave the outer access mapped.
697-
SILAccessKind accessKind = outerBeginAccess->getAccessKind();
698-
AccessInfo &outerAccessInfo = result.getAccessInfo(outerBeginAccess);
699-
if (!callSiteAccesses.mayConflictWith(accessKind, outerAccessInfo))
700-
continue;
700+
bool changed = false;
701+
do {
702+
changed = false;
703+
for (auto pair : conflictFreeSet) {
704+
auto *outerBeginAccess = pair.second;
705+
// If there is no potential conflict, leave the outer access mapped.
706+
SILAccessKind accessKind = outerBeginAccess->getAccessKind();
707+
AccessInfo &outerAccessInfo = result.getAccessInfo(outerBeginAccess);
708+
if (!callSiteAccesses.mayConflictWith(accessKind, outerAccessInfo))
709+
continue;
701710

702-
LLVM_DEBUG(
703-
llvm::dbgs() << *fullApply.getInstruction() << " call site access: ";
704-
callSiteAccesses.dump(); llvm::dbgs() << " may conflict with:\n";
705-
outerAccessInfo.dump());
711+
LLVM_DEBUG(
712+
llvm::dbgs() << *fullApply.getInstruction() << " call site access: ";
713+
callSiteAccesses.dump(); llvm::dbgs() << " may conflict with:\n";
714+
outerAccessInfo.dump());
706715

707-
recordConflict(info, outerAccessInfo);
708-
break;
709-
}
716+
recordConflict(info, outerAccessInfo);
717+
changed = true;
718+
break;
719+
}
720+
} while (changed);
710721
}
711722

712723
void AccessConflictAndMergeAnalysis::visitFullApply(FullApplySite fullApply,
@@ -761,18 +772,25 @@ void AccessConflictAndMergeAnalysis::mergePredAccesses(
761772
void AccessConflictAndMergeAnalysis::visitSetForConflicts(
762773
const DenseAccessMap &accessSet, RegionInfo &info,
763774
AccessConflictAndMergeAnalysis::AccessedStorageSet &loopStorage) {
764-
for (auto pair : accessSet) {
765-
BeginAccessInst *beginAccess = pair.second;
766-
AccessInfo &accessInfo = result.getAccessInfo(beginAccess);
767-
768-
for (auto loopAccess : loopStorage) {
769-
if (loopAccess.isDistinctFrom(accessInfo) && !info.unidentifiedAccess)
770-
continue;
771-
772-
recordConflict(info, loopAccess);
773-
break;
775+
bool changed = false;
776+
do {
777+
changed = false;
778+
for (auto pair : accessSet) {
779+
BeginAccessInst *beginAccess = pair.second;
780+
AccessInfo &accessInfo = result.getAccessInfo(beginAccess);
781+
782+
for (auto loopAccess : loopStorage) {
783+
if (loopAccess.isDistinctFrom(accessInfo) && !info.unidentifiedAccess)
784+
continue;
785+
786+
recordConflict(info, loopAccess);
787+
changed = true;
788+
break;
789+
}
790+
if (changed)
791+
break;
774792
}
775-
}
793+
} while (changed);
776794
}
777795

778796
void AccessConflictAndMergeAnalysis::detectConflictsInLoop(

0 commit comments

Comments
 (0)