Skip to content

[SILGen] Forward address-only self to borrowing accessors. #74235

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 1 commit into from
Jun 10, 2024
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
17 changes: 8 additions & 9 deletions lib/SILGen/SILGenApply.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6858,22 +6858,21 @@ bool AccessorBaseArgPreparer::shouldLoadBaseAddress() const {
ArgumentSource AccessorBaseArgPreparer::prepareAccessorAddressBaseArg() {
// If the base is currently an address, we may have to copy it.
if (shouldLoadBaseAddress()) {
if (selfParam.isConsumed() ||
(base.getType().isAddressOnly(SGF.F)
// If a move-only base is borrowed, then we have to try our best to
// borrow it in-place without copying.
// TODO: Can we avoid copying a non-move-only value too in this
// circumstance?
&& !base.getType().isMoveOnly())) {
if (selfParam.isConsumed() || base.getType().isAddressOnly(SGF.F)) {
// The load can only be a take if the base is a +1 rvalue.
auto shouldTake = IsTake_t(base.hasCleanup());

auto isGuaranteed = selfParam.isGuaranteed();

auto context =
isGuaranteed ? SGFContext::AllowImmediatePlusZero : SGFContext();

base = SGF.emitFormalAccessLoad(loc, base.forward(SGF),
SGF.getTypeLowering(baseLoweredType),
SGFContext(), shouldTake);
context, shouldTake, isGuaranteed);
return ArgumentSource(loc, RValue(SGF, loc, baseFormalType, base));
}

// If the type is address-only, we can borrow the memory location as is.
if (base.getType().isAddressOnly(SGF.F)) {
return ArgumentSource(loc, RValue(SGF, loc, baseFormalType, base));
Expand Down
9 changes: 2 additions & 7 deletions test/SILGen/boxed_existentials.swift
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,10 @@ func test_property(_ x: Error) -> String {
// CHECK-LABEL: sil hidden [ossa] @$s18boxed_existentials13test_propertyySSs5Error_pF
// CHECK: bb0([[ARG:%.*]] : @guaranteed $any Error):
// CHECK: [[VALUE:%.*]] = open_existential_box [[ARG]] : $any Error to $*[[VALUE_TYPE:@opened\(.*, any Error\) Self]]
// FIXME: Extraneous copy here
// CHECK-NEXT: [[COPY:%[0-9]+]] = alloc_stack $[[VALUE_TYPE]]
// CHECK-NEXT: copy_addr [[VALUE]] to [init] [[COPY]] : $*[[VALUE_TYPE]]
// CHECK: [[METHOD:%.*]] = witness_method $[[VALUE_TYPE]], #Error._domain!getter
// -- self parameter of witness is @in_guaranteed; no need to copy since
// value in box is immutable and box is guaranteed
// CHECK: [[RESULT:%.*]] = apply [[METHOD]]<[[VALUE_TYPE]]>([[COPY]])
// CHECK: [[RESULT:%.*]] = apply [[METHOD]]<[[VALUE_TYPE]]>([[VALUE]])
// CHECK-NOT: destroy_value [[ARG]]
// CHECK: return [[RESULT]]

Expand All @@ -93,10 +90,8 @@ func test_property_of_lvalue(_ x: Error) -> String {
// CHECK: [[COPY:%.*]] = alloc_stack $[[VALUE_TYPE]]
// CHECK: copy_addr [[VALUE]] to [init] [[COPY]]
// CHECK: destroy_value [[VALUE_BOX]]
// CHECK: [[BORROW:%.*]] = alloc_stack $[[VALUE_TYPE]]
// CHECK: copy_addr [[COPY]] to [init] [[BORROW]]
// CHECK: [[METHOD:%.*]] = witness_method $[[VALUE_TYPE]], #Error._domain!getter
// CHECK: [[RESULT:%.*]] = apply [[METHOD]]<[[VALUE_TYPE]]>([[BORROW]])
// CHECK: [[RESULT:%.*]] = apply [[METHOD]]<[[VALUE_TYPE]]>([[COPY]])
// CHECK: destroy_addr [[COPY]]
// CHECK: dealloc_stack [[COPY]]
// CHECK: end_borrow [[VAR_LIFETIME]]
Expand Down
12 changes: 3 additions & 9 deletions test/SILGen/copy_expr.swift
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,8 @@ func testCallMethodOnLoadableGlobal() {
// Calling computedK. It is borrowed.
// CHECK: [[TEMP:%.*]] = alloc_stack $T
// CHECK: explicit_copy_addr [[X]] to [init] [[TEMP]]
// CHECK: [[TEMP2:%.*]] = alloc_stack $T
// CHECK: copy_addr [[TEMP]] to [init] [[TEMP2]]
// CHECK: [[FUNC:%.*]] = witness_method $T, #P.computedK!getter : <Self where Self : P> (Self) -> () -> Klass : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @owned Klass
// CHECK: apply [[FUNC]]<(T)>([[TEMP2]])
// CHECK: apply [[FUNC]]<(T)>([[TEMP]])
//
// Calling computed consuming getter.
// CHECK: [[TEMP:%.*]] = alloc_stack $T
Expand Down Expand Up @@ -378,10 +376,8 @@ func testCallMethodOnAddressOnlyLetCopy<T : P>(_ t: T.Type) {
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
// CHECK: [[TEMP:%.*]] = alloc_stack $
// CHECK: explicit_copy_addr [[ACCESS]] to [init] [[TEMP]]
// CHECK: [[TEMP2:%.*]] = alloc_stack $
// CHECK: copy_addr [[TEMP]] to [init] [[TEMP2]]
// CHECK: [[FUNC:%.*]] = witness_method $T, #P.computedK!getter : <Self where Self : P> (Self) -> () -> Klass : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @owned Klass
// CHECK: apply [[FUNC]]<(T)>([[TEMP2]])
// CHECK: apply [[FUNC]]<(T)>([[TEMP]])
//
// Consuming computed getter.
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[PROJECT]]
Expand Down Expand Up @@ -423,10 +419,8 @@ func testCallMethodOnAddressOnlyVarCopy<T : P>(_ t: T.Type) {
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[ARG]]
// CHECK: [[TEMP:%.*]] = alloc_stack $
// CHECK: explicit_copy_addr [[ACCESS]] to [init] [[TEMP]]
// CHECK: [[TEMP2:%.*]] = alloc_stack $
// CHECK: copy_addr [[TEMP]] to [init] [[TEMP2]]
// CHECK: [[FUNC:%.*]] = witness_method $T, #P.computedK!getter : <Self where Self : P> (Self) -> () -> Klass : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @owned Klass
// CHECK: apply [[FUNC]]<(T)>([[TEMP2]])
// CHECK: apply [[FUNC]]<(T)>([[TEMP]])
//
// Consuming computed getter.
// CHECK: [[ACCESS:%.*]] = begin_access [read] [unknown] [[ARG]]
Expand Down
12 changes: 2 additions & 10 deletions test/SILGen/dynamic_self.swift
Original file line number Diff line number Diff line change
Expand Up @@ -96,27 +96,19 @@ func testExistentialDispatch(p: P) {

// CHECK: [[PCOPY_ADDR:%[0-9]+]] = open_existential_addr immutable_access [[P]] : $*any P to $*@opened([[N:".*"]], any P) Self
// CHECK: [[P_RESULT:%[0-9]+]] = alloc_stack $any P
// CHECK: [[PCOPY_ADDR_1:%[0-9]+]] = alloc_stack $@opened([[N]], any P) Self
// CHECK: copy_addr [[PCOPY_ADDR]] to [init] [[PCOPY_ADDR_1]] : $*@opened([[N]], any P) Self
// CHECK: [[P_P_GETTER:%[0-9]+]] = witness_method $@opened([[N]], any P) Self, #P.p!getter : {{.*}}, [[PCOPY_ADDR]]{{.*}} : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @out τ_0_0
// CHECK: [[P_RESULT_ADDR2:%[0-9]+]] = init_existential_addr [[P_RESULT]] : $*any P, $@opened([[N]], any P) Self
// CHECK: apply [[P_P_GETTER]]<@opened([[N]], any P) Self>([[P_RESULT_ADDR2]], [[PCOPY_ADDR_1]]) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @out τ_0_0
// CHECK: destroy_addr [[PCOPY_ADDR_1]] : $*@opened([[N]], any P) Self
// CHECK: apply [[P_P_GETTER]]<@opened([[N]], any P) Self>([[P_RESULT_ADDR2]], [[PCOPY_ADDR]]) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @out τ_0_0
// CHECK: destroy_addr [[P_RESULT]] : $*any P
// CHECK: dealloc_stack [[PCOPY_ADDR_1]] : $*@opened([[N]], any P) Self
// CHECK: dealloc_stack [[P_RESULT]] : $*any P
_ = p.p

// CHECK: [[PCOPY_ADDR:%[0-9]+]] = open_existential_addr immutable_access [[P]] : $*any P to $*@opened([[N:".*"]], any P) Self
// CHECK: [[P_RESULT:%[0-9]+]] = alloc_stack $any P
// CHECK: [[PCOPY_ADDR_1:%[0-9]+]] = alloc_stack $@opened([[N]], any P) Self
// CHECK: copy_addr [[PCOPY_ADDR]] to [init] [[PCOPY_ADDR_1]] : $*@opened([[N]], any P) Self
// CHECK: [[P_SUBSCRIPT_GETTER:%[0-9]+]] = witness_method $@opened([[N]], any P) Self, #P.subscript!getter : {{.*}}, [[PCOPY_ADDR]]{{.*}} : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @out τ_0_0
// CHECK: [[P_RESULT_ADDR:%[0-9]+]] = init_existential_addr [[P_RESULT]] : $*any P, $@opened([[N]], any P) Self
// CHECK: apply [[P_SUBSCRIPT_GETTER]]<@opened([[N]], any P) Self>([[P_RESULT_ADDR]], [[PCOPY_ADDR_1]]) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @out τ_0_0
// CHECK: destroy_addr [[PCOPY_ADDR_1]] : $*@opened([[N]], any P) Self
// CHECK: apply [[P_SUBSCRIPT_GETTER]]<@opened([[N]], any P) Self>([[P_RESULT_ADDR]], [[PCOPY_ADDR]]) : $@convention(witness_method: P) <τ_0_0 where τ_0_0 : P> (@in_guaranteed τ_0_0) -> @out τ_0_0
// CHECK: destroy_addr [[P_RESULT]] : $*any P
// CHECK: dealloc_stack [[PCOPY_ADDR_1]] : $*@opened([[N]], any P) Self
// CHECK: dealloc_stack [[P_RESULT]] : $*any P
_ = p[]
}
Expand Down
Loading