From a16b49bb933928facaff9ba9f3a22b98029f9923 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 15 Jun 2025 15:30:14 +0000 Subject: [PATCH 1/5] Manually invalidate caches in SimplifyCfg. --- compiler/rustc_mir_transform/src/simplify.rs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs index 8f88228d9bbd8..49270f0c5ca35 100644 --- a/compiler/rustc_mir_transform/src/simplify.rs +++ b/compiler/rustc_mir_transform/src/simplify.rs @@ -74,7 +74,9 @@ impl SimplifyCfg { } pub(super) fn simplify_cfg<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - CfgSimplifier::new(tcx, body).simplify(); + if CfgSimplifier::new(tcx, body).simplify() { + body.basic_blocks.invalidate_cfg_cache(); + } remove_dead_blocks(body); // FIXME: Should probably be moved into some kind of pass manager @@ -121,12 +123,15 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { // Preserve `SwitchInt` reads on built and analysis MIR, or if `-Zmir-preserve-ub`. let preserve_switch_reads = matches!(body.phase, MirPhase::Built | MirPhase::Analysis(_)) || tcx.sess.opts.unstable_opts.mir_preserve_ub; - let basic_blocks = body.basic_blocks_mut(); + let basic_blocks = body.basic_blocks.as_mut_preserves_cfg(); CfgSimplifier { preserve_switch_reads, basic_blocks, pred_count } } - fn simplify(mut self) { + /// Returns whether we actually simplified anything. In that case, the caller *must* invalidate + /// the CFG caches of the MIR body. + #[must_use] + fn simplify(mut self) -> bool { self.strip_nops(); // Vec of the blocks that should be merged. We store the indices here, instead of the @@ -134,6 +139,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { // We do not push the statements directly into the target block (`bb`) as that is slower // due to additional reallocations let mut merged_blocks = Vec::new(); + let mut outer_changed = false; loop { let mut changed = false; @@ -177,7 +183,11 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { if !changed { break; } + + outer_changed = true; } + + outer_changed } /// This function will return `None` if From a2d96875a58f0de8cad410a285fb8b4fa0dfa038 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Mon, 16 Jun 2025 12:31:36 +0000 Subject: [PATCH 2/5] Add comment. --- compiler/rustc_mir_transform/src/simplify.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_mir_transform/src/simplify.rs b/compiler/rustc_mir_transform/src/simplify.rs index 49270f0c5ca35..a54e548ad70ab 100644 --- a/compiler/rustc_mir_transform/src/simplify.rs +++ b/compiler/rustc_mir_transform/src/simplify.rs @@ -75,6 +75,8 @@ impl SimplifyCfg { pub(super) fn simplify_cfg<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { if CfgSimplifier::new(tcx, body).simplify() { + // `simplify` returns that it changed something. We must invalidate the CFG caches as they + // are not consistent with the modified CFG any more. body.basic_blocks.invalidate_cfg_cache(); } remove_dead_blocks(body); @@ -123,6 +125,7 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> { // Preserve `SwitchInt` reads on built and analysis MIR, or if `-Zmir-preserve-ub`. let preserve_switch_reads = matches!(body.phase, MirPhase::Built | MirPhase::Analysis(_)) || tcx.sess.opts.unstable_opts.mir_preserve_ub; + // Do not clear caches yet. The caller to `simplify` will do it if anything changed. let basic_blocks = body.basic_blocks.as_mut_preserves_cfg(); CfgSimplifier { preserve_switch_reads, basic_blocks, pred_count } From fdeaf2436633af30be7c20cc4f691c328fe33734 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 15 Jun 2025 13:59:41 +0000 Subject: [PATCH 3/5] Inform RegionRenumberer that it preserves MIR CFG. --- compiler/rustc_borrowck/src/renumber.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_borrowck/src/renumber.rs b/compiler/rustc_borrowck/src/renumber.rs index ff92b4168a86c..037ebc9c0a7a3 100644 --- a/compiler/rustc_borrowck/src/renumber.rs +++ b/compiler/rustc_borrowck/src/renumber.rs @@ -21,10 +21,10 @@ pub(crate) fn renumber_mir<'tcx>( let mut renumberer = RegionRenumberer { infcx }; for body in promoted.iter_mut() { - renumberer.visit_body(body); + renumberer.visit_body_preserves_cfg(body); } - renumberer.visit_body(body); + renumberer.visit_body_preserves_cfg(body); } // The fields are used only for debugging output in `sccs_info`. From 092c01ec9f3aa8d25ea49c1e2c0f48259836ca87 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 15 Jun 2025 14:16:11 +0000 Subject: [PATCH 4/5] Pre-cache traversals in mir_promoted. --- compiler/rustc_middle/src/mir/basic_blocks.rs | 9 +++++++++ compiler/rustc_mir_transform/src/lib.rs | 4 ++++ 2 files changed, 13 insertions(+) diff --git a/compiler/rustc_middle/src/mir/basic_blocks.rs b/compiler/rustc_middle/src/mir/basic_blocks.rs index d0dbf64dc5959..89d720bf09bd1 100644 --- a/compiler/rustc_middle/src/mir/basic_blocks.rs +++ b/compiler/rustc_middle/src/mir/basic_blocks.rs @@ -52,6 +52,15 @@ impl<'tcx> BasicBlocks<'tcx> { BasicBlocks { basic_blocks, cache: Cache::default() } } + /// Force caching of traversals. + pub fn cache_traversals(&self) { + let _ = self.predecessors(); + let _ = self.switch_sources(); + let _ = self.reverse_postorder(); + let _ = self.dominators(); + } + + #[inline] pub fn dominators(&self) -> &Dominators { self.cache.dominators.get_or_init(|| dominators(self)) } diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 572ad585c8c87..d068a23499047 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -446,6 +446,10 @@ fn mir_promoted( pm::Optimizations::Allowed, ); + // The returned MIR will be used for analyses. Make sure that the traversals are computed + // early, so we don't have to re-run them. + body.basic_blocks.cache_traversals(); + lint_tail_expr_drop_order::run_lint(tcx, def, &body); let promoted = promote_pass.promoted_fragments.into_inner(); From ba39fdf96b7b5d653ed714c3779ca4bd782e8ef9 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 15 Jun 2025 20:34:36 +0000 Subject: [PATCH 5/5] Use an Arc to share caches between clones. --- compiler/rustc_middle/src/mir/basic_blocks.rs | 24 +++++++++---------- compiler/rustc_mir_transform/src/lib.rs | 4 ---- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/compiler/rustc_middle/src/mir/basic_blocks.rs b/compiler/rustc_middle/src/mir/basic_blocks.rs index 89d720bf09bd1..02984589422e2 100644 --- a/compiler/rustc_middle/src/mir/basic_blocks.rs +++ b/compiler/rustc_middle/src/mir/basic_blocks.rs @@ -1,4 +1,4 @@ -use std::sync::OnceLock; +use std::sync::{Arc, OnceLock}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::graph; @@ -15,7 +15,8 @@ use crate::mir::{BasicBlock, BasicBlockData, START_BLOCK, Terminator, Terminator #[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable, TypeFoldable, TypeVisitable)] pub struct BasicBlocks<'tcx> { basic_blocks: IndexVec>, - cache: Cache, + /// Use an `Arc` so we can share the cache when we clone the MIR body, as borrowck does. + cache: Arc, } // Typically 95%+ of basic blocks have 4 or fewer predecessors. @@ -49,15 +50,7 @@ struct Cache { impl<'tcx> BasicBlocks<'tcx> { #[inline] pub fn new(basic_blocks: IndexVec>) -> Self { - BasicBlocks { basic_blocks, cache: Cache::default() } - } - - /// Force caching of traversals. - pub fn cache_traversals(&self) { - let _ = self.predecessors(); - let _ = self.switch_sources(); - let _ = self.reverse_postorder(); - let _ = self.dominators(); + BasicBlocks { basic_blocks, cache: Arc::new(Cache::default()) } } #[inline] @@ -151,7 +144,14 @@ impl<'tcx> BasicBlocks<'tcx> { /// All other methods that allow you to mutate the basic blocks also call this method /// themselves, thereby avoiding any risk of accidentally cache invalidation. pub fn invalidate_cfg_cache(&mut self) { - self.cache = Cache::default(); + if let Some(cache) = Arc::get_mut(&mut self.cache) { + // If we only have a single reference to this cache, clear it. + *cache = Cache::default(); + } else { + // If we have several references to this cache, overwrite the pointer itself so other + // users can continue to use their (valid) cache. + self.cache = Arc::new(Cache::default()); + } } } diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index d068a23499047..572ad585c8c87 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -446,10 +446,6 @@ fn mir_promoted( pm::Optimizations::Allowed, ); - // The returned MIR will be used for analyses. Make sure that the traversals are computed - // early, so we don't have to re-run them. - body.basic_blocks.cache_traversals(); - lint_tail_expr_drop_order::run_lint(tcx, def, &body); let promoted = promote_pass.promoted_fragments.into_inner();