Support for address based array initialization under opaque values mode #7470
Conversation
|
@swift-ci Please smoke test |
lib/SILGen/SILGenApply.cpp
Outdated
| @@ -2965,12 +2965,15 @@ namespace { | |||
| // it's not already there. (Note that this potentially includes | |||
| // conventions which pass indirectly without transferring | |||
| // ownership, like Itanium C++.) | |||
| if (SGF.silConv.isSILIndirect(param)) { | |||
| if (SGF.silConv.isSILIndirect(param) || specialDest) { | |||
There was a problem hiding this comment.
This seems a tad convoluted. Would it work to do this?
if (specialDest) {
emitIndirectInto
Args.push
return
}
if (isSILIndirect) {
emitIndirect
Args.push
return
}
lib/SILGen/SILGenApply.cpp
Outdated
| emitIndirectInto(std::move(arg), origParamType, | ||
| loweredSubstParamType, *specialDest); | ||
| Args.push_back(ManagedValue::forInContext()); | ||
| } else { | ||
| assert(SGF.silConv.isSILIndirect(param)); |
There was a problem hiding this comment.
Can you add an && message here?
There was a problem hiding this comment.
That assert was removed in updated PR - added && message to the new asserts
| // CHECK: apply %{{.*}}<Any>(%{{.*}}) : $@convention(thin) <τ_0_0> (Builtin.Word) -> (@owned Array<τ_0_0>, Builtin.RawPointer) | ||
| // CHECK: tuple_extract %{{.*}} : $(Array<Any>, Builtin.RawPointer) | ||
| // CHECK: pointer_to_address %{{.*}} : $Builtin.RawPointer to [strict] $*Any | ||
| // CHECK: init_existential_addr %{{.*}} : $*Any, $Int |
There was a problem hiding this comment.
Can you add more precise FileCheck lines here? I prefer to use register matching like:
[[FOO:%.*]]
Also can you use a } // end sil function '' check to make sure that we do not match lines below?
I have run into many of these issues in SILGen = /.
There was a problem hiding this comment.
Done - updated test case in PR
shajrawi
force-pushed
the
var_args_SpecialDest_opaque_mode
branch
from
6d13bff to
90dfd7f
Compare
|
@swift-ci Please smoke test |
| // CHECK: %[[APY:.*]] = apply %{{.*}}<Any>(%{{.*}}) : $@convention(thin) <τ_0_0> (Builtin.Word) -> (@owned Array<τ_0_0>, Builtin.RawPointer) | ||
| // CHECK: %[[BRW:.*]] = begin_borrow %[[APY]] | ||
| // CHECK: %[[TPL:.*]] = tuple_extract %[[BRW]] : $(Array<Any>, Builtin.RawPointer), 1 | ||
| // CHECK: %[[PTR:.*]] = pointer_to_address %[[TPL]] : $Builtin.RawPointer to [strict] $*Any |
There was a problem hiding this comment.
Can you add more to the test case for ownership. Specifically, can you make sure that we are ending the borrow and properly destroying APY?
shajrawi
force-pushed
the
var_args_SpecialDest_opaque_mode
branch
from
90dfd7f to
0e8e6d7
Compare
shajrawi
force-pushed
the
var_args_SpecialDest_opaque_mode
branch
from
0e8e6d7 to
03d55cd
Compare
|
@swift-ci Please smoke test |
part of rdar://problem/30080769
We have to keep (some) address operations on existentials unless we change the way we initialize arrays:
Consider the following code-patern:
Above we are initializing an array of any, we have a pointer to the array’s storage (which is actually a memory address), the init_existential_addr instruction initializes the array’s storage before we store the integer literal ‘3’ into it.
Adding a new init_existential instruction that takes an opaque value does not make any sense in here, we have to keep using the _addr instruction and deal with addresses.
This patch enables the pattern described above in opaque values mode.