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
A recent SILCloner rewrite removed a special case hack for single
basic block callee functions:
commit c6865c0dff
Merge: 76e6c4157e9e440d13a6
Author: Andrew Trick <atrick@apple.com>
Date: Thu Oct 11 14:23:32 2018
Merge pull request #19786 from atrick/silcloner-cleanup
SILCloner and SILInliner rewrite.
Instead, the new inliner simply merges trivial unconditional branches
after inlining the return block. This way, the CFG is always in
canonical state after inlining. This is more robust, and avoids
interfering with subsequent SIL passes when non-single-block callees
are inlined.
The problem is that inlining a series of calls within a large block
could result in interleaved block splitting and merging operations,
which is quadratic in the block size. This showed up when inlining the
tens of thousands of array subscript calls emitted for a large array
initialization.
The first half of the fix is to simply defer block merging until all
calls are inlined. We can't expect SimplifyCFG to run immediately
after inlining, nor would we want to do that, *especially* for
mandatory inlining. This fix instead exposes block merging as a
trivial utility.
Note: by eliminating some unconditional branches, this change could
reduce the number of debug locations emitted. This does not
fundamentally change any debug information guarantee, and I was unable
to observe any behavior difference in the debugger.
Avoid emitting unnecessary basic block for cleanup chains. This is a
general approach that handles all cases while simplifying SILGen
emission and keeping the CFG in a valid state during SILGen.
For example: a switch_enum for a single-case enum. It has only a single successor edge, but needs to be split as well (if the destination block has multiple predecessors).
Fixes a crash in SimplifyCFG.
rdar://problem/44675677
SIL passes were violating the existing invariant on non-cond-br
critical edges in several places. I fixed the places that I could
find. Wherever there was a post-pass to "clean up" critical edges, I
replaced it with a a call to verification that the critical edges
aren't broken in the first place.
We still need to eliminate critical edges entirely before enabling
ownership SIL.
Currently, the ownership verifier assumes that all cond_br with critical edges
do not have non-trivial arguments. This utility method fixes any such issues and
should be run /after/ any passes that mess with the CFG in ownership SIL if this
could come up.
Chopping this off of a larger commit.
rdar://31521023
This helper utility given a set of uses/defs determines the set of blocks that
together with the user blocks jointly post-dominate the def blocks. In a sense
this is completing the providing set of user blocks to a joint-post dominating
set. This completion is not unique.
I am adding this method because often times when working with Ownership SIL, one
needs to hoist/sink operations and then compensate to ensure that ownership
properties are still preserved.
I think with a little bit of work I can get the ownership model eliminator to
use this routine (since the condition for ownership to be correct on an @owned
value is an empty joint-post dominating completion. I wrote it b/c I need it in
pred-memopts and I expect it to be a general useful routine for Ownership SIL.
rdar://31521023
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.
Most of this involved sprinkling ValueOwnershipKind::Owned in many places. In
some of these places, I am sure I was too cavalier and I expect some of them to
be trivial. The verifier will help me to track those down.
On the other hand, I do expect there to be some places where we are willing to
accept guaranteed+trivial or owned+trivial. In those cases, I am going to
provide an aggregate ValueOwnershipKind that will then tell SILArgument that it
should disambiguate using the type. This will eliminate the ackwardness from
such code.
I am going to use a verifier to fix such cases.
This commit also begins the serialization of ValueOwnershipKind of arguments,
but does not implement parsing of value ownership kinds. That and undef are the
last places that we still use ValueOwnershipKind::Any.
rdar://29791263
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
This was already done for getSuccessorBlocks() to distinguish getting successor
blocks from getting the full list of SILSuccessors via getSuccessors(). This
commit just makes all of the successor/predecessor code follow that naming
convention.
Some examples:
getSingleSuccessor() => getSingleSuccessorBlock().
isSuccessor() => isSuccessorBlock().
getPreds() => getPredecessorBlocks().
Really, IMO, we should consider renaming SILSuccessor to a more verbose name so
that it is clear that it is more of an internal detail of SILBasicBlock's
implementation rather than something that one should consider as apart of one's
mental model of the IR when one really wants to be thinking about predecessor
and successor blocks. But that is not what this commit is trying to change, it
is just trying to eliminate a bit of technical debt by making the naming
conventions here consistent.
Before this commit all code relating to handling arguments in SILBasicBlock had
somewhere in the name BB. This is redundant given that the class's name is
already SILBasicBlock. This commit drops those names.
Some examples:
getBBArg() => getArgument()
BBArgList => ArgumentList
bbarg_begin() => args_begin()
This eliminates all inline creation of SILBasicBlock via placement new.
There are a few reasons to do this:
1. A SILBasicBlock is always created with a parent function. This commit
formalizes this into the SILBasicBlock API by only allowing for SILFunctions to
create SILBasicBlocks. This is implemented via the type system by making all
SILBasicBlock constructors private. Since SILFunction is a friend of
SILBasicBlock, SILFunction can still create a SILBasicBlock without issue.
2. Since all SILBasicBlocks will be created in only a few functions, it becomes
very easy to determine using instruments the amount of memory being allocated
for SILBasicBlocks by simply inverting the call tree in Allocations.
With LTO+PGO, normal inlining can occur if profitable so there shouldn't be
overhead that we care about in shipping compilers.
replaced by retain release code motion. This code has been disabled for sometime now.
This should bring the retain release code motion into a close. The retain release
code motion pipeline looks like this. There could be some minor cleanups after this though.
1. We perform a global data flow for retain release code motion in RRCM (RetainReleaseCodeMotion)
2. We perform a local form of retain release code motion in SILCodeMotion. This is more
for cases which can not be handled in RRCM. e.g. sinking into a switch is more efficiently
done in a local form, the retain is not needed on the None block. Release on SILArgument needs
to be split to incoming values, this can not be done in RRCM and other cases.
3. We do not perform code motion in ASO, only elimination which are very important.
Some modifications to test cases, they look different, but functionally the same.
RRCM has this canonicalization effect, i.e. it uses the rc root, instead of
the SSA value the retain/release is currently using. As a result some test cases need
to be modified.
I also removed some test cases that do not make sense anymore and lot of duplicate test
cases between earlycodemotion.sil and latecodemotion.sil. These tests cases only have retains
and should be used to test early code motion.
As there are no instructions left which produce multiple result values, this is a NFC regarding the generated SIL and generated code.
Although this commit is large, most changes are straightforward adoptions to the changes in the ValueBase and SILValue classes.
In all of the cases where this is being used, we already immediately perform an
unreachable if we find a TermKind::Invalid. So simplify the code and move it
into the conversion switch itself.
This exposed the first interesting bug found by using TermKind, in DCE we were
not properly handling switch_enum_addr and checked_cast_addr_br.
SR-335
rdar://23980060
(libraries now)
It has been generally agreed that we need to do this reorg, and now
seems like the perfect time. Some major pass reorganization is in the
works.
This does not have to be the final word on the matter. The consensus
among those working on the code is that it's much better than what we
had and a better starting point for future bike shedding.
Note that the previous organization was designed to allow separate
analysis and optimization libraries. It turns out this is an
artificial distinction and not an important goal.
Erik Eckstein
Adrian Prantl
Andrew Trick
Gogul Balakrishnan
Michael Gottesman
John McCall
Arnold Schwaighofer
Doug Gregor
Slava Pestov
Joe Shajrawi
Roman Levenstein
practicalswift
Michael Gottesman
Xin Tong
Zach Panzarino