Skip to content

Commit 54314d7

Browse files
committed
Improve how QueryCaches and QueryStates are stored.
`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`.
1 parent b3869b9 commit 54314d7

5 files changed

Lines changed: 27 additions & 78 deletions

File tree

compiler/rustc_middle/src/query/inner.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ pub(crate) fn query_feed<'tcx, Cache>(
100100
tcx: TyCtxt<'tcx>,
101101
dep_kind: DepKind,
102102
query_vtable: &QueryVTable<'tcx, Cache>,
103-
cache: &Cache,
104103
key: Cache::Key,
105104
value: Cache::Value,
106105
) where
@@ -110,7 +109,7 @@ pub(crate) fn query_feed<'tcx, Cache>(
110109
let format_value = query_vtable.format_value;
111110

112111
// Check whether the in-memory cache already has a value for this key.
113-
match try_get_cached(tcx, cache, &key) {
112+
match try_get_cached(tcx, &query_vtable.cache, &key) {
114113
Some(old) => {
115114
// The query already has a cached value for this key.
116115
// That's OK if both values are the same, i.e. they have the same hash,
@@ -153,7 +152,7 @@ pub(crate) fn query_feed<'tcx, Cache>(
153152
query_vtable.hash_result,
154153
query_vtable.format_value,
155154
);
156-
cache.complete(key, value, dep_node_index);
155+
query_vtable.cache.complete(key, value, dep_node_index);
157156
}
158157
}
159158
}

compiler/rustc_middle/src/query/plumbing.rs

Lines changed: 7 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ pub use sealed::IntoQueryParam;
1414
use crate::dep_graph;
1515
use crate::dep_graph::{DepKind, DepNode, DepNodeIndex, SerializedDepNodeIndex};
1616
use crate::ich::StableHashingContext;
17-
use crate::queries::{
18-
ExternProviders, PerQueryVTables, Providers, QueryArenas, QueryCaches, QueryEngine, QueryStates,
19-
};
17+
use crate::queries::{ExternProviders, PerQueryVTables, Providers, QueryArenas, QueryEngine};
2018
use crate::query::on_disk_cache::{CacheEncoder, EncodedDepNodeIndex, OnDiskCache};
2119
use crate::query::stack::{QueryStackDeferred, QueryStackFrame, QueryStackFrameExtra};
2220
use crate::query::{QueryCache, QueryInfo, QueryJob};
@@ -101,7 +99,7 @@ pub enum QueryMode {
10199
Ensure { check_cache: bool },
102100
}
103101

104-
/// Stores function pointers and other metadata for a particular query.
102+
/// Stores data and metadata (e.g. function pointers) for a particular query.
105103
pub struct QueryVTable<'tcx, C: QueryCache> {
106104
pub name: &'static str,
107105

@@ -117,10 +115,8 @@ pub struct QueryVTable<'tcx, C: QueryCache> {
117115
pub dep_kind: DepKind,
118116
/// How this query deals with query cycle errors.
119117
pub cycle_error_handling: CycleErrorHandling,
120-
// Offset of this query's state field in the QueryStates struct
121-
pub query_state: usize,
122-
// Offset of this query's cache field in the QueryCaches struct
123-
pub query_cache: usize,
118+
pub state: QueryState<'tcx, C::Key>,
119+
pub cache: C,
124120
pub will_cache_on_disk_for_key_fn: Option<WillCacheOnDiskForKeyFn<'tcx, C::Key>>,
125121

126122
/// Function pointer that calls `tcx.$query(key)` for this query and
@@ -169,30 +165,6 @@ impl<'tcx, C: QueryCache> QueryVTable<'tcx, C> {
169165
self.will_cache_on_disk_for_key_fn.map_or(false, |f| f(tcx, key))
170166
}
171167

