Improves SwiftSyntax release build speed by 4x.
Limit the size of the sets tracked by inter procedural
AccessedStorageAnalysis. There is a lot of leeway in this limit since
"normal" code doesn't come close to hitting it and SwiftSyntax compile
time isn't noticeably affected until 10x this limit.
This change also avoids reanalyzing function bodies once results have
bottomed out and avoids copying sets in the common case.
Fixes <rdar://problem/46905624> Release build time regression, a lot of time spent on AccessEnforcementOpts::run().
This is just a band aid. The fundamental problem is really:
<rdar://problem/47195282> Recomputing BottomUpIPAnalysis takes most of
the SwiftSyntax compile time.
SwiftSyntax compile time is still at least an order of magnitude
longer than it should be.
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
In the included, test case, the optimization was sinking
releases past is_escaping_closure.
Rewrite the isBarrier logic to be conservative and define the
mayCheckRefCount property in SIL/InstructionUtils. Properties that may
need to be updated when SIL changes belong there.
Note that it is particularly bad behavior if the presence of access
markers in the code cause miscompiles unrelated to access enforcement.
Fixes <rdar://problem/45846920> TestFoundation, TestProcess, closure
argument passed as @noescape to Objective-C has escaped.
This patch augments the infinite recursion checker to not warn if a
branch terminates, but still warns if a branch calls into something with
`@_semantics("arc.programtermination_point")`. This way, calling `fatalError`
doesn't disqualify you for the diagnostic, but calling `exit` does.
This also removes the warning workaround in the standard library, and
annotates the internal _assertionFailure functions as
`programtermination_point`s, so they get this treatment too.
I believe that these were in SILInstruction for historic reasons. This is a
separate API on top of SILInstruction so it makes sense to pull it out into its
own header.
Visual Studio objects to the existing construct with:
swift/SILOptimizer/Analysis/ARCAnalysis.h(233): warning C4927: illegal conversion; more than one user-defined conversion has been implicitly applied
swift/SILOptimizer/Analysis/ARCAnalysis.h(233): note: while calling the constructor 'llvm::Optional<llvm::ArrayRef<swift::SILInstruction *>>::Optiona(T &&)'
with
[
T=llvm::ArrayRef<swift::SILInstruction *>
]
llvm/ADT/Optional.h(143): note: see declaration of 'llvm::Optional<llvm::ArrayRef<swift::SILInstruction *>>::Optional'
This is a verification routine that is only invoked in PassManager
destructors. I am going to use this to ensure that the
PassManagerVerifierAnalysis only runs at such points (even when sil-verify-all
is enabled) since it is too expensive to run otherwise.
NOTE: The default implementation of verifyFull in this commit is a no-op. I
wanted to have verify() be the default implementation of verifyFull(), but I do
not have more time to invest in this and it seems to catch /real/ bugs, albeit
bugs unrelated to pass manager notification verification. Instead I am going to
file an SR for someone to look at it since I need to move on from this work back
to semantic SIL. At least we will not have notification failure issues anymore
and thus a large correctness issue in the compiler has been fixed. Forward
progress!
rdar://42301529
This enables us to have state independent of the liveness of the SILFunction's
that we are tracking.
I also changed the verifier to implement only verifyFull instead of verify to
ensure that when we run with sil-verify-all this only runs at the end of pass
manager pipelines.
rdar://42301529
I also thought I was fixing a performance issue by changing
invalidateFunction not to /insert/ new entries into the map, but
I guess those entries were present in practice. So this is just
a cleanup to make ownership easier.
The invariant is that this analysis should be able to stay in sync with the list
of functions stored in SILModule's function list. If any functions are
added/deleted then analyses can get out of sync with the state of the underlying
SILModule.
Some notes:
1. This is currently disabled by default since there are a bunch of
violations of this in the compiler. I am in the process of fixing violations.
Some examples: the linker and global opt.
2. This is a no-op in non-assert builds.
3. The full verification will only happen when -sil-verify-all is enabled.
Otherwise, we only check that when we delete a function, we had state for the
function.
rdar://42301529
This name makes it clear that the function has not yet been deleted and also
contrasts with the past tense used in the API notifyAddedOrModifiedFunction to
show that said function has already added/modified the function.
The name notifyAddFunction is actively harmful since the pass manager uses this
entrypoint to notify analyses of added *OR* modified functions. It is up to the
caller analysis to distinguish in between these cases.
I am not vouching for the design, just trying to make names match the
current behavior.
I believe that this was just a typo from a long time ago. Calling this parameter
a SILAnalysisTy is actively misleading since as a result it seems to a naive
reading that one should be writing a recursive template:
```
class MyAnalysis : public FunctionAnalysisBase<MyAnalysis> { ... }
```
Instead of passing in the function info of the derived analysis, i.e.:
```
class MyAnalysisFunctionInfo { ... }
class MyAnalysis : public FunctionAnalysisBase<MyAnalysisFunctionInfo> { ... }
```
I also added some documentation to that affect onto FunctionAnalysisBase.
Generally in the SIL/SILOptimizer libraries we have been putting kinds in the
swift namespace, not a nested scope in a type in swift (see ValueKind as an
example of this).
Some parts were using the more modern style that we are using in the optimizer
that involves having ivars and local variables be camelCase instead of
CamelCase.
The current dumping format consists of 1 row of information per function. This
will become unweildy to write patterns for when I add additional state to
FunctionInfo.
Instead, this commit converts the dumping format of the caller analysis into a
multi line yaml format. This yaml format looks as follows:
---
calleeName: closure1
hasCaller: false
minPartialAppliedArgs: 1
partialAppliers:
- partial_apply_one_arg
- partial_apply_two_args1
fullAppliers:
...
This can easily expand over time as we expand the queries that caller analysis
can answer.
As an additional advantage, there are definitely yaml parsers that can handle
multiple yaml documents in sequence in a stream. This means that by running via
sil-opt the caller-analysis-printer pass, one now will get a yaml description of
the caller analysis state, perfect and ready for analysis.
This converts a DenseMap to a SmallMapVector and a SetVector to a
SmallSetVector. Both of these create large malloced data structures by
default. This really makes no sense when there are many functions that don't use
a partial apply or many applies.
Additionally, by changing the DenseMap to a MapVector container, this commit is
eliminating a potential source of non-determinism in the compiler since often
times we are iterating over the DenseMap to produce the results. Today all of
the usages of the DenseMap in this way are safe, but to defensively future proof
this analysis, it makes sense to use a MapVector here.
I am tuning a new argument explosion heuristic to reduce code-size. One part of
the heuristic I am playing with is the part of the algorithm that attempts to
figure out if we could eliminate additonal arguments after performing
owned->guaranteed an additional release when we run FSO a second time. Today we
do this unconditionally. I am trying to do it in a more conservative way where
we only do it if we know that we aren't going to increase the number of
arguments too much.
rdar://41146023
This is particularly egrigious since we are only /reading/ from the DenseSet. So
we are basically mallocing/copying a DenseSet just to read from it... I don't
think I need to say more.
rdar://41146023
