Mostly functionally neutral:
- may fix latent bugs.
- may reduce useless basic blocks after inlining.
This rewrite encapsulates the cloner's internal state, providing a
clean API for the CRTP subclasses. The subclasses are rewritten to use
the exposed API and extension points. This makes it much easier to
understand, work with, and extend SIL cloners, which are central to
many optimization passes. Basic SIL invariants are now clearly
expressed and enforced. There is no longer a intricate dance between
multiple levels of subclasses operating on underlying low-level data
structures. All of the logic needed to keep the original SIL in a
consistent state is contained within the SILCloner itself. Subclasses
only need to be responsible for their own modifications.
The immediate motiviation is to make CFG updates self-contained so
that SIL remains in a valid state. This will allow the removal of
critical edge splitting hacks and will allow general SIL utilities to
take advantage of the fact that we don't allow critical edges.
This rewrite establishes a simple principal that should be followed
everywhere: aside from the primitive mutation APIs on SIL data types,
each SIL utility is responsibile for leaving SIL in a valid state and
the logic for doing so should exist in one central location.
This includes, for example:
- Generating a valid CFG, splitting edges if needed.
- Returning a valid instruction iterator if any instructions are removed.
- Updating dominance.
- Updating SSA (block arguments).
(Dominance info and SSA properties are fundamental to SIL verification).
LoopInfo is also somewhat fundamental to SIL, and should generally be
updated, but it isn't required.
This also fixes some latent bugs related to iterator invalidation in
recursivelyDeleteTriviallyDeadInstructions and SILInliner. Note that
the SILModule deletion callback should be avoided. It can be useful as
a simple cache invalidation mechanism, but it is otherwise bug prone,
too limited to be very useful, and basically bad design. Utilities
that mutate should return a valid instruction iterator and provide
their own deletion callbacks.
ConvertFunction and reabstraction thunks need this attribute. Otherwise,
there is no way to identify that withoutActuallyEscaping was used
to explicitly perform a conversion.
The destination of a [without_actually_escaping] conversion always has
an escaping function type. The source may have either an escaping or
@noescape function type. The conversion itself may be a nop, and there
is nothing distinctive about it. The thing that is special about these
conversions is that the source function type may have unboxed
captures. i.e. they have @inout_aliasable parameters. Exclusivity
requires that the compiler enforce a SIL data flow invariant that
nonescaping closures with unboxed captures can never be stored or
passed as an @escaping function argument. Adding this attribute allows
the compiler to enforce the invariant in general with an escape hatch
for withoutActuallyEscaping.
To do so this commit does a few different things:
1. I changed SILOptFunctionBuilder to notify the pass manager's logging
functionality when new functions are added to the module and to notify analyses
as well. NOTE: This on purpose does not put the new function on the pass manager
worklist since we do not want to by mistake introduce a large amount of
re-optimizations. Such a thing should be explicit.
2. I eliminated SILModuleTransform::notifyAddFunction. This just performed the
operations from 1. Now that SILOptFunctionBuilder performs this operation for
us, it is not needed.
3. I changed SILFunctionTransform::notifyAddFunction to just add the function to
the passmanager worklist. It does not need to notify the pass manager's logging
or analyses that a new function was added to the module since
SILOptFunctionBuilder now performs that operation. Given its reduced
functionality, I changed the name to addFunctionToPassManagerWorklist(...). The
name is a little long/verbose, but this is a feature since one should think
before getting the pass manager to rerun transforms on a function. Also, giving
it a longer name calls out the operation in the code visually, giving this
operation more prominance when reading code. NOTE: I did the rename using
Xcode's refactoring functionality!
rdar://42301529
I am going to add the code in a bit that does the notifications. I tried to pass
down the builder instead of the pass manager. I also tried not to change the
formatting.
rdar://42301529
This commit does not modify those APIs or their usage. It just:
1. Moves the APIs onto SILFunctionBuilder and makes SILFunctionBuilder a friend
of SILModule.
2. Hides the APIs on SILModule so all users need to use SILFunctionBuilder to
create/destroy functions.
I am doing this in order to allow for adding/removing function notifications to
be enforced via the type system in the SILOptimizer. In the process of finishing
off CallerAnalysis for FSO, I discovered that we were not doing this everywhere
we need to. After considering various other options such as:
1. Verifying after all passes that the notifications were sent correctly and
asserting. Turned out to be expensive.
2. Putting a callback in SILModule. This would add an unnecessary virtual call.
I realized that by using a builder we can:
1. Enforce that users of SILFunctionBuilder can only construct composed function
builders by making the composed function builder's friends of
SILFunctionBuilder (notice I did not use the word subclass, I am talking
about a pure composition).
2. Refactor a huge amount of code in SILOpt/SILGen that involve function
creation onto a SILGenFunctionBuilder/SILOptFunctionBuilder struct. Many of
the SILFunction creation code in question are straight up copies of each
other with small variations. A builder would be a great way to simplify that
code.
3. Reduce the size of SILModule.cpp by 25% from ~30k -> ~23k making the whole
file easier to read.
NOTE: In this commit, I do not hide the constructor of SILFunctionBuilder since
I have not created the derived builder structs yet. Once I have created those in
a subsequent commit, I will hide that constructor.
rdar://42301529
The "subclass scope" is meant to represent a connection to a vtable (and how
public something needs to be), for things that end up in class
vtables. Specializations and thunks are mostly internal implementation details
and do not end up there, so subclass scope is not applicable to them. This stops
the thunks and specializations being incorrectly public.
(Note, there are some thunks that _are_ public facing: if a function has its
signature optimized, the original entry point becomes a thunk, and this entry
point is what ends up in vtables etc., so needs to remain around, which means
keeping the same hacks for `private` members of an `open` class.)
Fixes rdar://problem/40738913.
In some cases the ClosureSpecializer did not invalidate the dominator tree, although it changes the CFG.
This could happen if during analysis critical CFG edges are broken, but at the end no functions are specialized.
To mark when a user of it is known to escape the value. This happens
with materializeForSet arguments which are captured and used in the
write-back. This means we need to keep the context alive until after
the write-back.
Follow-up patches to fully replace the PostponedCleanup hack in SILGen
by a mandatory SIL transformation pass to guarantee the proper lifetime
will use this flag to be more conservative when extending the lifetime.
The problem:
%pa = partial_apply %f(%some_context)
%cvt = convert_escape_to_noescape [not_guaranteed] [escaped] %pa
%ptr = %materialize_for_set(..., %cvt)
... write_back
... // <-- %pa needs to be alive until after write_back
We can just !SILFunction::hasQualifiedOwnership(). Plus as Andy pointed out,
even ignoring the functional aspects, having APIs with names this close can
create confusion.
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.
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.
Add a test to check that closure specialization can handle cases where the full closure type may have unsupported address type arguments (e.g. @in or @out), but the partial_apply has only supported address type arguments, i.e. @inout or @inout_aliasable.
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.
At some point, pass definitions were heavily macro-ized. Pass
descriptive names were added in two places. This is not only redundant
but a source of confusion. You could waste a lot of time grepping for
the wrong string. I removed all the getName() overrides which, at
around 90 passes, was a fairly significant amount of code bloat.
Any pass that we want to be able to invoke by name from a tool
(sil-opt) or pipeline plan *should* have unique type name, enum value,
commend-line string, and name string. I removed a comment about the
various inliner passes that contradicted that.
Side note: We should be consistent with the policy that a pass is
identified by its type. We have a couple passes, LICM and CSE, which
currently violate that convention.
Also, add a third [serializable] state for functions whose bodies we
*can* serialize, but only do so if they're referenced from another
serialized function.
This will be used for bodies synthesized for imported definitions,
such as init(rawValue:), etc, and various thunks, but for now this
change is NFC.
There are now separate functions for function addition and deletion instead of InvalidationKind::Function.
Also, there is a new function for witness/vtable invalidations.
rdar://problem/29311657
SubstitutionList is going to be a more compact representation of
a SubstitutionMap, suitable for inline allocation inside another
object.
For now, it's just a typedef for ArrayRef<Substitution>.
In all cases the DeclCtx field was supposed to be initialized from the
SILLocation of the function, so we can save one pointer per
SILFunction.
There is one test case change where a different (more precise)
diagnostic is being generated after this change.
Pass correct argument indices to the mangler for function specialization. This
has probably always been broken. The demangler doesn't support these manglings
anyway. It doesn't seem to have mattered in practice yet (aside from the
confusion of devs) like me. It will matter once we begin exposing public
specializations.
Separate formal lowered types from SIL types.
The SIL type of an argument will depend on the SIL module's conventions.
The module conventions are determined by the SIL stage and LangOpts.
Almost NFC, but specialized manglings are broken incidentally as a result of
fixes to the way passes handle book-keeping of aruments. The mangler is fixed in
the subsequent commit.
Otherwise, NFC is intended, but quite possible do to rewriting the logic in many
places.
We preserve the current behavior of assuming Any ownership always and use
default arguments to hide this change most of the time. There are asserts now in
the SILBasicBlock::{create,replace,insert}{PHI,Function}Argument to ensure that
the people can only create SILFunctionArguments in entry blocks and
SILPHIArguments in non-entry blocks. This will ensure that the code in tree
maintains the API distinction even if we are not using the full distinction in
between the two.
Once the verifier is finished being upstreamed, I am going to audit the
createPHIArgument cases for the proper ownership. This is b/c I will be able to
use the verifier to properly debug the code. At that point, I will also start
serializing/printing/parsing the ownershipkind of SILPHIArguments, but lets take
things one step at a time and move incrementally.
In the process, I also discovered a CSE bug. I am not sure how it ever worked.
Basically we replace an argument with a new argument type but return the uses of
the old argument to refer to the old argument instead of a new argument.
rdar://29671437