172-
// Don't use this method to access query results, instead use the methods on TyCtxt.
173-
#[inline(always)]
174-
pub fn query_state(&self, tcx: TyCtxt<'tcx>) -> &'tcx QueryState<'tcx, C::Key> {
175-
// Safety:
176-
// This is just manually doing the subfield referencing through pointer math.
177-
unsafe {
178-
&*(&tcx.query_system.states as *const QueryStates<'tcx>)
179-
.byte_add(self.query_state)
180-
.cast::<QueryState<'tcx, C::Key>>()
181-
}
182-
}
183-
184-
// Don't use this method to access query results, instead use the methods on TyCtxt.
185-
#[inline(always)]
186-
pub fn query_cache(&self, tcx: TyCtxt<'tcx>) -> &'tcx C {
187-
// Safety:
188-
// This is just manually doing the subfield referencing through pointer math.
189-
unsafe {
190-
&*(&tcx.query_system.caches as *const QueryCaches<'tcx>)
191-
.byte_add(self.query_cache)
192-
.cast::<C>()
193-
}
194-
}
195-
196168
#[inline(always)]
197169
pub fn try_load_from_disk(
198170
&self,
@@ -243,9 +215,7 @@ pub struct QuerySystemFns {
243215
}
244216

245217
pub struct QuerySystem<'tcx> {
246-
pub states: QueryStates<'tcx>,
247218
pub arenas: WorkerLocal<QueryArenas<'tcx>>,
248-
pub caches: QueryCaches<'tcx>,
249219
pub query_vtables: PerQueryVTables<'tcx>,
250220

251221
/// This provides access to the incremental compilation on-disk cache for query results.
@@ -498,13 +468,6 @@ macro_rules! define_callbacks {
498468
)*
499469
}
500470

501-
#[derive(Default)]
502-
pub struct QueryCaches<'tcx> {
503-
$(
504-
pub $name: $name::Storage<'tcx>,
505-
)*
506-
}
507-
508471
impl<'tcx> $crate::query::TyCtxtEnsureOk<'tcx> {
509472
$(
510473
$(#[$attr])*
@@ -524,7 +487,7 @@ macro_rules! define_callbacks {
524487
)(
525488
self.tcx,
526489
self.tcx.query_system.fns.engine.$name,
527-
&self.tcx.query_system.caches.$name,
490+
&self.tcx.query_system.query_vtables.$name.cache,
528491
$crate::query::IntoQueryParam::into_query_param(key),
529492
false,
530493
)
@@ -540,7 +503,7 @@ macro_rules! define_callbacks {
540503
crate::query::inner::query_ensure(
541504
self.tcx,
542505
self.tcx.query_system.fns.engine.$name,
543-
&self.tcx.query_system.caches.$name,
506+
&self.tcx.query_system.query_vtables.$name.cache,
544507
$crate::query::IntoQueryParam::into_query_param(key),
545508
true,
546509
);
@@ -569,7 +532,7 @@ macro_rules! define_callbacks {
569532
erase::restore_val::<$V>(inner::query_get_at(
570533
self.tcx,
571534
self.tcx.query_system.fns.engine.$name,
572-
&self.tcx.query_system.caches.$name,
535+
&self.tcx.query_system.query_vtables.$name.cache,
573536
self.span,
574537
$crate::query::IntoQueryParam::into_query_param(key),
575538
))
@@ -586,13 +549,6 @@ macro_rules! define_callbacks {
586549
)*
587550
}
588551

589-
#[derive(Default)]
590-
pub struct QueryStates<'tcx> {
591-
$(
592-
pub $name: $crate::query::QueryState<'tcx, $name::Key<'tcx>>,
593-
)*
594-
}
595-
596552
pub struct Providers {
597553
$(
598554
/// This is the provider for the query. Use `Find references` on this to
@@ -689,7 +645,6 @@ macro_rules! define_feedable {
689645
tcx,
690646
dep_graph::DepKind::$name,
691647
&tcx.query_system.query_vtables.$name,
692-
&tcx.query_system.caches.$name,
693648
key,
694649
erased_value,
695650
);

compiler/rustc_query_impl/src/execution.rs

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -244,12 +244,12 @@ fn wait_for_query<'tcx, C: QueryCache>(
244244

245245
match result {
246246
Ok(()) => {
247-
let Some((v, index)) = query.query_cache(tcx).lookup(&key) else {
247+
let Some((v, index)) = query.cache.lookup(&key) else {
248248
outline(|| {
249249
// We didn't find the query result in the query cache. Check if it was
250250
// poisoned due to a panic instead.
251251
let key_hash = sharded::make_hash(&key);
252-
let shard = query.query_state(tcx).active.lock_shard_by_hash(key_hash);
252+
let shard = query.state.active.lock_shard_by_hash(key_hash);
253253
match shard.find(key_hash, equivalent_key(&key)) {
254254
// The query we waited on panicked. Continue unwinding here.
255255
Some((_, ActiveKeyStatus::Poisoned)) => FatalError.raise(),
@@ -278,9 +278,8 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
278278
key: C::Key,
279279
dep_node: Option<DepNode>,
280280
) -> (C::Value, Option<DepNodeIndex>) {
281-
let state = query.query_state(tcx);
282281
let key_hash = sharded::make_hash(&key);
283-
let mut state_lock = state.active.lock_shard_by_hash(key_hash);
282+
let mut state_lock = query.state.active.lock_shard_by_hash(key_hash);
284283

285284
// For the parallel compiler we need to check both the query cache and query state structures
286285
// while holding the state lock to ensure that 1) the query has not yet completed and 2) the
@@ -289,7 +288,7 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
289288
// executing, but another thread may have already completed the query and stores it result
290289
// in the query cache.
291290
if tcx.sess.threads() > 1 {
292-
if let Some((value, index)) = query.query_cache(tcx).lookup(&key) {
291+
if let Some((value, index)) = query.cache.lookup(&key) {
293292
tcx.prof.query_cache_hit(index.into());
294293
return (value, Some(index));
295294
}
@@ -308,7 +307,7 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
308307
// Drop the lock before we start executing the query
309308
drop(state_lock);
310309

311-
execute_job::<C, INCR>(query, tcx, state, key, key_hash, id, dep_node)
310+
execute_job::<C, INCR>(query, tcx, key, key_hash, id, dep_node)
312311
}
313312
Entry::Occupied(mut entry) => {
314313
match &mut entry.get_mut().1 {
@@ -340,15 +339,14 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>(
340339
fn execute_job<'tcx, C: QueryCache, const INCR: bool>(
341340
query: &'tcx QueryVTable<'tcx, C>,
342341
tcx: TyCtxt<'tcx>,
343-
state: &'tcx QueryState<'tcx, C::Key>,
344342
key: C::Key,
345343
key_hash: u64,
346344
id: QueryJobId,
347345
dep_node: Option<DepNode>,
348346
) -> (C::Value, Option<DepNodeIndex>) {
349347
// Set up a guard object that will automatically poison the query if a
350348
// panic occurs while executing the query (or any intermediate plumbing).
351-
let job_guard = ActiveJobGuard { state, key, key_hash };
349+
let job_guard = ActiveJobGuard { state: &query.state, key, key_hash };
352350

353351
debug_assert_eq!(tcx.dep_graph.is_fully_enabled(), INCR);
354352

@@ -359,7 +357,7 @@ fn execute_job<'tcx, C: QueryCache, const INCR: bool>(
359357
execute_job_non_incr(query, tcx, key, id)
360358
};
361359

362-
let cache = query.query_cache(tcx);
360+
let cache = &query.cache;
363361
if query.feedable {
364362
// We should not compute queries that also got a value via feeding.
365363
// This can't happen, as query feeding adds the very dependencies to the fed query
@@ -681,7 +679,7 @@ pub(crate) fn force_query<'tcx, C: QueryCache>(
681679
) {
682680
// We may be concurrently trying both execute and force a query.
683681
// Ensure that only one of them runs the query.
684-
if let Some((_, index)) = query.query_cache(tcx).lookup(&key) {
682+
if let Some((_, index)) = query.cache.lookup(&key) {
685683
tcx.prof.query_cache_hit(index.into());
686684
return;
687685
}

compiler/rustc_query_impl/src/lib.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@
1010

1111
use rustc_data_structures::sync::AtomicU64;
1212
use rustc_middle::dep_graph;
13-
use rustc_middle::queries::{
14-
self, ExternProviders, Providers, QueryCaches, QueryEngine, QueryStates,
15-
};
13+
use rustc_middle::queries::{self, ExternProviders, Providers, QueryEngine};
1614
use rustc_middle::query::on_disk_cache::{CacheEncoder, EncodedDepNodeIndex, OnDiskCache};
1715
use rustc_middle::query::plumbing::{QuerySystem, QuerySystemFns, QueryVTable};
1816
use rustc_middle::query::{AsLocalKey, QueryCache, QueryMode};
@@ -62,9 +60,7 @@ pub fn query_system<'tcx>(
6260
incremental: bool,
6361
) -> QuerySystem<'tcx> {
6462
QuerySystem {
65-
states: Default::default(),
6663
arenas: Default::default(),
67-
caches: Default::default(),
6864
query_vtables: make_query_vtables(),
6965
on_disk_cache,
7066
fns: QuerySystemFns {

compiler/rustc_query_impl/src/plumbing.rs

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ pub(crate) fn create_deferred_query_stack_frame<'tcx, Cache>(
314314
where
315315
Cache: QueryCache,
316316
Cache::Key: Key + DynSend + DynSync,
317+
QueryVTable<'tcx, Cache>: DynSync,
317318
{
318319
let kind = vtable.dep_kind;
319320

@@ -335,9 +336,8 @@ pub(crate) fn encode_query_results<'a, 'tcx, Q, C: QueryCache>(
335336
{
336337
let _timer = tcx.prof.generic_activity_with_arg("encode_query_results_for", query.name);
337338

338-
assert!(all_inactive(query.query_state(tcx)));
339-
let cache = query.query_cache(tcx);
340-
cache.iter(&mut |key, value, dep_node| {
339+
assert!(all_inactive(&query.state));
340+
query.cache.iter(&mut |key, value, dep_node| {
341341
if query.will_cache_on_disk_for_key(tcx, key) {
342342
let dep_node = SerializedDepNodeIndex::new(dep_node.index());
343343

@@ -357,7 +357,7 @@ pub(crate) fn query_key_hash_verify<'tcx, C: QueryCache>(
357357
) {
358358
let _timer = tcx.prof.generic_activity_with_arg("query_key_hash_verify_for", query.name);
359359

360-
let cache = query.query_cache(tcx);
360+
let cache = &query.cache;
361361
let mut map = UnordMap::with_capacity(cache.len());
362362
cache.iter(&mut |key, _, _| {
363363
let node = DepNode::construct(tcx, query.dep_kind, key);
@@ -560,8 +560,8 @@ macro_rules! define_queries {
560560
feedable: is_feedable!([$($modifiers)*]),
561561
dep_kind: dep_graph::DepKind::$name,
562562
cycle_error_handling: cycle_error_handling!([$($modifiers)*]),
563-
query_state: std::mem::offset_of!(QueryStates<'tcx>, $name),
564-
query_cache: std::mem::offset_of!(QueryCaches<'tcx>, $name),
563+
state: Default::default(),
564+
cache: Default::default(),
565565
will_cache_on_disk_for_key_fn: if_cache_on_disk!([$($modifiers)*] {
566566
Some(::rustc_middle::queries::_cache_on_disk_if_fns::$name)
567567
} {
@@ -646,7 +646,8 @@ macro_rules! define_queries {
646646
};
647647

648648
// Call `gather_active_jobs_inner` to do the actual work.
649-
let res = crate::execution::gather_active_jobs_inner(&tcx.query_system.states.$name,
649+
let res = crate::execution::gather_active_jobs_inner(
650+
&tcx.query_system.query_vtables.$name.state,
650651
tcx,
651652
make_frame,
652653
require_complete,
@@ -672,7 +673,7 @@ macro_rules! define_queries {
672673
$crate::profiling_support::alloc_self_profile_query_strings_for_query_cache(
673674
tcx,
674675
stringify!($name),
675-
&tcx.query_system.caches.$name,
676+
&tcx.query_system.query_vtables.$name.cache,
676677
string_cache,
677678
)
678679
}

0 commit comments

Comments
 (0)