Note the purpose here is to make it simpler to add more subcommands,
without adding a lot of things to Driver.
This creates a copy of CodegenOptions, but it was double-registered at
present which felt odd. It's also fairly small right now. If this
becomes an issue, maybe we can look into using optional for delayed
initialization, or just go back to straight sharing.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Instead of the `call` instruction having a block with one argument per
explicit argument, preceded optionally by `self` and followed optionally
by a return slot, change the `call` to store only the *runtime*
arguments. Store an index on the runtime parameters to make it easier to
determine the correspondence between arguments and parameters in a call.
Compile-time parameters, whether implicit or explicit, are no longer
included in the call argument list. Instead, they're tracked only in the
`specific_id` on the callee.
For calls to generic classes and generic interfaces, it no longer makes
sense to form a `call` instruction, given that the entirety of the
result is determined by the `specific_id`, which is now formed when
checking the call. Instead, the `call` instruction now only models
function calls, and not calls to other kinds of parameterized entity
names, and we create a `class_type` or `interface_type` instead of a
`call` instruction to model these kinds of calls. Notionally the model
here is that we're following the #3720 approach for calls, but for now
we inline the `Call.Op` function when forming SemIR.
We now also track the enclosing specific for a generic class or generic
interface that appears within an enclosing generic. This is necessary in
order for deduction of the inner generic parameters to not get confused
by the outer generic parameters being absent.
In order to not regress diagnostics, the template argument deduction
mechanism has been extended to specify the name of the parameter we're
deducing against when possible, and call arity mismatch errors are now
diagnosed before performing deduction rather than afterwards.
I'm separating the options out so that it's easier to review. They
include a lot of boilerplate text that I think won't change much, and
makes it harder to review changes.
To explain filename differences, whereas `CodegenOptions` is shared (by
link and compile), `LinkOptions` and `CompileOptions` are
subcommand-specific. I'm planning to separate out the subcommands, so
I'm putting those in respective subcommand files. I'm still going to try
to use the `.h` to declare the interface, `.cpp` for bigger
implementation details (for better or worse, including comments on
options).
I'm also moving out corresponding Driver members to help shrink deltas
when refactoring. That is, the bodies aren't changing here, but a
refactoring of commands will make some changes. By moving the code to
different files now, it should be easier to identify what's changing
later.
---------
Co-authored-by: Geoff Romer <gromer@google.com>
This will catch some cases of bugs in the name mangling logic - if
within a single file we incorrectly mangle two distinct entities to the
same name, llvm::Function will assign a new name to the second copy
showing one of the two entities should have a distinct name/is missing
something in their mangling.
build_file_path works with bazel but I'd missed it's
[deprecated](https://bazel.build/rules/lib/builtins/ctx#build_file_path).
Relative path handling also could use some improvements.
Both of these are really to support non-standard environments,
essentially.
This switches `DCHECK` and `FATAL` as well.
The goal is to reduce the code size impact of these assertions so that
we can keep more of them enabled. Currently, the largest cost I see from
`CHECK` is not the actual check or the cold code itself, but actually
the failure to inline trivial functions due to the presence of the cold
code. This means that our goal isn't to reduce apparent code size in the
final binary but the LLVM IR cost assessed for these routines in the
inliner, which closely correlates with code size but is a bit different.
As discussed in #4283, experimentation shows that a single function call
with a minimal number of arguments is the lowest cost model for these.
This is easily achieved with a format-string API that internally uses
`llvm::formatv`. This PR is essentially the `CHECK` version of #4283.
However, the check macros are substantially harder to make work with
both format strings and streaming because they also take a condition.
Also, unexpectedly, I was very successful at devising a regular
expression based automated rewrite from the streaming to the format
string form with only low 10s of manual fixes. This includes compacting
strings broken up across lines, etc. Given how well that went, I've
prepared this PR which just directly switches to the format string API
and migrate everything to use it.
One nice side-effect is that the format string approach ends up greatly
simplifying the implementation here as well.
This is ... *shockingly* effective. Parsing speeds up by more than 3%
with just this change. And checking speeds up by **8%** with this change
alone:
```
BM_CompileAPIFileDenseDecls<Phase::Parse>/256 86.3µs ± 1% 82.9µs ± 1% -3.94% (p=0.000 n=17+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/1024 431µs ± 1% 415µs ± 1% -3.76% (p=0.000 n=18+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/4096 1.77ms ± 1% 1.71ms ± 1% -3.18% (p=0.000 n=18+19)
BM_CompileAPIFileDenseDecls<Phase::Parse>/16384 7.44ms ± 1% 7.17ms ± 2% -3.56% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Parse>/65536 30.7ms ± 1% 29.7ms ± 1% -3.15% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Parse>/262144 131ms ± 1% 127ms ± 1% -2.81% (p=0.000 n=18+18)
BM_CompileAPIFileDenseDecls<Phase::Check>/256 878µs ± 2% 800µs ± 1% -8.91% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/1024 1.88ms ± 2% 1.72ms ± 1% -8.56% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/4096 5.78ms ± 2% 5.28ms ± 1% -8.70% (p=0.000 n=20+18)
BM_CompileAPIFileDenseDecls<Phase::Check>/16384 21.9ms ± 1% 20.1ms ± 1% -8.02% (p=0.000 n=18+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/65536 90.4ms ± 2% 83.1ms ± 1% -8.04% (p=0.000 n=19+20)
BM_CompileAPIFileDenseDecls<Phase::Check>/262144 381ms ± 2% 352ms ± 1% -7.79% (p=0.000 n=19+19)
```
---------
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
I want to split commands out so that we don't keep piling onto driver
(particularly as I'm eyeing clang-related commands). This extracts out
the DriverEnv so that it can be easily shared, with the CompilationUnit
as an example.
CARBON_VLOG_TO is to remove the vlog_stream_ requirement of CARBON_VLOG.
The goal is to replace our stream operator APIs with format string APIs
that can be made to have much less impact on inlining and other
optimizations of the performance critical path through the code.
Several experiments show that the most compact representation we can
arrange for is one that calls an uninlined function and passes a minimal
number of arguments to it. It doesn't help to do any work to minimize
the arguments such as building a lambda -- the cost of extra code to
merge the arguments is likely to outweigh the benefit.
Initial experiments showed that switching a hot but uninlined function
to this new API enabled inlining and the subsequent performance
improvement.
This also adds a 'TemplateString` utility that allows using a string
literal as a template parameter. This is useful to remove the format
string itself from the arguments passed to the function by passing it as
a template argument instead.
Currently, support is left in place for both APIs because with
`CARBON_VLOG` we can detect whether or not any message was provided
expecting a format string. This should allow incrementally migrating
code to this API. I've added some test coverage in this PR, but I'll
separate out any switching of parts of the codebase over.
The goal is to eventually replace all the usages and remove the
streaming support entirely.
This PR doesn't update `CARBON_CHECK` in the same way because it is
substantially more complex to switch. I have a few experimental PRs
looking at that and will discuss how best to approach this with the
specific challenges check presents separately. But the goal is for all
of the macro-based output APIs to move to format strings rather than
streams.
---------
Co-authored-by: josh11b <15258583+josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Includes support for the `impls`, `=`, and `==` requirement operators to
the right of a `where`, but `and` to allow multiple requirements is
still a TODO.
---------
Co-authored-by: Josh L <josh11b@users.noreply.github.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
Just a small factoring thing. The `if` logic is sizeable, and there's no
need for it to be in handle_expr (we do also have handle_brace_expr,
handle_index_expr, etc)
Applies #4278 TEST_NAME substitution to tests. Note I've tried to
structure commits as:
1. Do all the replacements.
2. autoupdate (nothing else) -- this shows incorrect updates.
3. Fix up manually, including autoupdates to get back to original
output.
This removes the directory crawl because bazel doesn't remove files from
execroot when the rule generating them would no longer generate them.
Fixes#4288
This opens the door for replacing all `library ...` lines in toolchain
test files with `library "[[@TEST_NAME]]";`. That's technically more
typing in a lot of cases, but OTOH means we can just do some copy-paste
boilerplate and stop carefully writing library names.
Also cleans up the test setup, because it's getting messy. I'm trying to
make it easier to see the divisions of tests and the output associated
with them. StringSwitch looked like a way to do this, with a few edits
to make it work nicely.
I guess this technically would also allow code to pass check that hasn't
before, and that isn't covered by tests (since it's masked by other
failures in the tests that already test this functionality) - should I
add another test/add some code to a valid test case?
Also, this'll miscompile in lowering, since there's no support there yet
- should I do anything about that to make lowering fail in some way? Or
is it acceptable that some things just silently mis-lower? (I could add
a currently-miscompiling test case too, to demonstrate this? (not sure
if the autogenerated tests leave space for comments that would explain
that the currently-tested behavior is incorrect?))
Is the addition to EntityWithParamsBase suitable? of course not all
functions can be virtual, so it's a wasted bit at the moment for all
those cases (though it's free, since it's bitpacked - but as we want to
add more bits in there it might not be a scalable solution)?
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
This seems to be enough to avoid naming collisions for functions in any
of the current test cases (verified by asserting that the name of the
`llvm::Function` matches the name passed to create it - not triggering
LLVM's numbering that happens when names collide)
It currently implements mangling for namespace scopes, class scopes, and
impls.
Nothing generic is mangled yet - haven't looked at how that works,
though evidently it's not covered by existing testing, I guess.
Follow-up change will document the current mangling algorithm in
`toolchain/docs/lower.md`
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Carbon Infra Bot <carbon-external-infra@google.com>
I've repeatedly struggled with very obscure build errors that turned out
to be caused by a newly-introduced node kind getting inappropriately
defaulted to `Id::Kind::Invalid`. Dropping the default will turn those
mistakes into much more straightforward "missing case in switch" errors.
This makes each token info consist of 8 bytes of data:
- 1 byte of the kind
- 1 bit for whitespace tracking
- 23 bits of payload
- 32 bits for byte offset in the file
This builds directly on representing the location of the token as
a single 32-bit offset, now compressing the rest of the data into
a single 32-bit bitfield.
This adds some implementation limits: we can no longer lex more than
2^23 tokens in a single source file. Nor can we have more than 2^23
string literals, integer literals, real literals, or identifiers. Only
the first of these is even close to an issue, and even then seems
unlikely to ever be a problem in practice.
The memory efficiency here is great and the motivating goal. But to make
this work well, we also need to streamline how we create the tokens.
Otherwise, all the bit fiddling can end up erasing our gains. This PR
adds a number of APIs to manage creating and accessing the now
significantly more complex storage of token infos to try and help with
this.
One big change required to simplify the writes here is to switch from
computing whether a token has trailing space after-the-fact to
pre-computing whether a token will have leading space. That lets us have
the leading space information available immediately when forming the
token, and avoids doing a single bit flip afterward.
Another change that helps with this representation is to minimize the
updating of groups after-the-fact. The code now tries to set the opening
index directly when creating the closing token and only updates the
opening group afterward. Because of the bit packing, this is a reduction
of 0.5% of dynamic instructions in the compile benchmark, and has
dramatic improvements for the grouping symbol focused benchmarks.
All combined, this is a significant improvement on the lexer-focused
benchmarks despite the added complexity, and a significant win on our
compile time benchmarks due to both the lexer improvements and
downstream memory density improvements: 5-12% reduction in lex time,
growing larger as files get larger. About a 4.5% reduction in parse
time, and even a 1-2% reduction in total check time. =D
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Geoff Romer <gromer@google.com>
Add these interfaces to the core library. For now, they're two separate
interfaces because we don't yet support one interface extending another.
This collapses a lot of the layering in check: for example, the call
building logic depends on implicit conversions, conversions now depend
on the overloaded operator machinery, and that machinery depends on
building calls.
In passing, improve the diagnostics for failing to find a name required
from the prelude. Also convert all the transitively-called code from
`NodeId` to `LocId` given the latter is what the conversion machinery
has available.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Conversion can trigger new entities to be imported, which can invalidate
the reference it holds to an EntityWithParamBase. Instead of holding
such a reference, pull the information we need out of the entity early
and only pass that into ConvertCallArgs.
I couldn't find a good standalone way to test this, but this fixes the
test failure we otherwise see on MacOS after #4209, so it will be tested
once that PR lands.
From the driver's perspective, `FindPreludeFiles` is closely tied to
`compile`. This makes it difficult to refactor commands without
affecting the test dependencies on `FindPreludeFiles`. `InstallPaths`
seems like a decent home since it is responsible for the install
structure.
I'm switching to an `Error` return to allow callers to choose how to
handle it (e.g., in file tests, we typically don't want the direct error
stream).
Mainly, changes the default from -1 to 0 in DiagnosticLoc, still trying
to keep reusing that. Nothing except for the lowered output is affected,
so I think this is fine.
Also, have lowering consistently call GetDiagnosticLoc.
Pulls in one of the CHECKs suggested from #4251
Co-authored-by: David Blaikie <dblaikie@gmail.com>
First, this replaces the separate line index and column index in the
token information with a single 32-bit byte offset of the token. This is
then used to compute line and column numbers with a binary search of the
line structure and then using that to compute the column within the
line. In practice, this is _much_ more efficient:
- Smaller token data structure. This will hopefully combine with a
subsequent optimization PR that shrinks the token data structure still
further.
- Fewer stores to form each token's information in the tight hot loop of
the lexer.
- Less state to maintain while lexing, fewer computations while lexing.
We only have to search to build the line and column information off the
hot lexing path, and so this ends up being a significant win and shrinks
some of the more significant data structures.
Second, this shrinks the line start to a 32-bit integer and removes the
line length. Our source buffer already ensures we only have 2 GiB of
source with a nice diagnostic. I've just added a check to help document
this in the lexer. The line length can be avoided in all of the cases it
was being used, largely by looking at the next line's start and working
from there. This also precipitated cleaning up some code that dated from
when lines were only built during lexing rather than being pre-built,
which resulted in nice simplifications.
With this PR, I think it makes sense to re-name a bunch of methods on
`TokenizedBuffer`, but to an extent that was already needed as these
methods somewhat predate the more pervasive style conventions. I avoided
that here to keep this PR focused on the implementation change, I'll
create a subsequent PR to update the API to both better nomenclature and
remove deviations from our conventions.
There may also be a way to de-duplicate the binary search in the
diagnostic location conversion and the main line accessor binary search,
but it wasn't obvious to me that it would be a net savings, so left it
alone for now.
The performance impact of this varies quite a bit...
The lexer's benchmark improves pretty consistent across the board on
both x86 and Arm. For x86, where I have nice comparison tools, it
appears 3% to 20% faster depending on the specific pattern. For Arm
server CPUs at least it seems a much smaller but still an improvement.
The overall compilation benchmarks however don't improve much with these
changes alone on x86. Significant reduction in instruction count
required for lexing, but the overall performance is bottlenecked
elsewhere in the overall compilation it seems. However, on Arm, despite
the more modest gains in special cases of lexing, this shows fairly
consistent 1-2% improvements in overall lexing performance on our
compilation benchmark. And the expectaiton is these improvements will
compound with subsequent work to further compact our representation.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Establish a process for getting commit access. We will:
- Grant access based on a developer's commit history.
- Someone with commit access should nominate, and a contributor may ask.
- A lead will approve nominations. Only one lead is needed.
- Remove commit access once someone is idle for 6 months.
- "Idle" means no significant project activity on any of GitHub,
Discord,
or in meetings.
- Access removed due to being idle will be restored on request.
---------
Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
Remove `ReusingLoc` and add enforcement that even for imported
locations, the kind of the parse node for an instruction matches the
kind specified in the instruction definition.
Change the node kind for a few instructions to `NodeId`:
- A couple of instructions had a typed node but could be created
implicitly with any node as part of a builtin implicit conversion. This
happened for `AddrOf`, `ArrayIndex`, and `Deref`.
- A bunch of instructions had `InvalidNodeId` as their associated parse
node kind but were actually always created with a location.
---------
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Chandler Carruth
josh11bandJosh L
Richard Smith
Brymer Meneses