Skip to content

Two optimization improvements related to access instructions #66592

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2312,6 +2312,19 @@ void LifetimeChecker::handleSelfInitUse(unsigned UseID) {
}
}

// In case of `var` initializations, SILGen creates a dynamic begin/end_access
// pair around the initialization store. If it's an initialization (and not
// a re-assign) it's guaranteed that it's an exclusive access and we can
// convert the access to an `[init] [static]` access.
static void setStaticInitAccess(SILValue memoryAddress) {
if (auto *ba = dyn_cast<BeginAccessInst>(memoryAddress)) {
if (ba->getEnforcement() == SILAccessEnforcement::Dynamic) {
ba->setEnforcement(SILAccessEnforcement::Static);
if (ba->getAccessKind() == SILAccessKind::Modify)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eeckstein I think you might be able to take this even a step further and change SILGen to always emit these as inits, unknown. It would then be an error if access control Can not convert it to static. I did something like this for consuming parameters which use deinit. Anyways, just a friendly thought.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about this, too. But SILGen doesn't know yet if it's an init or re-assign. And e.g. in a class initializer you can let self escape (after all members are initialized) and then re-assign a member. I'm not sure if it's even safe to guard such a reassign-after-self-escape as a statically checked access.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/guanranteed/guaranteed

ba->setAccessKind(SILAccessKind::Init);
}
}
}

/// updateInstructionForInitState - When an instruction being analyzed moves
/// from being InitOrAssign to some concrete state, update it for that state.
Expand All @@ -2336,6 +2349,8 @@ void LifetimeChecker::updateInstructionForInitState(unsigned UseID) {
assert(!CA->isInitializationOfDest() &&
"should not modify copy_addr that already knows it is initialized");
CA->setIsInitializationOfDest(InitKind);
if (InitKind == IsInitialization)
setStaticInitAccess(CA->getDest());
return;
}

Expand Down Expand Up @@ -2382,6 +2397,7 @@ void LifetimeChecker::updateInstructionForInitState(unsigned UseID) {
MarkMustCheckInst::CheckKind::InitableButNotConsumable);
}
}
setStaticInitAccess(AI->getDest());
}

