Skip to content

Commit f3f5848

Browse files
committed
Remove a stale UnenforcedAccess marker from SILGenPattern.
NFC unless -enable-verify-exclusivity is on. Boxed (indirect) enum loads are not formal accesses so they should not have access markers. For a while, we used UnenforcedAccess in this case to signal to the verifier that this is actually a safe load. But that isn't needed. The verifier just ignores loads from unidentified storage instead. There are two code paths that had this UnenforcedAccess marker. One was cleaned up, and the other was not. So finish the cleanup and add a unit test for both SILGen code paths.
1 parent baf2e4e commit f3f5848

File tree

2 files changed

+31
-28
lines changed

2 files changed

+31
-28
lines changed

lib/SILGen/SILGenPattern.cpp

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1920,12 +1920,7 @@ void PatternMatchEmission::emitEnumElementObjectDispatch(
19201920
ManagedValue boxedValue =
19211921
SGF.B.createProjectBox(loc, origCMV.getFinalManagedValue(), 0);
19221922
eltTL = &SGF.getTypeLowering(boxedValue.getType());
1923-
1924-
// TODO: If we have something that is not loadable
19251923
if (eltTL->isLoadable()) {
1926-
// The boxed value may be shared, so we need to load the value at +0
1927-
// to make sure that we copy if we try to use it outside of the switch
1928-
// statement itself.
19291924
boxedValue = SGF.B.createLoadBorrow(loc, boxedValue);
19301925
eltCMV = {boxedValue, CastConsumptionKind::BorrowAlways};
19311926
} else {
@@ -2151,37 +2146,20 @@ void PatternMatchEmission::emitEnumElementDispatch(
21512146

21522147
// If the payload is boxed, project it.
21532148
if (elt->isIndirect() || elt->getParentEnum()->isIndirect()) {
2154-
SILValue boxedValue = SGF.B.createProjectBox(loc, origCMV.getValue(), 0);
2155-
eltTL = &SGF.getTypeLowering(boxedValue->getType());
2149+
ManagedValue boxedValue =
2150+
SGF.B.createProjectBox(loc, origCMV.getFinalManagedValue(), 0);
2151+
eltTL = &SGF.getTypeLowering(boxedValue.getType());
21562152
if (eltTL->isLoadable()) {
2157-
UnenforcedAccess access;
2158-
SILValue accessAddress =
2159-
access.beginAccess(SGF, loc, boxedValue, SILAccessKind::Read);
2160-
2161-
// If we needed to do another begin_access, we need to perform a
2162-
// load_copy. This is because we are going to immediately close the
2163-
// access here. If we are already in a different access, we can
2164-
// perform a load_borrow instead here.
2165-
auto accessMV = ManagedValue::forUnmanaged(accessAddress);
2166-
ManagedValue newLoadedBoxValue;
2167-
if (accessAddress == boxedValue) {
2168-
eltCMV = {SGF.B.createLoadBorrow(loc, accessMV),
2169-
CastConsumptionKind::BorrowAlways};
2170-
} else {
2171-
// Since we made a copy, send down TakeAlways.
2172-
eltCMV = {SGF.B.createLoadCopy(loc, accessMV),
2173-
CastConsumptionKind::TakeAlways};
2174-
}
2175-
access.endAccess(SGF);
2153+
boxedValue = SGF.B.createLoadBorrow(loc, boxedValue);
2154+
eltCMV = {boxedValue, CastConsumptionKind::BorrowAlways};
21762155
} else {
21772156
// The boxed value may be shared, so we always have to copy it.
2178-
eltCMV = getManagedSubobject(SGF, boxedValue, *eltTL,
2157+
eltCMV = getManagedSubobject(SGF, boxedValue.getValue(), *eltTL,
21792158
CastConsumptionKind::CopyOnSuccess);
21802159
}
21812160
}
21822161

21832162
// Reabstract to the substituted type, if needed.
2184-
21852163
CanType substEltTy =
21862164
sourceType->getTypeOfMember(SGF.SGM.M.getSwiftModule(), elt,
21872165
elt->getArgumentInterfaceType())

test/SILOptimizer/access_marker_verify.swift

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,31 @@ enum IntEnum {
385385
// CHECK: load [trivial] [[PROJ]] : $*Int
386386
// CHECK-LABEL: } // end sil function '$s20access_marker_verify7IntEnumO8getValueSivg'
387387

388+
// Also test SILGenPattern for address-only enums with indirect loadable payloads.
389+
enum IntTEnum<T> {
390+
indirect case int(Int)
391+
case other(T)
392+
393+
var getValue: Int {
394+
switch self {
395+
case .int(let x): return x
396+
case .other: return 0
397+
}
398+
}
399+
}
400+
// IntTEnum.getValue.getter
401+
// CHECK-LABEL: sil hidden [ossa] @$s20access_marker_verify8IntTEnumO8getValueSivg : $@convention(method) <T> (@in_guaranteed IntTEnum<T>) -> Int {
402+
// CHECK: bb0(%0 : $*IntTEnum<T>):
403+
// CHECK: switch_enum_addr %{{.*}} : $*IntTEnum<T>, case #IntTEnum.int!enumelt.1: bb1, case #IntTEnum.other!enumelt.1: bb2
404+
// CHECK: bb1:
405+
// CHECK: [[UTEDA:%.*]] = unchecked_take_enum_data_addr %{{.*}} : $*IntTEnum<T>, #IntTEnum.int!enumelt.1
406+
// CHECK-NOT: begin_access
407+
// CHECK: [[BOX:%.*]] = load [take] [[UTEDA]] : $*<τ_0_0> { var Int } <T>
408+
// CHECK: [[PROJ:%.*]] = project_box [[BOX]] : $<τ_0_0> { var Int } <T>, 0
409+
// CHECK-NOT: begin_access
410+
// CHECK: load [trivial] [[PROJ]] : $*Int
411+
// CHECK-LABEL: } // end sil function '$s20access_marker_verify8IntTEnumO8getValueSivg'
412+
388413
// -- indirect enum reference.
389414
enum RefEnum {
390415
indirect case ref(BaseClass)

0 commit comments

Comments
 (0)