From eadefc09a9ccae33dcc606376855eb25354bd668 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Mon, 9 Nov 2020 17:36:15 -0800 Subject: [PATCH] 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) --- include/swift/SIL/MemAccessUtils.h | 1 + lib/SIL/Utils/MemAccessUtils.cpp | 5 ++ test/SILOptimizer/access_marker_verify.swift | 5 +- test/SILOptimizer/accesspath_uses.sil | 51 ++++++++++++++++++++ test/SILOptimizer/load_borrow_verify.sil | 19 ++++++++ 5 files changed, 79 insertions(+), 2 deletions(-) diff --git a/include/swift/SIL/MemAccessUtils.h b/include/swift/SIL/MemAccessUtils.h index 2b01282935a2e..9439d50db3044 100644 --- a/include/swift/SIL/MemAccessUtils.h +++ b/include/swift/SIL/MemAccessUtils.h @@ -1239,6 +1239,7 @@ inline Operand *getAccessProjectionOperand(SingleValueInstruction *svi) { case SILInstructionKind::TupleElementAddrInst: case SILInstructionKind::IndexAddrInst: case SILInstructionKind::TailAddrInst: + case SILInstructionKind::InitEnumDataAddrInst: // open_existential_addr and unchecked_take_enum_data_addr are problematic // because they both modify memory and are access projections. Ideally, they // would not be casts, but will likely be eliminated with opaque values. diff --git a/lib/SIL/Utils/MemAccessUtils.cpp b/lib/SIL/Utils/MemAccessUtils.cpp index 1abab93e3d816..21859ef12e02b 100644 --- a/lib/SIL/Utils/MemAccessUtils.cpp +++ b/lib/SIL/Utils/MemAccessUtils.cpp @@ -927,6 +927,7 @@ class AccessPathVisitor : public FindAccessVisitorImpl { // Ignore everything in getAccessProjectionOperand that is an access // projection with no affect on the access path. assert(isa(projectedAddr) + || isa(projectedAddr) || isa(projectedAddr) || isa(projectedAddr)); } @@ -1390,6 +1391,10 @@ AccessPathDefUseTraversal::visitSingleValueUser(SingleValueInstruction *svi, return IgnoredUse; } + case SILInstructionKind::InitEnumDataAddrInst: + pushUsers(svi, dfs); + return IgnoredUse; + // open_existential_addr and unchecked_take_enum_data_addr are classified as // access projections, but they also modify memory. Both see through them and // also report them as uses. diff --git a/test/SILOptimizer/access_marker_verify.swift b/test/SILOptimizer/access_marker_verify.swift index 6062f4116579f..9b6e06027636d 100644 --- a/test/SILOptimizer/access_marker_verify.swift +++ b/test/SILOptimizer/access_marker_verify.swift @@ -333,8 +333,9 @@ func testInitGenericEnum(t: T) -> GenericEnum? { // CHECK: copy_addr %1 to [initialization] [[ADR1]] : $*T // CHECK: [[STK:%.*]] = alloc_stack $GenericEnum // CHECK: [[ENUMDATAADDR:%.*]] = init_enum_data_addr [[STK]] -// CHECK-NOT: begin_access -// CHECK: copy_addr [take] [[ADR1]] to [initialization] [[ENUMDATAADDR]] : $*T +// CHECK: [[ACCESSENUM:%.*]] = begin_access [modify] [unsafe] [[ENUMDATAADDR]] : $*T +// CHECK: copy_addr [take] [[ADR1]] to [initialization] [[ACCESSENUM]] : $*T +// CHECK: end_access [[ACCESSENUM]] : $*T // CHECK: inject_enum_addr // CHECK: [[ACCESS:%.*]] = begin_access [modify] [unknown] [[PROJ]] // CHECK: copy_addr [take] %{{.*}} to [[ACCESS]] : $*GenericEnum diff --git a/test/SILOptimizer/accesspath_uses.sil b/test/SILOptimizer/accesspath_uses.sil index 82e72c6438bd6..6557725bf8a32 100644 --- a/test/SILOptimizer/accesspath_uses.sil +++ b/test/SILOptimizer/accesspath_uses.sil @@ -866,3 +866,54 @@ bb5: %999 = tuple () return %999 : $() } + +// CHECK-LABEL: @test_init_enum_addr +// CHECK: ###For MemOp: store %0 to [init] %2 : $*AnyObject +// CHECK: Stack %1 = alloc_stack $Optional +// CHECK: Path: () +// CHECK: Exact Uses { +// CHECK-NEXT: store %0 to [init] %{{.*}} : $*AnyObject +// CHECK-NEXT: Path: () +// CHECK-NEXT: inject_enum_addr %1 : $*Optional, #Optional.some!enumelt +// CHECK-NEXT: Path: () +// CHECK-NEXT: %{{.*}} = load [copy] %1 : $*Optional +// CHECK-NEXT: Path: () +// CHECK-NEXT: dealloc_stack %1 : $*Optional +// CHECK-NEXT: Path: () +// CHECK-NEXT: } +// CHECK: ###For MemOp: inject_enum_addr %1 : $*Optional, #Optional.some!enumelt +// CHECK: Stack %1 = alloc_stack $Optional +// CHECK: Path: () +// CHECK: Exact Uses { +// CHECK-NEXT: store %0 to [init] %{{.*}} : $*AnyObject +// CHECK-NEXT: Path: () +// CHECK-NEXT: inject_enum_addr %1 : $*Optional, #Optional.some!enumelt +// CHECK-NEXT: Path: () +// CHECK-NEXT: %{{.*}} = load [copy] %1 : $*Optional +// CHECK-NEXT: Path: () +// CHECK-NEXT: dealloc_stack %1 : $*Optional +// CHECK-NEXT: Path: () +// CHECK-NEXT: } +// CHECK: ###For MemOp: %5 = load [copy] %1 : $*Optional +// CHECK: Stack %1 = alloc_stack $Optional +// CHECK: Path: () +// CHECK: Exact Uses { +// CHECK-NEXT: store %0 to [init] %{{.*}} : $*AnyObject +// CHECK-NEXT: Path: () +// CHECK-NEXT: inject_enum_addr %1 : $*Optional, #Optional.some!enumelt +// CHECK-NEXT: Path: () +// CHECK-NEXT: %{{.*}} = load [copy] %1 : $*Optional +// CHECK-NEXT: Path: () +// CHECK-NEXT: dealloc_stack %1 : $*Optional +// CHECK-NEXT: Path: () +// CHECK-NEXT: } +sil [ossa] @test_init_enum_addr : $@convention(thin) (@owned AnyObject) -> @owned Optional { +bb0(%0 : @owned $AnyObject): + %1 = alloc_stack $Optional + %2 = init_enum_data_addr %1 : $*Optional, #Optional.some!enumelt + store %0 to [init] %2 : $*AnyObject + inject_enum_addr %1 : $*Optional, #Optional.some!enumelt + %5 = load [copy] %1 : $*Optional + dealloc_stack %1 : $*Optional + return %5 : $Optional +} diff --git a/test/SILOptimizer/load_borrow_verify.sil b/test/SILOptimizer/load_borrow_verify.sil index 72620cd797833..143ea1b36341a 100644 --- a/test/SILOptimizer/load_borrow_verify.sil +++ b/test/SILOptimizer/load_borrow_verify.sil @@ -58,3 +58,22 @@ bb0(%0 : $Int64): end_access %33 : $*Int64 return %35 : $Int64 } + +// CHECK-LABEL: sil [ossa] @test_borrow_init_enum_addr : $@convention(thin) (@owned AnyObject) -> () { +// CHECK: bb0(%0 : @owned $AnyObject): +// CHECK: [[ALLOC:%.*]] = alloc_stack $Optional +// CHECK: init_enum_data_addr [[ALLOC]] : $*Optional, #Optional.some!enumelt +// CHECK: load_borrow [[ALLOC]] : $*Optional +// CHECK-LABEL: } // end sil function 'test_borrow_init_enum_addr' +sil [ossa] @test_borrow_init_enum_addr : $@convention(thin) (@owned AnyObject) -> () { +bb0(%0 : @owned $AnyObject): + %1 = alloc_stack $Optional + %2 = init_enum_data_addr %1 : $*Optional, #Optional.some!enumelt + store %0 to [init] %2 : $*AnyObject + inject_enum_addr %1 : $*Optional, #Optional.some!enumelt + %5 = load_borrow %1 : $*Optional + end_borrow %5 : $Optional + dealloc_stack %1 : $*Optional + %99 = tuple () + return %99 : $() +} \ No newline at end of file