Skip to content

Commit eadefc0

Browse files
committed
AccessPath: Add init_enum_data_addr to "access projections" set.
AccessPath was treating init_enum_data_addr as an address base, which is not ideal. It should be able to identify the underlying enum object as the base. This issue was caught by LoadBorrowImmutabilityChecker during SIL verification. Instead handle init_enum_data_addr as a access projection that does not affect the access path. I expect this SIL pattern to disappear with SIL opaque values, but it still needs to be handled properly after lowering addresses. Functionality changes: - any user of AccessPath now sees enum initialization stores as writes to the underlying enum object - SILGen now generates begin/end access markers for enum initialization patterns. (Originally, we did not "see through" init_enum_data_addr because we didn't want to generate these markers, but that behavior was inconsistent and problematic). Fixes rdar://70725514 fatal error encountered during compilation; Unknown instruction: init_enum_data_addr)
1 parent c2b13cd commit eadefc0

File tree

5 files changed

+79
-2
lines changed

5 files changed

+79
-2
lines changed

include/swift/SIL/MemAccessUtils.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1239,6 +1239,7 @@ inline Operand *getAccessProjectionOperand(SingleValueInstruction *svi) {
12391239
case SILInstructionKind::TupleElementAddrInst:
12401240
case SILInstructionKind::IndexAddrInst:
12411241
case SILInstructionKind::TailAddrInst:
1242+
case SILInstructionKind::InitEnumDataAddrInst:
12421243
// open_existential_addr and unchecked_take_enum_data_addr are problematic
12431244
// because they both modify memory and are access projections. Ideally, they
12441245
// would not be casts, but will likely be eliminated with opaque values.

lib/SIL/Utils/MemAccessUtils.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,7 @@ class AccessPathVisitor : public FindAccessVisitorImpl<AccessPathVisitor> {
927927
// Ignore everything in getAccessProjectionOperand that is an access
928928
// projection with no affect on the access path.
929929
assert(isa<OpenExistentialAddrInst>(projectedAddr)
930+
|| isa<InitEnumDataAddrInst>(projectedAddr)
930931
|| isa<UncheckedTakeEnumDataAddrInst>(projectedAddr)
931932
|| isa<ProjectBoxInst>(projectedAddr));
932933
}
@@ -1390,6 +1391,10 @@ AccessPathDefUseTraversal::visitSingleValueUser(SingleValueInstruction *svi,
13901391
return IgnoredUse;
13911392
}
13921393

1394+
case SILInstructionKind::InitEnumDataAddrInst:
1395+
pushUsers(svi, dfs);
1396+
return IgnoredUse;
1397+
13931398
// open_existential_addr and unchecked_take_enum_data_addr are classified as
13941399
// access projections, but they also modify memory. Both see through them and
13951400
// also report them as uses.

test/SILOptimizer/access_marker_verify.swift

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -333,8 +333,9 @@ func testInitGenericEnum<T>(t: T) -> GenericEnum<T>? {
333333
// CHECK: copy_addr %1 to [initialization] [[ADR1]] : $*T
334334
// CHECK: [[STK:%.*]] = alloc_stack $GenericEnum<T>
335335
// CHECK: [[ENUMDATAADDR:%.*]] = init_enum_data_addr [[STK]]
336-
// CHECK-NOT: begin_access
337-
// CHECK: copy_addr [take] [[ADR1]] to [initialization] [[ENUMDATAADDR]] : $*T
336+
// CHECK: [[ACCESSENUM:%.*]] = begin_access [modify] [unsafe] [[ENUMDATAADDR]] : $*T
337+
// CHECK: copy_addr [take] [[ADR1]] to [initialization] [[ACCESSENUM]] : $*T
338+
// CHECK: end_access [[ACCESSENUM]] : $*T
338339
// CHECK: inject_enum_addr
339340
// CHECK: [[ACCESS:%.*]] = begin_access [modify] [unknown] [[PROJ]]
340341
// CHECK: copy_addr [take] %{{.*}} to [[ACCESS]] : $*GenericEnum<T>

test/SILOptimizer/accesspath_uses.sil

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -866,3 +866,54 @@ bb5:
866866
%999 = tuple ()
867867
return %999 : $()
868868
}
869+
870+
// CHECK-LABEL: @test_init_enum_addr
871+
// CHECK: ###For MemOp: store %0 to [init] %2 : $*AnyObject
872+
// CHECK: Stack %1 = alloc_stack $Optional<AnyObject>
873+
// CHECK: Path: ()
874+
// CHECK: Exact Uses {
875+
// CHECK-NEXT: store %0 to [init] %{{.*}} : $*AnyObject
876+
// CHECK-NEXT: Path: ()
877+
// CHECK-NEXT: inject_enum_addr %1 : $*Optional<AnyObject>, #Optional.some!enumelt
878+
// CHECK-NEXT: Path: ()
879+
// CHECK-NEXT: %{{.*}} = load [copy] %1 : $*Optional<AnyObject>
880+
// CHECK-NEXT: Path: ()
881+
// CHECK-NEXT: dealloc_stack %1 : $*Optional<AnyObject>
882+
// CHECK-NEXT: Path: ()
883+
// CHECK-NEXT: }
884+
// CHECK: ###For MemOp: inject_enum_addr %1 : $*Optional<AnyObject>, #Optional.some!enumelt
885+
// CHECK: Stack %1 = alloc_stack $Optional<AnyObject>
886+
// CHECK: Path: ()
887+
// CHECK: Exact Uses {
888+
// CHECK-NEXT: store %0 to [init] %{{.*}} : $*AnyObject
889+
// CHECK-NEXT: Path: ()
890+
// CHECK-NEXT: inject_enum_addr %1 : $*Optional<AnyObject>, #Optional.some!enumelt
891+
// CHECK-NEXT: Path: ()
892+
// CHECK-NEXT: %{{.*}} = load [copy] %1 : $*Optional<AnyObject>
893+
// CHECK-NEXT: Path: ()
894+
// CHECK-NEXT: dealloc_stack %1 : $*Optional<AnyObject>
895+
// CHECK-NEXT: Path: ()
896+
// CHECK-NEXT: }
897+
// CHECK: ###For MemOp: %5 = load [copy] %1 : $*Optional<AnyObject>
898+
// CHECK: Stack %1 = alloc_stack $Optional<AnyObject>
899+
// CHECK: Path: ()
900+
// CHECK: Exact Uses {
901+
// CHECK-NEXT: store %0 to [init] %{{.*}} : $*AnyObject
902+
// CHECK-NEXT: Path: ()
903+
// CHECK-NEXT: inject_enum_addr %1 : $*Optional<AnyObject>, #Optional.some!enumelt
904+
// CHECK-NEXT: Path: ()
905+
// CHECK-NEXT: %{{.*}} = load [copy] %1 : $*Optional<AnyObject>
906+
// CHECK-NEXT: Path: ()
907+
// CHECK-NEXT: dealloc_stack %1 : $*Optional<AnyObject>
908+
// CHECK-NEXT: Path: ()
909+
// CHECK-NEXT: }
910+
sil [ossa] @test_init_enum_addr : $@convention(thin) (@owned AnyObject) -> @owned Optional<AnyObject> {
911+
bb0(%0 : @owned $AnyObject):
912+
%1 = alloc_stack $Optional<AnyObject>
913+
%2 = init_enum_data_addr %1 : $*Optional<AnyObject>, #Optional.some!enumelt
914+
store %0 to [init] %2 : $*AnyObject
915+
inject_enum_addr %1 : $*Optional<AnyObject>, #Optional.some!enumelt
916+
%5 = load [copy] %1 : $*Optional<AnyObject>
917+
dealloc_stack %1 : $*Optional<AnyObject>
918+
return %5 : $Optional<AnyObject>
919+
}

test/SILOptimizer/load_borrow_verify.sil

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,3 +58,22 @@ bb0(%0 : $Int64):
5858
end_access %33 : $*Int64
5959
return %35 : $Int64
6060
}
61+
62+
// CHECK-LABEL: sil [ossa] @test_borrow_init_enum_addr : $@convention(thin) (@owned AnyObject) -> () {
63+
// CHECK: bb0(%0 : @owned $AnyObject):
64+
// CHECK: [[ALLOC:%.*]] = alloc_stack $Optional<AnyObject>
65+
// CHECK: init_enum_data_addr [[ALLOC]] : $*Optional<AnyObject>, #Optional.some!enumelt
66+
// CHECK: load_borrow [[ALLOC]] : $*Optional<AnyObject>
67+
// CHECK-LABEL: } // end sil function 'test_borrow_init_enum_addr'
68+
sil [ossa] @test_borrow_init_enum_addr : $@convention(thin) (@owned AnyObject) -> () {
69+
bb0(%0 : @owned $AnyObject):
70+
%1 = alloc_stack $Optional<AnyObject>
71+
%2 = init_enum_data_addr %1 : $*Optional<AnyObject>, #Optional.some!enumelt
72+
store %0 to [init] %2 : $*AnyObject
73+
inject_enum_addr %1 : $*Optional<AnyObject>, #Optional.some!enumelt
74+
%5 = load_borrow %1 : $*Optional<AnyObject>
75+
end_borrow %5 : $Optional<AnyObject>
76+
dealloc_stack %1 : $*Optional<AnyObject>
77+
%99 = tuple ()
78+
return %99 : $()
79+
}

0 commit comments

Comments
 (0)