return;
Expand Down
3 changes: 3 additions & 0 deletions lib/SILOptimizer/Transforms/DeadObjectElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,9 @@ static bool canZapInstruction(SILInstruction *Inst, bool acceptRefCountInsts,
if (isa<InitExistentialAddrInst>(Inst))
return true;

if (isa<BeginAccessInst>(Inst) || isa<EndAccessInst>(Inst))
return true;

// If Inst does not read or write to memory, have side effects, and is not a
// terminator, we can zap it.
if (!Inst->mayHaveSideEffects() && !Inst->mayReadFromMemory() &&
Expand Down
2 changes: 1 addition & 1 deletion test/Concurrency/default_actor_definit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ actor C {
// CHECK-LABEL: sil hidden @$s21default_actor_definit1CC1yACSgSi_tcfc
// CHECK: builtin "initializeDefaultActor"(%1 : $C)
// CHECK: [[X:%.*]] = ref_element_addr %1 : $C, #C.x
// CHECK-NEXT: [[ACCESS:%.*]] = begin_access [modify] [dynamic] [[X]] : $*String
// CHECK-NEXT: [[ACCESS:%.*]] = begin_access [init] [static] [[X]] : $*String
// CHECK-NEXT: store {{.*}} to [[ACCESS]] : $*String
// CHECK-NEXT: end_access [[ACCESS]] : $*String
// CHECK: cond_br {{%.*}}, bb1, bb2
Expand Down
4 changes: 3 additions & 1 deletion test/SILOptimizer/dead_alloc_elim.sil
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ sil @store_to_trivial_property : $@convention(thin) (Int) -> () {
bb0(%0 : $Int):
%20 = alloc_ref [stack] $NontrivialDestructor
%21 = ref_element_addr %20 : $NontrivialDestructor, #NontrivialDestructor.i
store %0 to %21 : $*Int
%22 = begin_access [modify] [dynamic] %21 : $*Int
store %0 to %22 : $*Int
end_access %22 : $*Int
set_deallocating %20 : $NontrivialDestructor
dealloc_ref %20 : $NontrivialDestructor
dealloc_stack_ref %20 : $NontrivialDestructor
Expand Down
5 changes: 2 additions & 3 deletions test/SILOptimizer/default-cmo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,8 @@ public func getSubmoduleKlassMember() -> Int {
}

// CHECK-LABEL: sil @$s4Main26getSubmoduleKlassMemberTBDSiyF
// CHECK: [[F:%[0-9]+]] = function_ref @$s9ModuleTBD20submoduleKlassMemberSiyF
// CHECK: [[I:%[0-9]+]] = apply [[F]]
// CHECK: return [[I]]
// CHECK-NOT: function_ref
// CHECK-NOT: apply
// CHECK: } // end sil function '$s4Main26getSubmoduleKlassMemberTBDSiyF'
public func getSubmoduleKlassMemberTBD() -> Int {
return ModuleTBD.submoduleKlassMember()
Expand Down
4 changes: 2 additions & 2 deletions test/SILOptimizer/definite_init_actor.swift
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ actor BoringActor {
// CHECK-LABEL: sil hidden @$s4test14SingleVarActorC10iterationsACSi_tYacfc : $@convention(method) @async (Int, @owned SingleVarActor) -> @owned SingleVarActor {
// CHECK: bb0({{%[0-9]+}} : $Int, [[SELF:%[0-9]+]] : $SingleVarActor):
// CHECK: [[MYVAR_REF:%[0-9]+]] = ref_element_addr [[SELF]] : $SingleVarActor, #SingleVarActor.myVar
// CHECK: [[MYVAR:%[0-9]+]] = begin_access [modify] [dynamic] [[MYVAR_REF]] : $*Int
// CHECK: [[MYVAR:%[0-9]+]] = begin_access [init] [static] [[MYVAR_REF]] : $*Int
// CHECK: store {{%[0-9]+}} to [[MYVAR]] : $*Int
// CHECK-NEXT: end_access [[MYVAR]]
// CHECK-NEXT: hop_to_executor [[SELF]] : $SingleVarActor
Expand Down Expand Up @@ -196,7 +196,7 @@ actor MultiVarActor {
// CHECK-LABEL: sil hidden @$s4test13MultiVarActorC10doNotThrowACSb_tYaKcfc : $@convention(method) @async (Bool, @owned MultiVarActor) -> (@owned MultiVarActor, @error any Error) {
// CHECK: bb0({{%[0-9]+}} : $Bool, [[SELF:%[0-9]+]] : $MultiVarActor):
// CHECK: [[REF:%[0-9]+]] = ref_element_addr [[SELF]] : $MultiVarActor, #MultiVarActor.firstVar
// CHECK: [[VAR:%[0-9]+]] = begin_access [modify] [dynamic] [[REF]] : $*Int
// CHECK: [[VAR:%[0-9]+]] = begin_access [init] [static] [[REF]] : $*Int
// CHECK: store {{%[0-9]+}} to [[VAR]] : $*Int
// CHECK-NEXT: end_access [[VAR]]
// CHECK-NEXT: hop_to_executor %1 : $MultiVarActor
Expand Down
4 changes: 2 additions & 2 deletions test/SILOptimizer/definite_init_failable_initializers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -735,7 +735,7 @@ class FailableBaseClass {
// CHECK: bb0(%0 : $FailableBaseClass):
// CHECK: [[CANARY:%.*]] = apply
// CHECK-NEXT: [[MEMBER_ADDR:%.*]] = ref_element_addr %0
// CHECK-NEXT: [[WRITE:%.*]] = begin_access [modify] [dynamic] [[MEMBER_ADDR]] : $*Canary
// CHECK-NEXT: [[WRITE:%.*]] = begin_access [init] [static] [[MEMBER_ADDR]] : $*Canary
// CHECK-NEXT: store [[CANARY]] to [[WRITE]]
// CHECK-NEXT: end_access [[WRITE]] : $*Canary
// CHECK-NEXT: strong_release %0
Expand Down Expand Up @@ -849,7 +849,7 @@ class FailableDerivedClass : FailableBaseClass {
// CHECK: [[CANARY_FUN:%.*]] = function_ref @$s35definite_init_failable_initializers6CanaryCACycfC :
// CHECK: [[CANARY:%.*]] = apply [[CANARY_FUN]](
// CHECK-NEXT: [[MEMBER_ADDR:%.*]] = ref_element_addr [[SELF]]
// CHECK-NEXT: [[WRITE:%.*]] = begin_access [modify] [dynamic] [[MEMBER_ADDR]] : $*Canary
// CHECK-NEXT: [[WRITE:%.*]] = begin_access [init] [static] [[MEMBER_ADDR]] : $*Canary
// CHECK-NEXT: store [[CANARY]] to [[WRITE]]
// CHECK-NEXT: end_access [[WRITE]] : $*Canary
// CHECK-NEXT: strong_release [[SELF]]
Expand Down
53 changes: 53 additions & 0 deletions test/SILOptimizer/definite_init_markuninitialized_rootself.sil
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ class RootClassWithNontrivialStoredProperties {
}

class SomeClass {}

final class IntClass {
var i: Int
init(_ i: Int)
}

final class GenericClass<T> {
var t: T
init(_ t: T)
}

sil @getSomeClass : $@convention(thin) () -> @owned SomeClass
sil @getSomeOptionalClass : $@convention(thin) () -> Optional<SomeClass>

Expand Down Expand Up @@ -131,3 +142,45 @@ bb0(%0 : @owned $RootClassWithNontrivialStoredProperties):
%13 = tuple ()
return %13 : $()
}

// CHECK-LABEL: sil [ossa] @init_IntClass :
// CHECK: [[ACCESS:%.*]] = begin_access [init] [static]
// CHECK: store %0 to [trivial] [[ACCESS]]
// CHECK: [[ACCESS2:%.*]] = begin_access [modify] [dynamic]
// CHECK: store %0 to [trivial] [[ACCESS2]]
// CHECK: } // end sil function 'init_IntClass'
sil [ossa] @init_IntClass : $@convention(method) (Int, @owned IntClass) -> @owned IntClass {
bb0(%0 : $Int, %1 : @owned $IntClass):
%4 = mark_uninitialized [rootself] %1 : $IntClass
%5 = begin_borrow %4 : $IntClass
%6 = ref_element_addr %5 : $IntClass, #IntClass.i
%7 = begin_access [modify] [dynamic] %6 : $*Int
assign %0 to %7 : $*Int
end_access %7 : $*Int
%10 = begin_access [modify] [dynamic] %6 : $*Int
assign %0 to %10 : $*Int
end_access %10 : $*Int
end_borrow %5 : $IntClass
return %4 : $IntClass
}

// CHECK-LABEL: sil [ossa] @init_GenericClass :
// CHECK: [[ACCESS:%.*]] = begin_access [init] [static]
// CHECK: copy_addr %0 to [init] [[ACCESS]]
// CHECK: [[ACCESS2:%.*]] = begin_access [modify] [dynamic]
// CHECK: copy_addr [take] %0 to [[ACCESS2]]
// CHECK: } // end sil function 'init_GenericClass'
sil [ossa] @init_GenericClass : $@convention(method) <T> (@in T, @owned GenericClass<T>) -> @owned GenericClass<T> {
bb0(%0 : $*T, %1 : @owned $GenericClass<T>):
%4 = mark_uninitialized [rootself] %1 : $GenericClass<T>
%5 = begin_borrow %4 : $GenericClass<T>
%8 = ref_element_addr %5 : $GenericClass<T>, #GenericClass.t
%9 = begin_access [modify] [dynamic] %8 : $*T
copy_addr %0 to %9 : $*T
end_access %9 : $*T
%12 = begin_access [modify] [dynamic] %8 : $*T
copy_addr [take] %0 to %12 : $*T
end_access %12 : $*T
end_borrow %5 : $GenericClass<T>
return %4 : $GenericClass<T>
}
6 changes: 3 additions & 3 deletions test/SILOptimizer/definite_init_root_class.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class FirstClass {
// CHECK: [[INIT:%.*]] = function_ref @$s24definite_init_root_class10OtherClassCACycfC : $@convention(method) (@thick OtherClass.Type) -> @owned OtherClass
// CHECK: [[OTHER:%.*]] = apply [[INIT]]([[METATYPE]]) : $@convention(method) (@thick OtherClass.Type) -> @owned OtherClass
// CHECK: [[X_ADDR:%.*]] = ref_element_addr %1 : $FirstClass, #FirstClass.x
// CHECK: [[X_ACCESS:%.*]] = begin_access [modify] [dynamic] %15 : $*OtherClass
// CHECK: [[X_ACCESS:%.*]] = begin_access [init] [static] %15 : $*OtherClass
// CHECK: [[ONE:%.*]] = integer_literal $Builtin.Int1, -1
// CHECK: store [[ONE]] to [[CONTROL]] : $*Builtin.Int1
// CHECK: store [[OTHER]] to [[X_ACCESS]] : $*OtherClass
Expand Down Expand Up @@ -115,7 +115,7 @@ class SecondClass {
// CHECK: [[INIT:%.*]] = function_ref @$s24definite_init_root_class10OtherClassCACycfC : $@convention(method) (@thick OtherClass.Type) -> @owned OtherClass
// CHECK: [[OTHER:%.*]] = apply [[INIT]]([[METATYPE]]) : $@convention(method) (@thick OtherClass.Type) -> @owned OtherClass
// CHECK: [[X_ADDR:%.*]] = ref_element_addr %1 : $SecondClass, #SecondClass.x
// CHECK: [[X_ACCESS:%.*]] = begin_access [modify] [dynamic] [[X_ADDR]] : $*OtherClass
// CHECK: [[X_ACCESS:%.*]] = begin_access [init] [static] [[X_ADDR]] : $*OtherClass
// CHECK: [[ONE:%.*]] = integer_literal $Builtin.Int2, 1
// CHECK: store [[ONE]] to [[CONTROL]] : $*Builtin.Int2
// CHECK: store [[OTHER]] to [[X_ACCESS]] : $*OtherClass
Expand All @@ -138,7 +138,7 @@ class SecondClass {
// CHECK: [[INIT:%.*]] = function_ref @$s24definite_init_root_class10OtherClassCACycfC : $@convention(method) (@thick OtherClass.Type) -> @owned OtherClass
// CHECK: [[OTHER:%.*]] = apply [[INIT]]([[METATYPE]]) : $@convention(method) (@thick OtherClass.Type) -> @owned OtherClass
// CHECK: [[Y_ADDR:%.*]] = ref_element_addr %1 : $SecondClass, #SecondClass.y
// CHECK: [[Y_ACCESS:%.*]] = begin_access [modify] [dynamic] [[Y_ADDR]] : $*OtherClass
// CHECK: [[Y_ACCESS:%.*]] = begin_access [init] [static] [[Y_ADDR]] : $*OtherClass
// CHECK: [[THREE:%.*]] = integer_literal $Builtin.Int2, -1
// CHECK: store [[THREE]] to [[CONTROL]] : $*Builtin.Int2
// CHECK: store [[OTHER]] to [[Y_ACCESS]] : $*OtherClass
Expand Down
12 changes: 6 additions & 6 deletions test/SILOptimizer/devirt_speculative_init.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,17 @@ public class BigCat : Cat {
}
}

public func make(type: Cat.Type, cats: Int) {
type.init(cats: cats)
public func make(type: Cat.Type, cats: Int) -> Cat {
return type.init(cats: cats)
}

// CHECK-LABEL: sil @$s23devirt_speculative_init4make4type4catsyAA3CatCm_SitF : $@convention(thin) (@thick Cat.Type, Int) -> ()
// CHECK-LABEL: sil @$s23devirt_speculative_init4make4type4catsAA3CatCAFm_SitF : $@convention(thin) (@thick Cat.Type, Int) -> @owned Cat {
// CHECK: checked_cast_br [exact] %0 : $@thick Cat.Type to @thick Cat.Type, bb2, bb3
// CHECK: bb1:
// CHECK: bb1{{.*}}:
// CHECK: return
// CHECK: bb2({{%.*}} : $@thick Cat.Type):
// CHECK: alloc_ref [stack] $Cat
// CHECK: alloc_ref $Cat
// CHECK: br bb1
// CHECK: bb3:
// CHECK: alloc_ref [stack] $BigCat
// CHECK: alloc_ref $BigCat
// CHECK: br bb1
3 changes: 1 addition & 2 deletions test/SILOptimizer/merge_exclusivity.swift
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,7 @@ private struct EscapedTransform<T>: WriteProt {

// TESTSIL-LABEL: sil [noinline] @$s17merge_exclusivity14run_MergeTest9yySiF : $@convention(thin)
// TESTSIL: [[REFADDR:%.*]] = ref_element_addr {{.*}} : $StreamClass, #StreamClass.buffer
// TESTSIL-NEXT: [[B1:%.*]] = begin_access [modify] [{{.*}}] [no_nested_conflict] [[REFADDR]]
// TESTSIL: end_access [[B1]]
// TESTSIL-NEXT: store {{.*}} to [[REFADDR]]
// TESTSIL: [[BCONF:%.*]] = begin_access [modify] [{{.*}}] [[REFADDR]]
// TESTSIL: end_access [[BCONF]]
// TESTSIL: [[BCONF:%.*]] = begin_access [modify] [{{.*}}] [[REFADDR]]
Expand Down
2 changes: 1 addition & 1 deletion test/sil-func-extractor/basic.swift
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
// EXTRACT-INIT-LABEL: sil @$s5basic7VehicleC1nACSi_tcfc : $@convention(method) (Int, @owned Vehicle) -> @owned Vehicle {
// EXTRACT-INIT: bb0
// EXTRACT-INIT-NEXT: ref_element_addr
// EXTRACT-INIT-NEXT: begin_access [modify] [dynamic]
// EXTRACT-INIT-NEXT: begin_access [init] [static]
// EXTRACT-INIT-NEXT: store
// EXTRACT-INIT-NEXT: end_access
// EXTRACT-INIT-NEXT: return
Expand Down