While tightening the requirements of the debug info generator in
IRGenSIL I noticed that SILCloner didn't correctly transfer variable
debug info on alloc_box and alloc_stack instructions. In order to make
these mistakes easier to find I added an assertion to SILBuilder and
fixed all issues uncovered by that assertion, too.
The result is a moderate increase in debug info coverage in optimized code.
On stdlib/public/core/OSX/x86_64/Swift.o "variables with location"
increases from 60134 to 60299.
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.
This provides a singular instruction for convert an unmanaged value to a ref,
then strong_retain it. I expanded the definition of UNCHECKED_REF_STORAGE to
include these copy like instructions. This instruction is valid in all SIL.
The reason why I am adding this instruction is that currently when we emit an
access to an unowned (unsafe) ivar, we use an unmanaged_to_ref and a strong
retain. This can look to the optimizer like a strong retain that can potentially
be optimized. By combining the two together into a new instruction, we can avoid
this potential problem since the pattern matching will break.
This flag is set by DefinitInitialization if the lifetime of the stored value is controlled dynamically.
If the flag is set, it's not (easily) possibly to statically calculate the lifetime of the stored value.
Specifically:
1. I removed an extra defensive copy that we put in place some time ago that
isn't really warranted. We know that we have an @owned value, so can safely just
pass the value as a @guaranteed parameter. This also eliminates an ownership
error that would occur due to my not having updated this code for ownership in
tree.
2. I also ensured that if we are performing a loadable address bridging cast ->
value bridging cast that we store the loadable value back into memory after we
perform the cast. Otherwise, it appears to leak to the ownership verifier.
I also centralized the non-ownership tests for this into one place
(const_fold_objc_bridge.sil => constant_propagation_objc.sil).
Specifically, when we optimize conversions such as:
Optional<@escaping () -> ()>
->
Optional<@noescape () -> ()>
->
Optional<@noescape @convention(block) () -> ()>
previously we were lifetime extending over the @noescape lifetime barrier by
making a copy and then putting a mark_dependence from the copy onto the original
value. This was just a quick way to tell the ownership verifier that the copy
was tied to the other value and thus should not be eliminated. The correctness
of the actual lifetime extension comes from the optimizer being conservative
around rr insts.
This commit instead changes our optimization to borrow the copied optional
value, extract the payload, and use that instead.
This is a large patch; I couldn't split it up further while still
keeping things working. There are four things being changed at
once here:
- Places that call SILType::isAddressOnly()/isLoadable() now call
the SILFunction overload and not the SILModule one.
- SILFunction's overloads of getTypeLowering() and getLoweredType()
now pass the function's resilience expansion down, instead of
hardcoding ResilienceExpansion::Minimal.
- Various other places with '// FIXME: Expansion' now use a better
resilience expansion.
- A few tests were updated to reflect SILGen's improved code
generation, and some new tests are added to cover more code paths
that previously were uncovered and only manifested themselves as
standard library build failures while I was working on this change.
Each call site will soon have to think about passing in the right expansion
instead of just assuming the default will be OK. But there are now only a
few call sites left, because most have been refactored to use convenience
APIs that pass in the right resilience expansion already.
This comes up because when we perform mandatory inlining, we perform the
transform as we inline. So the tests for this are in mandatory_inlining
naturally.
I discovered this due to the mandatory inliner doing devirtualization. I ported
all of the relevant SIL tests to increase code coverage of this code when
ownership is enabled.
It does not take ownership of its non-trivial arguments, is a trivial
function type and therefore must not be destroyed. The compiler must
make sure to extend the lifetime of non-trivial arguments beyond the
last use of the closure.
%objc = copy_value %0 : $AnObject
%closure = partial_apply [stack] [callee_guaranteed] %16(%obj) : $@convention(thin) (@guaranteed AnObject) -> ()
%closure2 = mark_dependence %closure : $@noescape @callee_guaranteed () -> () on %obj : $AnObject
%user = function_ref @useClosure : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
apply %user(%closure2) : $@convention(thin) (@noescape @callee_guaranteed () -> ()) -> ()
dealloc_stack %closure : $() ->()
destroy_value %obj : $AnObject // noescape closure does not take ownership
SR-904
rdar://35590578
Specifically we add a groups of APIs for destructure operations. The destructure
helpers are a family of functions built around
emitDestructureValueOperation. These in ossa produce destructures and pass the
results off to the caller in some manner that hides the internal destructure
instruction. In non-ossa, the appropriate projections are created and passed off
to the caller.
In a previous commit, I banned in the verifier any SILValue from producing
ValueOwnershipKind::Any in preparation for this.
This change arises out of discussions in between John, Andy, and I around
ValueOwnershipKind::Trivial. The specific realization was that this ownership
kind was an unnecessary conflation of the a type system idea (triviality) with
an ownership idea (@any, an ownership kind that is compatible with any other
ownership kind at value merge points and can only create). This caused the
ownership model to have to contort to handle the non-payloaded or trivial cases
of non-trivial enums. This is unnecessary if we just eliminate the any case and
in the verifier separately verify that trivial => @any (notice that we do not
verify that @any => trivial).
NOTE: This is technically an NFC intended change since I am just replacing
Trivial with Any. That is why if you look at the tests you will see that I
actually did not need to update anything except removing some @trivial ownership
since @any ownership is represented without writing @any in the parsed sil.
rdar://46294760
We've been running doxygen with the autobrief option for a couple of
years now. This makes the \brief markers into our comments
redundant. Since they are a visual distraction and we don't want to
encourage more \brief markers in new code either, this patch removes
them all.
Patch produced by
for i in $(git grep -l '\\brief'); do perl -pi -e 's/\\brief //g' $i & done
Previously we would always calculate these instructions ownership dynamically
when asked and rely on the ownership verifier to catch if we made any
mistakes. Instead with this commit we move to a more static model where the
ownership that these instructions can take are frozen on construction. This is a
more static model that simplifies the ownership model.
I also eliminated a few asserts that are enforced in other places that caused
problems when parsing since we may not have a Function while Parsing (it was
generally asserts if a type was trivial).
In this commit I added a more convenient API for doing this sort of operation.
Specifically: SILBuilder::emitScopedBorrowOperation. This performs either a
load_borrow or begin_borrow, then calls a user provided closure, and finally
inserts the end_borrow after the scope has closed.
rdar://43398898
Allow a new SILBuilder to be created for an insertion point, while
providing all of its necessary context.
Ultimately, the builder's constructor should take an insertion point,
DebugLocation, and context. Then we won't need to pass SILLocation to
all of its methods.
This makes much more sense and is much safer than saving the insertion
point via an RAII object or defining a separate
SILBuilderWithScope. Those broken abstractions should go away.
I changed all of the places that used end_borrow_argument to use end_borrow.
NOTE: I discovered in the process of this patch that we are not verifying
guaranteed block arguments completely. I disabled the tests here that show this
bad behavior and am going to re-enable them with more tests in a separate PR.
This has not been a problem since SILGen does not emit any such arguments as
guaranteed today. But once I do the SILGenPattern work this will change.
rdar://33440767
This does not eliminate the entrypoints on SILBuilder yet. I want to do this in
two parts so that it is functionally easier to disentangle changing the APIs
above SILBuilder and changing the underlying instruction itself.
rdar://33440767
