Auto merge of #153472 - Zoxc:rem-ensure_done, r=petrochenkov

Remove `ensure_done` execution path



This removes the `ensure_done` execution path as it doesn't have a performance impact. `ensure_done` calls are left as markers still.

Author: bors

compiler/rustc_middle/src/query/inner.rs

@@ -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
@@ -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::EnsureOk);
         }
     }
 }
@@ -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::EnsureOk) {
                 // We executed the query. Convert the successful result.
                 Some(res) => convert(res),
 

compiler/rustc_middle/src/query/mod.rs

@@ -5,7 +5,7 @@ pub use self::into_query_key::IntoQueryKey;
 pub use self::job::{QueryJob, QueryJobId, QueryLatch, QueryWaiter};
 pub use self::keys::{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;

compiler/rustc_middle/src/query/plumbing.rs

@@ -61,18 +61,8 @@ pub struct Cycle<'tcx> {
 pub enum QueryMode {
     /// This is a normal query call to `tcx.$query(..)` or `tcx.at(span).$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(..)`.
+    EnsureOk,
 }
 
 /// Stores data and metadata (e.g. function pointers) for a particular query.
@@ -268,20 +258,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)]
@@ -593,11 +576,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,
                     )
                 }
             )*
@@ -627,12 +609,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);
                 }
             )*
         }

compiler/rustc_query_impl/src/execution.rs

@@ -8,8 +8,8 @@ use rustc_data_structures::{defer, 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;
@@ -19,7 +19,7 @@ use tracing::debug;
 use crate::dep_graph::{DepNode, DepNodeIndex};
 use crate::handle_cycle_error;
 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]
@@ -564,7 +564,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
@@ -574,9 +574,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 {
@@ -593,25 +591,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(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
         }
     }
 }
@@ -640,9 +628,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::EnsureOk = mode
+        && ensure_can_skip_execution(query, tcx, dep_node)
     {
         return None;
     }

compiler/rustc_query_impl/src/plumbing.rs

@@ -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,