Improve the forcing/promotion functions in `DepKindVTable`
This is a bundle of changes to the two function pointers in `DepKindVTable` that are responsible for forcing dep nodes, or promoting disk-cached values from the previous session into memory.
The perf improvements to incr-unchanged and incr-patched are likely from skipping more of the “promotion” plumbing for queries that never cache to disk.
- `force_from_dep_node` → `force_from_dep_node_fn`
- `try_load_from_disk_cache` → `promote_from_disk_fn`
This commit also inlines the wrapper function around `promote_from_disk_fn`.
Clean up `QueryVTable::hash_result` into `hash_value_fn`
This PR:
- Renames the query vtable field `hash_result` to `hash_value_fn`
- Removes the unhelpful `HashResult` type alias, which was hiding an important layer of `Option`
- Replaces the cryptic `hash_result!` helper macro with a more straightforward `if_no_hash!` helper, in line with other modifier-checking macros
- Renames a few identifiers to refer to a query's return value as `value`
There should be no change to compiler behaviour.
r? nnethercote (or compiler)
Remove query function arrays
`define_queries!` produces four arrays of function pointers, which other functions iterate over. These aren't actually necessary.
r? @petrochenkov
This commit:
- Renames the query vtable field `hash_result` to `hash_value_fn`
- Removes the unhelpful `HashResult` type alias, which was hiding an
important layer of `Option`
- Replaces the cryptic `hash_result!` helper macro with a more straightforward
`if_no_hash!` helper, in line with other modifier-checking macros
- Renames a few identifiers to refer to a query's return value as `value`
Don't allow subdiagnostic to use variables from their parent
Tangentially related to https://github.com/rust-lang/rust/issues/151366
This is PR 1/2 for structured diagnostics, will do the unstructured ones next. After the second PR I will be able to remove some code that should compensate for this PR being positive.
Regardless of this PR having a positive diff, I feel that subdiagnostics being able to use variables from their parent is very confusing, so this is for the better,.
r? @jdonszelmann
And also `alloc_self_profile_query_strings` for each query. This is done
by generating the top-level `alloc_self_profile_query_strings` and
having it do things more directly.
Currently `gather_active_jobs` and `gather_active_jobs_inner` do some of
the work each. This commit changes things so that `gather_active_jobs`
is just a thin wrapper around `gather_active_jobs_inner`. This paves the
way for removing `gather_active_jobs` in the next commit.
Clean up some code related to `QueryVTable::execute_query_fn`
This PR is an assortment of small cleanups to code that interacts with `execute_query_fn` in the query vtable.
I also experimented with trying to replace the macro-generated `__rust_end_short_backtrace` functions with a single shared generic function, but I couldn't manage to avoid breaking short backtraces, so I left a note behind to document my attempt.
r? nnethercote (or compiler)
`QuerySystem` has two function pointers: `encode_query_results` and
`try_mark_green`. These exist so that `rustc_middle` can call functions
from upstream crates.
But we have a more general mechanism for that: hooks. So this commit
converts these two cases into hooks.
`QueryEngine` is a struct with one function pointer per query. This
commit removes it by moving each function pointer into the relevant
query's vtable, which is already a collection of function pointers for
that query. This makes things simpler.
Currently type variables that impl `QueryCache` are called either `C` or
`Cache`. I find the former clearer because single char type variables
names are very common and longer type variable names are easy to mistake
for type or trait names -- e.g. I find the trait bound `C: QueryCache`
much easier and faster to read than `Cache: QueryCache`.
`QuerySystem` has these fields:
```
pub states: QueryStates<'tcx>,
pub caches: QueryCaches<'tcx>,
pub query_vtables: PerQueryVTables<'tcx>,
```
Each one has an entry for each query.
Some methods that take a query-specific `QueryVTable` need to access the
corresponding query-specific states and/or caches. So `QueryVTable`
stores the *byte offset* to the relevant fields within `states` and
`caches`, and uses that to (with `unsafe`) access the fields. This is
bizarre, and I hope it made sense in the past when the code was
structured differently.
This commit moves `QueryState` and `QueryCache` inside `QueryVTable`. As
a result, the query-specific states and/or caches are directly
accessible, and no unsafe offset computations are required. Much simpler
and more normal. Also, `QueryCaches` and `QueryStates` are no longer
needed, which makes `define_callbacks!` a bit simpler.
The commit also rename the fields and their getters to `states` and
`caches`.
Clarify how "ensure" queries check whether they can skip execution
The current `check_cache: bool` field in QueryMode is confusing for a few reasons:
- It actually refers to checking the *disk cache*, not the in-memory cache.
- It gives no indication of what condition is being checked, or why.
- It obscures the link between `tcx.ensure_ok()` and `tcx.ensure_done()`, and the actual check.
This PR replaces that field with an `EnsureMode` enum that distinguishes between ensure-ok and ensure-done, and leaves the actual check as an implementation detail of the ensure-done mode.
This PR also renames `ensure_must_run` → `check_if_ensure_can_skip_execution`, and uses a return struct to more clearly indicate how this helper function gives permission for its caller to skip execution of a query that would otherwise be executed. This also inverts the sense of the returned boolean, which was previously “must run” but is now ”skip execution”.
r? nnethercote (or compiler)
It's now `query_vtable` because its return type changed. And thanks to
the previous commit it can be manually inlined in several places. (The
only remaining calls to it are in `make_dep_kind_vtable_for_query`,
which are more challenging to remove.)
`SemiDynamicQueryDispatcher` is just a `QueryVTable` wrapper with an
additional `const FLAGS: QueryFlags` generic parameter that contains
three booleans. This arrangement exists as a performance optimization.
But the performance effects are very small and it adds quite a bit of
complexity to an already overly-complex part of the codebase. If it
didn't exist and somebody proposed adding it and asked me to review, I
almost certainly wouldn't approve it.
This commit removes it. The three booleans in `QueryFlags` are moved
into `QueryVTable` The non-trivial methods of
`SemiDynamicQueryDispatcher` become methods of `QueryVTable`.
Within `define_callbacks!`:
- Use `Key<'tcx>` where possible instead of `$($K)*`.
- Use `Value<'tcx>` where possible instead of `$V`.
In the other `define_*!` macros:
- Use `$K:ty` where possible instead of `$($K:tt)*`.
- Add comments about unused `$K` and `$V` cases.
- Add `()` key types to the special nodes in the `define_dep_nodes!`
invocation for consistency with normal queries.
There is a bunch of plumbing to record the string `"foo"` for each
variant `DepKind::foo`. But that's what the `Debug `impl` now produces.
So this commit removes the plumbing.
It was removed in #115920 to enable it being moved to
`rustc_query_system`, a move that has recently been reversed. It's much
simpler as an enum.
Also:
- Remove the overly complicated `Debug` impl for `DepKind`.
- Remove the trivial `DepKind` associated constants (`NULL` et al.)
- Add an assertion to ensure that the number of `DepKinds` fits within a
`u16`.
- Rename `DEP_KIND_VARIANTS` as `DEP_KIND_NUM_VARIANTS`, to make it
clearer that it's a count, not a collection.
- Use `stringify!` in one place to make the code clearer.
This struct was only wrapping `TyCtxt` in order to implement traits that
were removed by RUST-152636.
This commit also slightly simplifies the signature of `execute_job_incr`, by
having it call `tcx.dep_graph.data()` internally.
Big query system cleanups
Recent PRs have moved a lot of code from `rustc_query_system` to `rustc_middle` and `rustc_query_impl`, where this code now has access to `TyCtxt`, e.g. rust-lang/rust#152419, rust-lang/rust#152516. As a result, a lot of abstraction and indirection that existed to work around this limitation is no longer necessary. This PR removes a lot of it.
r? @Zalathar
By removing the generic `D` parameter from `DepGraph`, `DepGraphData`,
`CurrentDepGraph`, `SerializedDepGraph`, `SerializedNodeHeader`, and
`EncoderState`.
Most of the files within the `dep_graph` module can be moved wholesale
into `rustc_middle`. But two of them (`mod.rs` and `dep_node.rs`) have
the same name as existing files in `rustc_middle`, so for those I just
copied the contents into the existing files.
The commit also moves `QueryContext` and `incremental_verify_ich*`
because they are tightly intertwined with the dep graph code. And a
couple of error structs moved as well.
bors