This corresponds to the parameter-passing convention of the Itanium C++
ABI, in which the argument is passed indirectly and possibly modified,
but not destroyed, by the callee.
@in_cxx is handled the same way as @in in callers and @in_guaranteed in
callees. OwnershipModelEliminator emits the call to destroy_addr that is
needed to destroy the argument in the caller.
rdar://122707697
Although I don't plan to bring over new assertions wholesale
into the current qualification branch, it's entirely possible
that various minor changes in main will use the new assertions;
having this basic support in the release branch will simplify that.
(This is why I'm adding the includes as a separate pass from
rewriting the individual assertions)
The subexpression of a MaterializePackExpr (which is always a tuple value
currently) is emitted while preparing to emit a pack expansion expr, and its
elements are projected from within the dynamic pack loop. This means that a
materialized pack is only evaluated once, rather than being evaluated on
every iteration over the pack elements.
- SILPackType carries whether the elements are stored directly
in the pack, which we're not currently using in the lowering,
but it's probably something we'll want in the final ABI.
Having this also makes it clear that we're doing the right
thing with substitution and element lowering. I also toyed
with making this a scalar type, which made it necessary in
various places, although eventually I pulled back to the
design where we always use packs as addresses.
- Pack boundaries are a core ABI concept, so the lowering has
to wrap parameter pack expansions up as packs. There are huge
unimplemented holes here where the abstraction pattern will
need to tell us how many elements to gather into the pack,
but a naive approach is good enough to get things off the
ground.
- Pack conventions are related to the existing parameter and
result conventions, but they're different on enough grounds
that they deserve to be separated.
Currently PMO tries to scalarize empty tuple, and ends up deleting
store of an empty tuple. This causes a cascade of problems.
Mem2Reg can end up seeing loads without stores of empty tuple type,
and creates undef while optimizing the alloca away.
This is bad, we don't want to create undef's unnecessarily in SIL.
Some optimization can queiry the function or the block on the undef leading to nullptr
and a compiler crash.
Fixes rdar://94829482
This mostly requires changing various entry points to pass around a
TypeConverter instead of a SILModule. I've left behind entry points
that take a SILModule for a few methods like SILType::subst() to
avoid creating even more churn.
Add `llvm_unreachable` to mark covered switches which MSVC does not
analyze correctly and believes that there exists a path through the
function without a return value.
This reduces the diff in between -Onone output when stripping before/after
serialization.
We support load_borrow by translating it to the load [copy] case. Specifically,
for +1, we normally perform the following transform.
store %1 to [init] %0
...
%2 = load [copy] %0
...
use(%2)
...
destroy_value %2
=>
%1a = copy_value %1
store %1 to [init] %0
...
use(%1a)
...
destroy_value %1a
We analogously can optimize load_borrow by replacing the load with a
begin_borrow:
store %1 to [init] %0
...
%2 = load_borrow %0
...
use(%2)
...
end_borrow %2
=>
%1a = copy_value %1
store %1 to [init] %0
...
%2 = begin_borrow %1a
...
use(%2)
...
end_borrow %2
destroy_value %1a
The store from outside a loop being used by a load_borrow inside a loop is a
similar transformation as the +0 version except that we use a begin_borrow
inside the loop instead of a copy_value (making it even more efficient).
PMO uses InitOrAssign for trivially typed things and Init/Assign for non-trivial
things, so I think this was an oversight from a long time ago. There is actually
no /real/ effect on the code today since after exploding the copy_addr, the
store will still be used to produce the right available value and since for
stores, init/assign/initorassign all result in allocations being removed. Once
though I change assign to not allow for allocation removal (the proper way to
model this), without this change, certain trivial allocations will no longer be
removed, harming perf as seen via the benchmarking run on the bots in #21918.
I am removing these for the following reasons:
* PMO does not have any tests for these code paths. (1).
* PMO does not try to promote these loads (it explicitly pattern matches load,
copy_addr) or get available values from these (it explicitly pattern matches
store or explodes a copy_addr to get the copy_addr's stores). This means that
removing this code will not effect our constant propagation diagnostics. So,
removing this untested code path at worst could cause us to no longer
eliminate some dead objects that we otherwise would be able to eliminate at
-Onone (low-priority). (2).
----
(1). I believe that the lack of PMO tests is due to this being a vestigal
remnant of DI code in PMO. My suspicion arises since:
* The code was added when the two passes were both sharing the same use
collector and auxillary data structures. Since then I have changed DI/PMO
to each have their own copies.
* DI has a bunch of tests that verify behavior around these instructions.
(2). I expect the number of actually removed allocations that are no longer
removed should be small since we do not promote loads from such allocations
and PMO will not eliminate an allocation that has any loads.
PartialStore is a PMOUseKind that is a vestigal remnant of Definite Init in the
PMO source. This can be seen by noting that in Definite Init, PartialStore is
how Definite Init diagnoses partially initialized values and errors. In contrast
in PMO the semantics of PartialStore are:
1. It can only be produced if we have a raw store use or a copy_addr.
2. We allow for the use to provide an available value just like if it was an
assign or an init.
3. We ignore it for the purposes of removing store only allocations since by
itself without ownership, stores (and stores from exploded copy_addr) do not
effect ownership in any way.
Rather than keeping this around, in this commit I review it since it doesn't
provide any additional value or [init] or [assign]. Functionally there should be
no change.
This will cause this code to automagically work correctly in ossa code since we
will instead of emitting the tuple_extracts will emit a destructure operation
automagically without code change.
Since this entrypoint will emit tuple_extracts in non-ossa code, this is a NFC
patch.
This is technically a NFC commit. The changes are:
1. When we scalarize loads/stores, we need to not just use unqualified
loads/stores. Instead we need to use the createTrivial{Load,Store}Or APIs. In
OSSA mode, this will propagate through the original ownership qualifier if the
sub-type is non-trivial, but if the sub-type is non-trivial will change the
qualifier to trivial. Today when the pass runs without ownership nothing is
changed since I am passing in the "supports unqualified" flag to the
createTrivial{Load,Store}Or API so that we just create an unqualified memop if
we are passed in an unqualified memop. Once we fully move pmo to ownership, this
flag will be removed and we will assert.
2. The container walker is taught about copy_value, destroy_value. Specifically,
we teach the walker how to recursively look through copy_values during the walk
and to treat a destroy_value of the box like a strong_release,
release_value. Since destroy_value, copy_value only exist in [ossa] today, this
is also NFC.
Since:
1. We only handle alloc_stack, alloc_box in predictable memopts.
2. alloc_stack can not be released.
We know that the release collecting in collectFrom can just be done in
collectContainerUses() [which only processes alloc_box].
This also let me simplify some code as well and add a defensive check in case
for some reason we are passed a release_value on the box. NOTE: I verified that
previously this did not result in a bug since we would consider the
release_value to be an escape of the underlying value even though we didn't
handle it in collectFrom. But the proper way to handle release_value is like
strong_release, so I added code to do that as well.
TLDR: This does not eliminate the struct/tuple flat namespace from Predictable
Mem Opts. Just the tuple specific flat namespace code from PMOMemoryUseCollector
that we were computing and then throwing away. I explain below in more detail.
First note that this is cruft from when def-init and pmo were one pass. What we
were doing here was maintaing a flattened tuple namespace while we were
collecting uses in PMOMemoryUseCollector. We never actually used them for
anything since we recomputed this information including information about
structs in PMO itself! So this information was truly completely dead.
This commit removes that and related logic and from a maintenance standpoint
makes PMOMemoryUseCollector a simple visitor that doesn't have any real special
logic in it beyond the tuple scalarization.
Specifically, we are putting dealloc_stack, destroy_box into the Releases array
in PMOMemoryUseCollector only to ignore them in the only place that we use the
Releases array in PredictableMemOpts.
Just a part of a series of small cleanups I found over the break in PMO that I
am landing in preparation for landing patches that fix PMO for ownership.
This was done early on during the split of predictable mem opts from DI. This
has been done for a long time, so eliminate the "Ownership" basename suffix.
This was never implemented correctly way back in 2013-2014. It was originally
added I believe so we could DI checks, but the promotion part was never added.
Given that DI is now completely split from PMO, we can just turn this off and if
necessary add it back on master "properly".
rdar://41161408
I am going to DI and predictable mem opts have been split for a long time and
their subroutines aren't going to be joined in the future... so replace the DI
prefixes in pred-mem-opts impl with PMO and rename DIMemoryUseCollector =>
PMOUseCollector.
Been sitting on this for a long time... just happy to get it in.
