Fixes <rdar://40555427> [SR-7773]:
SILCombiner::propagateConcreteTypeOfInitExistential fails to full propagate type
substitutions.
Fixes <rdar://problem/40923849>
SILCombiner::propagateConcreteTypeOfInitExistential crashes on protocol
compositions.
This rewrite fixes several fundamental bugs in the SILCombiner optimization that
propagates concrete types. In particular, the pass needs to handle:
- Arguments of callee Self type in non-self position.
- Indirect and direct return values of Self type.
- Types that indirectly depend on Self within callee function signature.
- Protocol composition existentials.
- All of the above need to work for protocol extensions as well as witness methods.
- For protocol extensions, conformance lookup should be based on the existential's conformance list.
Additionally, the optimization should not depend on a SILFunction's DeclContext,
which is not serialized. (In fact, we should prevent SIL passes from using
DeclContext). Furthermore, the code needs to be expressed in a way that one can
reason about correctness and invariants.
The root cause of these bugs is that SIL passes are written based on untested
assumptions of Swift type system. A SIL pass needs to handle all verifiable SIL
input because passes need to be composable. Bail-out logic can be added to
simplify the design; however, _the bail-out logic itself cannot make any
assumptions about the language or type system_ that aren't clearly and
explicitly enforced in the SIL verifier. This is a common mistake and major
source of bugs.
I created as many unit tests as I reasonably could to prevent this code from
regressing. Creating enough unit tests to cover all corner cases that were
broken in the original code would be intractable. But the code has been
simplified such that many corner cases disappear.
This opens up some oportunity for generalizing the optimization and eliminating
special cases. However, I want this PR to be limited to fixing correctness
issues only. In the long term, it would be preferable to replace this
optimization entirely with a much more powerful general type propagation pass.
Introduced during the bring-up of the generics system in July, 2012,
Substitution (and SubstitutionList) has been completely superseded by
SubstitutionMap. R.I.P.
This is a property of an instruction and should be a member
function of `SILInstruction` and not a free function in
`DebugUtils`. Discussed with Adrian.
Certain patterns of directly applied partial_apply's were not being
inlined. This can happen when a closure is defined in the
implementation of a generic witness method. I noticed this issue while
debugging SR-6254: "Fix (or explain) strange ways to make code >3
times faster/slower".
After the witness method is devirtualization and specialized, the
closure application looks like this:
%pa = partial_apply [callee_guaranteed] %fn() : $@convention(thin) (@in Int) -> @out Int
%cvt = convert_escape_to_noescape %pa
apply %cvt(...)
SILCombine already removes the partial_apply and convert_escape_to_noescape generating:
%thick = thin_to_thick %fn
apply %thick(...)
However, surprisingly, neither the inliner nor SILCombine can handle this.
I cleaned up the code in SILCombine's apply visitor that handles
thin_to_thick and generalized it to handle this and other patterns.
I also added a restriction to this code so that it doesn't break SIL
ownership in the future, as I discussed with Arnold.
Certain patterns of directly applied partial_apply's were not being
inlined. This can happen when a closure is defined in the
implementation of a generic witness method. I noticed this issue while
debugging SR-6254: "Fix (or explain) strange ways to make code >3
times faster/slower".
After the witness method is devirtualization and specialized, the
closure application looks like this:
%pa = partial_apply [callee_guaranteed] %fn() : $@convention(thin) (@in Int) -> @out Int
%cvt = convert_escape_to_noescape %pa
apply %cvt(...)
SILCombine already removes the partial_apply and convert_escape_to_noescape generating:
%thick = thin_to_thick %fn
apply %thick(...)
However, surprisingly, neither the inliner nor SILCombine can handle this.
I cleaned up the code in SILCombine's apply visitor that handles
thin_to_thick and generalized it to handle this and other patterns.
I also added a restriction to this code so that it doesn't break SIL
ownership in the future, as I discussed with Arnold.
This patch both makes debug variable information it optional on
alloc_stack and alloc_box instructions, and forced variable
information on debug_value and debug_value_addr instructions. The
change of the interface uncovered a plethora of bugs in SILGen,
SILTransform, and IRGen's LoadableByAddress pass.
Most importantly this fixes the previously commented part of the
DebugInfo/local-vars.swift.gyb testcase.
rdar://problem/37720555
Create helpers in InstructionUtils.h wherever we need a guarantee that the diagnostics cover the same patterns as the verifier. Eventually this will be called from both SILVerifier and the diagnostic pass:
- findAccessedAddressBase
- isPossibleFormalAccessBase
- isPartialApplyOfReabstractionThunk
- findClosureForAppliedArg
- visitAccessedAddress
Add partial_apply verification assert.
This applies the normal "find a closure" logic inside the "find all partial_apply uses" verification. Making the verifier round-trip ensures that we don't have holes in exclusivity enforcement related to this logic.
The bug was introduced recently:
commit ac8a48b2
Date: Fri Oct 6 12:34:14 2017 -0700
Fixes <rdar://35800315> (crash using freed OpenedArchetypesTracker).
This is not NFC because the isUseAfterFree helper and surrounding code
is rewritten. The previous code's intention is unclear, but it was at
best imprecise and unsafe w.r.t. future SIL changes.
Pattern matching code that becomes highly complicated should
be commented with motivating SIL examples.
Support for @noescape SILFunctionTypes.
These are the underlying SIL changes necessary to implement the new
closure capture ABI.
Note: This includes a change to function name mangling that
primarily affects reabstraction thunks.
The new ABI will allow stack allocation of non-escaping closures as a
simple optimization.
The new ABI, and the stack allocation optimization, also require
closure context to be @guaranteed. That will be implemented as the
next step.
Many SIL passes pattern match partial_apply sequences. These all
needed to be fixed to handle the convert_function that SILGen now
emits. The conversion is now needed whenever a function declaration,
which has an escaping type, is passed into a @NoEscape argument.
In addition to supporting new SIL patterns, some optimizations like
inlining and SIL combine are now stronger which could perturb some
benchmark results.
These underlying SIL changes should be merged now to avoid conflicting
with other work. Minor benchmark discrepancies can be investigated as part of
the stack-allocation work.
* Add a noescape attribute to SILFunctionType.
And set this attribute correctly when lowering formal function types to SILFunctionTypes based on @escaping.
This will allow stack allocation of closures, and unblock a related ABI change.
* Flip the polarity on @noescape on SILFunctionType and clarify that
we don't default it.
* Emit withoutActuallyEscaping using a convert_function instruction.
It might be better to use a specialized instruction here, but I'll leave that up to Andy.
Andy: And I'll leave that to Arnold who is implementing SIL support for guaranteed ownership of thick function types.
* Fix SILGen and SIL Parsing.
* Fix the LoadableByAddress pass.
* Fix ClosureSpecializer.
* Fix performance inliner constant propagation.
* Fix the PartialApplyCombiner.
* Adjust SILFunctionType for thunks.
* Add mangling for @noescape/@escaping.
* Fix test cases for @noescape attribute, mangling, convert_function, etc.
* Fix exclusivity test cases.
* Fix AccessEnforcement.
* Fix SILCombine of convert_function -> apply.
* Fix ObjC bridging thunks.
* Various MandatoryInlining fixes.
* Fix SILCombine optimizeApplyOfConvertFunction.
* Fix more test cases after merging (again).
* Fix ClosureSpecializer. Hande convert_function cloning.
Be conservative when combining convert_function. Most of our code doesn't know
how to deal with function type mismatches yet.
* Fix MandatoryInlining.
Be conservative with function conversion. The inliner does not yet know how to
cast arguments or convert between throwing forms.
* Fix PartialApplyCombiner.
Except GenericEnvironment.h, because you can't meaningfully use a
GenericEnvironment without its signature. Lots less depends on
GenericSignature.h now. NFC
In case of a mutating method call we replaced the self argument with the source of a copy_addr. This let the call modify the wrong self value.
rdar://problem/34753633
introduce a common superclass, SILNode.
This is in preparation for allowing instructions to have multiple
results. It is also a somewhat more elegant representation for
instructions that have zero results. Instructions that are known
to have exactly one result inherit from a class, SingleValueInstruction,
that subclasses both ValueBase and SILInstruction. Some care must be
taken when working with SILNode pointers and testing for equality;
please see the comment on SILNode for more information.
A number of SIL passes needed to be updated in order to handle this
new distinction between SIL values and SIL instructions.
Note that the SIL parser is now stricter about not trying to assign
a result value from an instruction (like 'return' or 'strong_retain')
that does not produce any.
This addresses the unsupported case outlined in the FIXME.
We have an init_existential_{addr,ref} instruction which
constructs an existential of type R from a value whose
concrete type conforms to R.
Later on, we have a witness_method invoking a method of P,
where R refines Q refines P.
In order to give the witness_method instruction the correct
conformance after substituting the concrete type, we would
call ProtocolConformanceRef::getInherited().
However, this only supported a single level of protocol
refinement, so we would just bail out in the unsupported
case. Instead, this patch builds a SubstitutionMap from the
generic signature <T : R>, and uses the SubstitutionMap to
perform the conformance lookup of T to P.
Indirectly inherited conformances cannot be properly handled yet due to some representation issues with init_existential_* instructions.
This fixes a compiler crash reported in rdar://34022255.
What is going on here is that currently this optimization if itneeds to perform
lfietime extension always creates an alloc_stack at the beginning/end of a
function. If the object whose lifetime is being extended has a dependent type,
then the alloc_stack will be created before the dependent type exists resulting
in the compiler crashing.
rdar://33595317
Till now createApply, createTryApply, createPartialApply were taking some arguments like SubstCalleeType or ResultType. But these arguments are redundant and can be easily derived from other arguments of these functions. There is no need to put the burden of their computation on the clients of these APIs.
The removal of these redundant parameters simplifies the APIs and reduces the possibility of providing mismatched types by clients, which often happened in the past.
Replace `NameOfType foo = dyn_cast<NameOfType>(bar)` with DRY version `auto foo = dyn_cast<NameOfType>(bar)`.
The DRY auto version is by far the dominant form already used in the repo, so this PR merely brings the exceptional cases (redundant repetition form) in line with the dominant form (auto form).
See the [C++ Core Guidelines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#es11-use-auto-to-avoid-redundant-repetition-of-type-names) for a general discussion on why to use `auto` to avoid redundant repetition of type names.
