Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 7 additions & 14 deletions compiler/rustc_middle/src/query/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_span::{DUMMY_SP, ErrorGuaranteed, Span};
use crate::dep_graph;
use crate::dep_graph::DepNodeKey;
use crate::query::erase::{self, Erasable, Erased};
use crate::query::{EnsureMode, QueryCache, QueryMode, QueryVTable};
use crate::query::{QueryCache, QueryMode, QueryVTable};
use crate::ty::TyCtxt;

/// Checks whether there is already a value for this key in the in-memory
Expand All @@ -28,8 +28,8 @@ where
}
}

/// Shared implementation of `tcx.$query(..)` and `tcx.at(span).$query(..)`
/// for all queries.
/// Shared implementation of `tcx.$query(..)`, `tcx.at(span).$query(..)` and
/// `tcx.ensure_done().$query(..)` for all queries.
#[inline(always)]
pub(crate) fn query_get_at<'tcx, C>(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be renamed query_get_or_ensure_done. (I think the at can be dropped because it's low-value and QueryMode doesn't have it.)

tcx: TyCtxt<'tcx>,
Expand All @@ -46,21 +46,19 @@ where
}
}

/// Shared implementation of `tcx.ensure_ok().$query(..)` and
/// `tcx.ensure_done().$query(..)` for all queries.
/// Implementation of `tcx.ensure_ok().$query(..)` for all queries.
#[inline]
pub(crate) fn query_ensure_ok_or_done<'tcx, C>(
pub(crate) fn query_ensure_ok<'tcx, C>(
tcx: TyCtxt<'tcx>,
query: &'tcx QueryVTable<'tcx, C>,
key: C::Key,
ensure_mode: EnsureMode,
) where
C: QueryCache,
{
match try_get_cached(tcx, &query.cache, key) {
Some(_value) => {}
None => {
(query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Ensure { ensure_mode });
(query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Ensure);
}
}
}
Expand All @@ -87,12 +85,7 @@ where
match try_get_cached(tcx, &query.cache, key) {
Some(value) => convert(value),
None => {
match (query.execute_query_fn)(
tcx,
DUMMY_SP,
key,
QueryMode::Ensure { ensure_mode: EnsureMode::Ok },
) {
match (query.execute_query_fn)(tcx, DUMMY_SP, key, QueryMode::Ensure) {
// We executed the query. Convert the successful result.
Some(res) => convert(res),

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ pub use self::into_query_key::IntoQueryKey;
pub use self::job::{QueryJob, QueryJobId, QueryLatch, QueryWaiter};
pub use self::keys::{AsLocalQueryKey, LocalCrate, QueryKey};
pub use self::plumbing::{
ActiveKeyStatus, Cycle, EnsureMode, QueryMode, QueryState, QuerySystem, QueryVTable, TyCtxtAt,
ActiveKeyStatus, Cycle, QueryMode, QueryState, QuerySystem, QueryVTable, TyCtxtAt,
TyCtxtEnsureDone, TyCtxtEnsureOk, TyCtxtEnsureResult,
};
pub use self::stack::QueryStackFrame;
Expand Down
41 changes: 10 additions & 31 deletions compiler/rustc_middle/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,10 @@ pub struct Cycle<'tcx> {

#[derive(Debug)]
pub enum QueryMode {
/// This is a normal query call to `tcx.$query(..)` or `tcx.at(span).$query(..)`.
/// This is a query call to `tcx.$query(..)`, `tcx.at(span).$query(..)` or `tcx.ensure_done().$query(..)`.
Get,
/// This is a call to `tcx.ensure_ok().$query(..)` or `tcx.ensure_done().$query(..)`.
Ensure { ensure_mode: EnsureMode },
}

/// Distinguishes between `tcx.ensure_ok()` and `tcx.ensure_done()` in shared
/// code paths that handle both modes.
#[derive(Debug)]
pub enum EnsureMode {
/// Corresponds to [`TyCtxt::ensure_ok`].
Ok,
/// Corresponds to [`TyCtxt::ensure_done`].
Done,
/// This is a call to `tcx.ensure_ok().$query(..)`.
Ensure,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variants should be named GetOrEnsureDone and EnsureOk. (A bit clumsy but extremely clear.)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think GetOrEnsureDone is more misleading.

It's not a get-or-ensure-done mode; it's a get mode that ensure-done delegates to because it currently doesn't have a specialized path of its own.

}

/// Stores data and metadata (e.g. function pointers) for a particular query.
Expand Down Expand Up @@ -245,20 +235,13 @@ impl<'tcx> TyCtxt<'tcx> {
TyCtxtEnsureResult { tcx: self }
}

/// Wrapper that calls queries in a special "ensure done" mode, for callers
/// that don't need the return value and just want to guarantee that the
/// query won't be executed in the future, by executing it now if necessary.
/// Wrapper that calls queries where callers don't need the return value and
/// just want to guarantee that the query won't be executed in the future.
///
/// This is useful for queries that read from a [`Steal`] value, to ensure
/// that they are executed before the query that will steal the value.
///
/// Unlike [`Self::ensure_ok`], a query with all-green inputs will only be
/// skipped if its return value is stored in the disk-cache. This is still
/// more efficient than a regular query, because in that situation the
/// return value doesn't necessarily need to be decoded.
///
/// (As with all query calls, execution is also skipped if the query result
/// is already cached in memory.)
/// Currently this causes the query to be executed normally, but this behavior may change.
///
/// [`Steal`]: rustc_data_structures::steal::Steal
#[inline(always)]
Expand Down Expand Up @@ -583,11 +566,10 @@ macro_rules! define_callbacks {
$(#[$attr])*
#[inline(always)]
pub fn $name(self, key: maybe_into_query_key!($($K)*)) {
$crate::query::inner::query_ensure_ok_or_done(
$crate::query::inner::query_ensure_ok(
self.tcx,
&self.tcx.query_system.query_vtables.$name,
$crate::query::IntoQueryKey::into_query_key(key),
$crate::query::EnsureMode::Ok,
)
}
)*
Expand Down Expand Up @@ -617,12 +599,9 @@ macro_rules! define_callbacks {
$(#[$attr])*
#[inline(always)]
pub fn $name(self, key: maybe_into_query_key!($($K)*)) {
$crate::query::inner::query_ensure_ok_or_done(
self.tcx,
&self.tcx.query_system.query_vtables.$name,
$crate::query::IntoQueryKey::into_query_key(key),
$crate::query::EnsureMode::Done,
);
// This has the same implementation as `tcx.$query(..)` as it isn't currently
// beneficial to have an optimized variant due to how promotion works.
let _ = self.tcx.$name(key);
}
)*
}
Expand Down
40 changes: 14 additions & 26 deletions compiler/rustc_query_impl/src/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use rustc_data_structures::{outline, sharded, sync};
use rustc_errors::FatalError;
use rustc_middle::dep_graph::{DepGraphData, DepNodeKey, SerializedDepNodeIndex};
use rustc_middle::query::{
ActiveKeyStatus, Cycle, EnsureMode, QueryCache, QueryJob, QueryJobId, QueryKey, QueryLatch,
QueryMode, QueryState, QueryVTable,
ActiveKeyStatus, Cycle, QueryCache, QueryJob, QueryJobId, QueryKey, QueryLatch, QueryMode,
QueryState, QueryVTable,
};
use rustc_middle::ty::TyCtxt;
use rustc_middle::verify_ich::incremental_verify_ich;
Expand All @@ -18,7 +18,7 @@ use tracing::warn;

use crate::dep_graph::{DepNode, DepNodeIndex};
use crate::job::{QueryJobInfo, QueryJobMap, create_cycle_error, find_cycle_in_stack};
use crate::plumbing::{current_query_job, loadable_from_disk, next_job_id, start_query};
use crate::plumbing::{current_query_job, next_job_id, start_query};
use crate::query_impl::for_each_query_vtable;

#[inline]
Expand Down Expand Up @@ -532,7 +532,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>(
value
}

/// Checks whether a `tcx.ensure_ok()` or `tcx.ensure_done()` query call can
/// Checks whether a `tcx.ensure_ok()` query call can
/// return early without actually trying to execute.
///
/// This only makes sense during incremental compilation, because it relies
Expand All @@ -542,9 +542,7 @@ fn load_from_disk_or_invoke_provider_green<'tcx, C: QueryCache>(
fn ensure_can_skip_execution<'tcx, C: QueryCache>(
query: &'tcx QueryVTable<'tcx, C>,
tcx: TyCtxt<'tcx>,
key: C::Key,
dep_node: DepNode,
ensure_mode: EnsureMode,
) -> bool {
// Queries with `eval_always` should never skip execution.
if query.eval_always {
Expand All @@ -561,25 +559,15 @@ fn ensure_can_skip_execution<'tcx, C: QueryCache>(
// in-memory cache, or another query down the line will.
false
}
Some((serialized_dep_node_index, dep_node_index)) => {
Some((_, dep_node_index)) => {
tcx.dep_graph.read_index(dep_node_index);
tcx.prof.query_cache_hit(dep_node_index.into());
match ensure_mode {
// In ensure-ok mode, we can skip execution for this key if the
// node is green. It must have succeeded in the previous
// session, and therefore would succeed in the current session
// if executed.
EnsureMode::Ok => true,

// In ensure-done mode, we can only skip execution for this key
// if there's a disk-cached value available to load later if
// needed, which guarantees the query provider will never run
// for this key.
EnsureMode::Done => {
(query.will_cache_on_disk_for_key_fn)(tcx, key)
&& loadable_from_disk(tcx, serialized_dep_node_index)
}
}

// We can skip execution for this key if the
// node is green. It must have succeeded in the previous
// session, and therefore would succeed in the current session
// if executed.
true
}
}
}
Expand Down Expand Up @@ -608,9 +596,9 @@ pub(super) fn execute_query_incr_inner<'tcx, C: QueryCache>(
) -> Option<C::Value> {
let dep_node = DepNode::construct(tcx, query.dep_kind, &key);

// Check if query execution can be skipped, for `ensure_ok` or `ensure_done`.
if let QueryMode::Ensure { ensure_mode } = mode
&& ensure_can_skip_execution(query, tcx, key, dep_node, ensure_mode)
// Check if query execution can be skipped, for `ensure_ok`.
if let QueryMode::Ensure = mode
&& ensure_can_skip_execution(query, tcx, dep_node)
{
return None;
}
Expand Down
8 changes: 0 additions & 8 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,6 @@ pub(crate) fn promote_from_disk_inner<'tcx, C: QueryCache>(
}
}

pub(crate) fn loadable_from_disk<'tcx>(tcx: TyCtxt<'tcx>, id: SerializedDepNodeIndex) -> bool {
if let Some(cache) = tcx.query_system.on_disk_cache.as_ref() {
cache.loadable_from_disk(id)
} else {
false
}
}

pub(crate) fn try_load_from_disk<'tcx, V>(
tcx: TyCtxt<'tcx>,
prev_index: SerializedDepNodeIndex,
Expand Down
Loading