Skip to content

Commit 3da2cc9

Browse files
committed
Clarify comments.
1 parent f4c7d46 commit 3da2cc9

File tree

2 files changed

+18
-4
lines changed

2 files changed

+18
-4
lines changed

lib/SILOptimizer/Transforms/AccessEnforcementDom.cpp

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,8 +221,7 @@ DominatedAccessAnalysis::Result DominatedAccessAnalysis::analyze() && {
221221
}
222222
if (isa<BeginUnpairedAccessInst>(&I)) {
223223
// Unpaired accesses could be tracked, but are ignored because they are
224-
// mostly irrelevant and hard to test. Rather than making the results
225-
// conservative, we return an empty result.
224+
// mostly irrelevant and hard to test. Completely bail on this function.
226225
result.accessMap.clear();
227226
return std::move(result);
228227
}
@@ -456,6 +455,21 @@ bool DominatedAccessRemoval::checkDominatedAccess(
456455
// non-distinct read and isn't contained by another non-distinct read. The
457456
// containsRead and isInner flags from in DominatedAccessAnalysis answer this
458457
// conservatively.
458+
//
459+
// Note: Promoting an earlier access to a modify could cause a program to
460+
// trap when optimized even if the unoptimized program does not trap; the
461+
// original modify access may be on an unreachable code path. This is acceptable
462+
// because:
463+
//
464+
// (a) in theory, exclusivity violations do not need to be executed to be
465+
// considered program violations. Promoting the access does not introduce any
466+
// new conflict where one didn't already exist statically. Catching these
467+
// violations at runtime is only an implementation compromise, and the more true
468+
// violations are caught, the better.
469+
//
470+
// (b) in practice, this situation is so exceedingly unlikely that it won't
471+
// cause any pervasive usability problem where programs have stronger
472+
// enforcement only when optimized.
459473
bool DominatedAccessRemoval::optimizeDominatedAccess(
460474
BeginAccessInst *BAI, DomAccessedStorage currAccessInfo,
461475
const DominatingAccess &domAccess) {

stdlib/public/core/KeyPath.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1772,7 +1772,7 @@ func _getAtKeyPath<Root, Value>(
17721772
}
17731773

17741774
// The release that ends the access scope is guaranteed to happen
1775-
// immediate at the end_apply call because the continuation is a
1775+
// immediately at the end_apply call because the continuation is a
17761776
// runtime call with a manual release (access scopes cannot be extended).
17771777
@_silgen_name("_swift_modifyAtWritableKeyPath_impl")
17781778
public // runtime entrypoint
@@ -1784,7 +1784,7 @@ func _modifyAtWritableKeyPath_impl<Root, Value>(
17841784
}
17851785

17861786
// The release that ends the access scope is guaranteed to happen
1787-
// immediate at the end_apply call because the continuation is a
1787+
// immediately at the end_apply call because the continuation is a
17881788
// runtime call with a manual release (access scopes cannot be extended).
17891789
@_silgen_name("_swift_modifyAtReferenceWritableKeyPath_impl")
17901790
public // runtime entrypoint

0 commit comments

Comments
 (0)