Deserialization is calling AccessMarkerElimination repeatedly on the
same function.
The bug was introduced here:
commit 872bf40e17
Date: Mon Aug 13 10:24:20 2018
[sil-optimizer] Centralize how we send out serialization notifications.
Where the code that uniques the deserialization callbacks was simply
removed!
As a result, this pass was being invoked a number of times equal to
the number of functions in the module *multiplied* by the number of
functions being deserialized.
Fixes rdar://117141871 (Building spends most of its time in
AccessMarkerElimination)
When an instruction is "deleted" from the SIL, it is put into the SILModule::scheduledForDeletion list.
The instructions in this list are eventually deleted for real in SILModule::flushDeletedInsts(), which is called by the pass manager after each pass run.
In other words: instruction deletion is deferred to the end of a pass.
This avoids dangling instruction pointers within the run of a pass and in analysis caches.
Note that the analysis invalidation mechanism ensures that analysis caches are invalidated before flushDeletedInsts().
This silences the instances of the warning from Visual Studio about not all
codepaths returning a value. This makes the output more readable and less
likely to lose useful warnings. NFC.
Previously SILModule contained two different pathways for the deserializer to
send notifications that it had created functions:
1. A list of function pointers that were called when a function's body was
deserialized. This was added recently so that access enforcement elimination is
run on newly deserialized SIL code if we have already eliminated access
enforcement from the module.
2. SILModule::SerializationCallback. This is an implementation of the full
callback interface and is used by the SILModule to update linkage and other
sorts of book keeping.
To fix the pass manager notification infrastructure, I need to be able to send
notifications to a SILPassManager when deserializing. I also need to be able to
eliminate these callbacks when a SILPassManager is destroyed. These requirements
are incompatible with the current two implementations since: (2) is an
implementation detail of SILModule and (1) only notifies on function bodies
being deserialized instead of the creation of new declarations (what the caller
analysis wants).
Rather than adding a third group of callbacks, this commit refactors the
infrastructure in such a way that all of these use cases can use one
implementation. This is done by:
1. Lifting the interface of SerializedSILLoader::Callback into a base
notification protocol for deserialization called
DeserializationNotificationHandlerBase and its base no-op implementation into an
implementation of the aforementioned protocol:
DeserializationNotificationHandler.
2. Changing SILModule::SerializationCallback to implement
DeserializationNotificationHandler.
3. Creating a class called FunctionBodyDeserializationNotificationHandler that
takes in a function pointer and uses that to just override the
didDeserializeFunctionBody. This eliminates the need for the specific function
body deserialization list.
4. Replacing the state associated with the two other pathways with a single
DeserializationNotificationHandlerSet class that contains a set of
DeserializationNotificationHandler and chains notifications to them. This set
implements DeserializationNotificationHandlerBase so we know that its
implementation will always be in sync with DeserializationNotificationHandler.
rdar://42301529
Now that SILGen change adds Unsafe access markers to addressors and
materializeForSet, we can use that as a sentinel to enable strict
verification everywhere.
* Teach findAccessedStorage about global addressors.
AccessedStorage now properly represents access to global variables, even if they
haven't been fully optimized down to global_addr instructions.
This is essential for optimizing dynamic exclusivity checks. As a
verified SIL property, all access to globals and class properties
needs to be identifiable.
* Add stronger SILVerifier support for formal access.
Ensure that all formal access follows recognizable patterns
at all points in the SIL pipeline.
This is important to run acccess enforcement optimization late in the pipeline.
Modify IRGen to emit builtin access markers with an error flag in
Swift 3 mode.
KeyPath enforcement is required by user code in Swift 4+ mode, but is
implemented within the standard library. A [builtin] flag marks the
special case for access generated by Builtins so that they are
always enforced as an error regardless of the language mode.
This is necessary for Swift 4.2 because the standard library continues
to build in Swift 3 mode. Once the standard library build migrates,
this is all irrelevant.
This does not actually affect existing Swift 3 code, since the KeyPath
feature wasn't introduced until Swift 4.
<rdar://problem/40115738> [Exclusivity] Enforce Keypath access as an error, not a warning in 4.2.
Use begin_unpaired_access [no_nested_conflict] for
Builtin.performInstantaneousReadAccess. This can't be optimized away
and is the proper marker to use when the access scope is unknown.
Drop the requirement that
_semantics("optimize.sil.preserve_exclusivity") be @inline(never). We
actually want theses inlined into user code. Verify that the
@_semantic functions are not inlined or otherwise tampered with prior
to serialization.
Make *no* change to propagate @inline(__always) into LLVM. This no longer has
any relationship to this PR and can be investigated seperately.
This is a special @_semantics attribute that preserves exclusivity even if we
are eliminating exclusivity in other parts of a module in release mode. This is
done by:
1. Teaching the Access Marker Elimination pass to skip any function with the
semantics tag.
2. Requiring all functions with the semantics tag to be noinline. This ensures
that the SIL level inliner will not inline these functions into any callers
without the protection of the semantics tag. This is enforced in IRGenPrepare
and ensures that our access markers will live to IRGen time.
3. In IRGenPrepare, we convert these functions from noinline to always
inline. After IRGen this then allows for the LLVM inliner to inline these
trivial functions that just perform the exclusivity checks ensuring that we do
not have extra calls in the fast path.
This ensures that we can fix the keypaths exclusivity issue without having to
enable exclusivity across the entire stdlib and deal with any of the potential
performance issues therein.
rdar://39335800
-enforce-exclusivity=checked now does what it says. It emits checks at -O, not
just -Onone.
This does not change the default.
-Onone default: -enforce-exclusivity=checked
-O default: -enforce-exclusivity=unchecked
Until other bugs are fixed, this is still required in the short term after
mandatory inlining of debug libraries into an optimized executable.
Also, enable rerunning access marker elimination on deserialized functions,
because theoretically, the same problem could occur if we emit an external
function without inlining it.
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.
In raw SIL, access markers are unconditionally retained. In canonical SIL,
markers are still removed prior to optimization.
A new flag, -sil-optimized-access-markers, allows testing access markers in
optimized builds, but it is not yet fully supported.
AccessMarkerElimination now registers a callback so that any subsequently
deserialized function bodies will have access markers stripped for optimization.
rdar:31908496 Assertion failed: (isa<X>(Val) && "cast<Ty>() argument of
incompatible type!") in SILPerformanceInliner
LLVM's ilist::erase is actually correct. It just implements a nonstandard remove
method that modifies it's iterator argument.
I forgot to add "continue" statements when fixing the iterator-invalidation problem.
I reversed this loop's direction over the instruction list and forgot to change
the order of erasing an instruction with respect to advancing the iterator.
Thankfully ASAN is far smarter than I.
Converting between forward/reverse iterators makes the loop unreadable.
Add an iterator return value to BasicBlock::erase(SILInstruction*).
