From 599c8212441a50246fcb784441224c1357df5573 Mon Sep 17 00:00:00 2001 From: zoecarver Date: Wed, 29 Apr 2020 14:11:29 -0700 Subject: [PATCH 1/2] [opt] Update alloc_refs on the stack to have stack memory kind. Update MemAccessUtils to clasify class allocs on the stack as "AccessedStorage::Kind::Stack" instead of "::Class". This allows class element access markers to be promoted to have static access enforcement. --- include/swift/SIL/MemAccessUtils.h | 4 +- lib/SIL/Utils/MemAccessUtils.cpp | 20 ++- lib/SILOptimizer/PassManager/PassPipeline.cpp | 4 + test/SILOptimizer/access_enforcement_opts.sil | 133 ++++++++++++++++++ 4 files changed, 157 insertions(+), 4 deletions(-) diff --git a/include/swift/SIL/MemAccessUtils.h b/include/swift/SIL/MemAccessUtils.h index cb4c918ab1384..cf6837ea90b52 100644 --- a/include/swift/SIL/MemAccessUtils.h +++ b/include/swift/SIL/MemAccessUtils.h @@ -582,9 +582,7 @@ class AccessUseDefChainVisitor { // Visitors for specific identified access kinds. These default to calling out // to visitIdentified. - Result visitClassAccess(RefElementAddrInst *field) { - return asImpl().visitBase(field, AccessedStorage::Class); - } + Result visitClassAccess(RefElementAddrInst *field); Result visitArgumentAccess(SILFunctionArgument *arg) { return asImpl().visitBase(arg, AccessedStorage::Argument); } diff --git a/lib/SIL/Utils/MemAccessUtils.cpp b/lib/SIL/Utils/MemAccessUtils.cpp index 2841239bf0e0c..06668973b7be8 100644 --- a/lib/SIL/Utils/MemAccessUtils.cpp +++ b/lib/SIL/Utils/MemAccessUtils.cpp @@ -100,7 +100,6 @@ AccessedStorage::AccessedStorage(SILValue base, Kind kind) { value = base; break; case Stack: - assert(isa(base)); value = base; break; case Nested: @@ -772,3 +771,22 @@ void swift::visitAccessedAddress(SILInstruction *I, return; } } + +template +Result swift::AccessUseDefChainVisitor::visitClassAccess( + RefElementAddrInst *field) { + auto operand = field->getOperand(); + while (isa(operand) || + isa(operand) || + isa(operand) || isa(operand)) + operand = operand.getDefiningInstruction()->getOperand(0); + + if (isa(operand)) + return asImpl().visitBase(field, AccessedStorage::Stack); + if (auto allocRef = dyn_cast(operand)) { + if (allocRef->isAllocatingStack()) + return asImpl().visitBase(field, AccessedStorage::Stack); + } + + return asImpl().visitBase(field, AccessedStorage::Class); +} diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp index 3f586c5cc2734..c74b683863444 100644 --- a/lib/SILOptimizer/PassManager/PassPipeline.cpp +++ b/lib/SILOptimizer/PassManager/PassPipeline.cpp @@ -588,6 +588,10 @@ static void addClosureSpecializePassPipeline(SILPassPipelinePlan &P) { static void addLowLevelPassPipeline(SILPassPipelinePlan &P) { P.startPipeline("LowLevel,Function", true /*isFunctionPassPipeline*/); + P.addAccessEnforcementOpts(); + P.addAccessEnforcementWMO(); + P.addAccessMarkerElimination(); + // Should be after FunctionSignatureOpts and before the last inliner. P.addReleaseDevirtualizer(); diff --git a/test/SILOptimizer/access_enforcement_opts.sil b/test/SILOptimizer/access_enforcement_opts.sil index 5f644b6a3411d..cae299834c5c2 100644 --- a/test/SILOptimizer/access_enforcement_opts.sil +++ b/test/SILOptimizer/access_enforcement_opts.sil @@ -1737,3 +1737,136 @@ bb0(%0 : ${ var Int64 }): end_access %access : $*Int64 return %val : $Int64 } + +// CHECK-LABEL: sil @promote_local_class_to_static +// CHECK: begin_access [modify] [static] [no_nested_conflict] +// CHECK: begin_access [read] [static] [no_nested_conflict] +// CHECK-LABEL: end sil function 'promote_local_class_to_static' +sil @promote_local_class_to_static : $@convention(thin) () -> () { +bb0: + %0 = integer_literal $Builtin.Int32, 0 + %1 = struct $Int32 (%0 : $Builtin.Int32) + %4 = alloc_ref [stack] $RefElemClass + + %8 = ref_element_addr %4 : $RefElemClass, #RefElemClass.y + %9 = begin_access [modify] [dynamic] [no_nested_conflict] %8 : $*Int32 + store %1 to %9 : $*Int32 + end_access %9 : $*Int32 + + %17 = begin_access [read] [dynamic] [no_nested_conflict] %8 : $*Int32 + %18 = struct_element_addr %17 : $*Int32, #Int32._value + %19 = load %18 : $*Builtin.Int32 + end_access %17 : $*Int32 + + set_deallocating %4 : $RefElemClass + dealloc_ref %4 : $RefElemClass + dealloc_ref [stack] %4 : $RefElemClass + + %9999 = tuple () + return %9999 : $() +} + +class Class1 { + @_hasStorage @_hasInitialValue var x: Int32 { get set } + deinit + init() +} + +struct Struct1 { + @_hasStorage @_hasInitialValue var c: Class1 { get set } + init(c: Class1 = Class1()) + init() +} + +class Class2 { + @_hasStorage @_hasInitialValue var s: Struct1 { get set } + deinit + init() +} + +// CHECK-LABEL: sil @promote_local_class_in_struct_in_class_to_static +// CHECK: begin_access [modify] [static] [no_nested_conflict] +// CHECK: begin_access [read] [static] [no_nested_conflict] +// CHECK-LABEL: end sil function 'promote_local_class_in_struct_in_class_to_static' +sil @promote_local_class_in_struct_in_class_to_static : $@convention(thin) () -> () { +bb0: + %0 = integer_literal $Builtin.Int32, 0 + %1 = struct $Int32 (%0 : $Builtin.Int32) + %4 = alloc_ref [stack] $Class2 + + %5 = ref_element_addr %4 : $Class2, #Class2.s + %6 = struct_element_addr %5 : $*Struct1, #Struct1.c + %7 = load %6 : $*Class1 + %8 = ref_element_addr %7 : $Class1, #Class1.x + %9 = begin_access [modify] [dynamic] [no_nested_conflict] %8 : $*Int32 + store %1 to %9 : $*Int32 + end_access %9 : $*Int32 + + %17 = begin_access [read] [dynamic] [no_nested_conflict] %8 : $*Int32 + %18 = struct_element_addr %17 : $*Int32, #Int32._value + %19 = load %18 : $*Builtin.Int32 + end_access %17 : $*Int32 + + set_deallocating %4 : $Class2 + dealloc_ref %4 : $Class2 + dealloc_ref [stack] %4 : $Class2 + + %9999 = tuple () + return %9999 : $() +} + +// CHECK-LABEL: sil @promote_local_class_in_struct_to_static +// CHECK: begin_access [modify] [static] [no_nested_conflict] +// CHECK: begin_access [read] [static] [no_nested_conflict] +// CHECK-LABEL: end sil function 'promote_local_class_in_struct_to_static' +sil @promote_local_class_in_struct_to_static : $@convention(thin) () -> () { +bb0: + %0 = integer_literal $Builtin.Int32, 0 + %1 = struct $Int32 (%0 : $Builtin.Int32) + %4 = alloc_stack $Struct1 + + %6 = struct_element_addr %4 : $*Struct1, #Struct1.c + %7 = load %6 : $*Class1 + %8 = ref_element_addr %7 : $Class1, #Class1.x + %9 = begin_access [modify] [dynamic] [no_nested_conflict] %8 : $*Int32 + store %1 to %9 : $*Int32 + end_access %9 : $*Int32 + + %17 = begin_access [read] [dynamic] [no_nested_conflict] %8 : $*Int32 + %18 = struct_element_addr %17 : $*Int32, #Int32._value + %19 = load %18 : $*Builtin.Int32 + end_access %17 : $*Int32 + + dealloc_stack %4 : $*Struct1 + + %9999 = tuple () + return %9999 : $() +} + +// CHECK-LABEL: sil @promote_local_class_in_tuple_to_static +// CHECK: begin_access [modify] [static] [no_nested_conflict] +// CHECK: begin_access [read] [static] [no_nested_conflict] +// CHECK-LABEL: end sil function 'promote_local_class_in_tuple_to_static' +sil @promote_local_class_in_tuple_to_static : $@convention(thin) () -> () { +bb0: + %0 = integer_literal $Builtin.Int32, 0 + %1 = struct $Int32 (%0 : $Builtin.Int32) + %4 = alloc_stack $(Class1, Int) + + %6 = tuple_element_addr %4 : $*(Class1, Int), 0 + %7 = load %6 : $*Class1 + %8 = ref_element_addr %7 : $Class1, #Class1.x + %9 = begin_access [modify] [dynamic] [no_nested_conflict] %8 : $*Int32 + store %1 to %9 : $*Int32 + end_access %9 : $*Int32 + + %17 = begin_access [read] [dynamic] [no_nested_conflict] %8 : $*Int32 + %18 = struct_element_addr %17 : $*Int32, #Int32._value + %19 = load %18 : $*Builtin.Int32 + end_access %17 : $*Int32 + + dealloc_stack %4 : $*(Class1, Int) + + %9999 = tuple () + return %9999 : $() +} From da23b3509097a71f9a32cc6c58537d5219dfc53d Mon Sep 17 00:00:00 2001 From: zoecarver Date: Tue, 14 Jul 2020 20:53:17 -0700 Subject: [PATCH 2/2] Remove AccessMarkerElimination from LowLevel,Function pipeline --- lib/SILOptimizer/PassManager/PassPipeline.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp index c74b683863444..326c4229535b8 100644 --- a/lib/SILOptimizer/PassManager/PassPipeline.cpp +++ b/lib/SILOptimizer/PassManager/PassPipeline.cpp @@ -590,7 +590,7 @@ static void addLowLevelPassPipeline(SILPassPipelinePlan &P) { P.addAccessEnforcementOpts(); P.addAccessEnforcementWMO(); - P.addAccessMarkerElimination(); + // P.addAccessMarkerElimination(); // Should be after FunctionSignatureOpts and before the last inliner. P.addReleaseDevirtualizer();