Preserve ownership for empty non-trivial structs. This currently applies to
~Escapable structs. People often use empty structs to investigate language
behavior. They should behave just like a struct that wraps a
pointer.
Previously, this would crash later during OSSA lifetime completion:
Assertion failed: (isa<UnreachableInst>(block->getTerminator())),
function computeRegion, file OSSALifetimeCompletion.cpp.
Type annotations for instruction operands are omitted, e.g.
```
%3 = struct $S(%1, %2)
```
Operand types are redundant anyway and were only used for sanity checking in the SIL parser.
But: operand types _are_ printed if the definition of the operand value was not printed yet.
This happens:
* if the block with the definition appears after the block where the operand's instruction is located
* if a block or instruction is printed in isolation, e.g. in a debugger
The old behavior can be restored with `-Xllvm -sil-print-types`.
This option is added to many existing test files which check for operand types in their check-lines.
Previously, `isScopeAffectingInstructionDead` determined that
an otherwise satisfactory `load` was not dead if it was a `load [take]`.
The immediate effect was that dead `load [take]`s were not deleted. The
downstream effect was that otherwise dead graphs of instructions would
not be deleted. This was especially a problem for OSLogOptimization
which deletes a great deal of code.
Here, the InstructionDeleter is taught to compensate for the deletion of
such `load [take]` by inserting `destroy_addr`s in their stead.
rdar://117011668
This commit is fixing two things:
1. In certain cases, we are seeing cases where either SILGen or the optimizer
are eliminating destroy_addr along paths where we know that an enum is
dynamically trivial. This can not be expressed in OSSA, so I added code to
pred-deadalloc-elim so that I check if any of our available values after we
finish promoting away an allocation now need to have their consuming use set
completed.
2. That led me to discover that in certain cases load [take] that we were
promoting were available values of other load [take]. This means that we have a
memory safety issue if we promote one load before the other. Consider the
following SIL:
```
%mem = alloc_stack
store %arg to [init] %mem
%0 = load [take] %mem
store %0 to [init] %mem
%1 = load [take] %mem
destroy_value %1
dealloc_stack %mem
```
In this case, if we eliminate %0 before we eliminate %1, we will have a stale
pointer to %0.
I also took this as an opportunity to turn off predictable mem access opt on SIL
that was deserialized canonicalized and non-OSSA SIL. We evidently need to still
do this for pred mem opts for perf reasons (not sure why). But I am pretty sure
this isn't needed and allows me to avoid some nasty code.
I am doing this to eliminate some differences in codegen before/after
serialization ownership. It just means less of the tests need to be touched when
I flip the switch.
Specifically, today this change allows us to handle certain cases where there is
a dead allocation being used to pass around a value at +1 by performing a load
[take] and then storing a value back into the memory. The general format is an
allocation that only has stores, load [take], and destroy_addr users. Consider
the following SIL:
```
store %x to [init] %mem (0)
%xhat = load [take] %mem (1)
%xhat_cast = apply %f(%xhat) (2)
store %xhat_cast to [init] %mem (3)
destroy_addr %mem
```
Notice how assuming that we can get rid of the store, we can perform the
following store -> load forwarding:
```
%xhat_cast = apply %f(%x) (2)
store %xhat_cast to [init] %mem (3)
destroy_addr %mem
```
In contrast, notice how we get an ownership violation (double consume of %x by
(0) and (2)) if we can not get rid of the store:
```
store %x to [init] %mem
%xhat_cast = apply %f(%x)
store %xhat_cast to [init] %mem (2)
destroy_addr %mem
```
This is in fact the same condition for promoting a destroy_addr since when a
destroy_addr is a load [take] + destroy_value. So I was able to generalize the
code for destroy_addr to handle this case.
I also removed the -verify-sil-ownership flag in favor of a disable flag
-disable-sil-ownership-verifier. I used this on only two tests that still need
work to get them to pass with ownership, but whose problems are well understood,
small corner cases. I am going to fix them in follow on commits. I detail them
below:
1. SILOptimizer/definite_init_inout_super_init.swift. This is a test case where
DI is supposed to error. The only problem is that we crash before we error since
the code emitting by SILGen to trigger this error does not pass ownership
invariants. I have spoken with JoeG about this and he suggested that I fix this
earlier in the compiler. Since we do not run the ownership verifier without
asserts enabled, this should not affect compiler users. Given that it has
triggered DI errors previously I think it is safe to disable ownership here.
2. PrintAsObjC/extensions.swift. In this case, the signature generated by type
lowering for one of the thunks here uses an unsafe +0 return value instead of
doing an autorelease return. The ownership checker rightly flags this leak. This
is going to require either an AST level change or a change to TypeLowering. I
think it is safe to turn this off since it is such a corner case that it was
found by a test that has nothing to do with it.
rdar://43398898
I have been meaning to do this change for a minute, but kept on putting it off.
This describes what is actually happening and is a better name for the option.
I also translated predictable_deadalloc_elim.sil into an ownership test and
added more tests that double checks the ownership specific functionality. This
change should be NFC without ownership.
