Skip to content

Commit 0e5f13e

Browse files
Auto merge of #142540 - cjgillot:renumber-cfg, r=<try>
Pre-compute MIR CFG caches for borrowck and other analyses I was puzzled that #142390 introduces additional computations of CFG traversals: borrowck computes them, right? It turns out that borrowck clones the MIR body, so doesn't share its cache with other analyses. This PR: - forces the computation of all caches in `mir_promoted` query; - modifies region renumbering to avoid dropping that cache. <!-- homu-ignore:start --> <!-- If this PR is related to an unstable feature or an otherwise tracked effort, please link to the relevant tracking issue here. If you don't know of a related tracking issue or there are none, feel free to ignore this. This PR will get automatically assigned to a reviewer. In case you would like a specific user to review your work, you can assign it to them by using r? <reviewer name> --> <!-- homu-ignore:end -->
2 parents 68ac5ab + ba39fdf commit 0e5f13e

File tree

3 files changed

+31
-9
lines changed

3 files changed

+31
-9
lines changed

compiler/rustc_borrowck/src/renumber.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ pub(crate) fn renumber_mir<'tcx>(
2121
let mut renumberer = RegionRenumberer { infcx };
2222

2323
for body in promoted.iter_mut() {
24-
renumberer.visit_body(body);
24+
renumberer.visit_body_preserves_cfg(body);
2525
}
2626

27-
renumberer.visit_body(body);
27+
renumberer.visit_body_preserves_cfg(body);
2828
}
2929

3030
// The fields are used only for debugging output in `sccs_info`.

compiler/rustc_middle/src/mir/basic_blocks.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::sync::OnceLock;
1+
use std::sync::{Arc, OnceLock};
22

33
use rustc_data_structures::fx::FxHashMap;
44
use rustc_data_structures::graph;
@@ -15,7 +15,8 @@ use crate::mir::{BasicBlock, BasicBlockData, START_BLOCK, Terminator, Terminator
1515
#[derive(Clone, TyEncodable, TyDecodable, Debug, HashStable, TypeFoldable, TypeVisitable)]
1616
pub struct BasicBlocks<'tcx> {
1717
basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>,
18-
cache: Cache,
18+
/// Use an `Arc` so we can share the cache when we clone the MIR body, as borrowck does.
19+
cache: Arc<Cache>,
1920
}
2021

2122
// Typically 95%+ of basic blocks have 4 or fewer predecessors.
@@ -49,9 +50,10 @@ struct Cache {
4950
impl<'tcx> BasicBlocks<'tcx> {
5051
#[inline]
5152
pub fn new(basic_blocks: IndexVec<BasicBlock, BasicBlockData<'tcx>>) -> Self {
52-
BasicBlocks { basic_blocks, cache: Cache::default() }
53+
BasicBlocks { basic_blocks, cache: Arc::new(Cache::default()) }
5354
}
5455

56+
#[inline]
5557
pub fn dominators(&self) -> &Dominators<BasicBlock> {
5658
self.cache.dominators.get_or_init(|| dominators(self))
5759
}
@@ -142,7 +144,14 @@ impl<'tcx> BasicBlocks<'tcx> {
142144
/// All other methods that allow you to mutate the basic blocks also call this method
143145
/// themselves, thereby avoiding any risk of accidentally cache invalidation.
144146
pub fn invalidate_cfg_cache(&mut self) {
145-
self.cache = Cache::default();
147+
if let Some(cache) = Arc::get_mut(&mut self.cache) {
148+
// If we only have a single reference to this cache, clear it.
149+
*cache = Cache::default();
150+
} else {
151+
// If we have several references to this cache, overwrite the pointer itself so other
152+
// users can continue to use their (valid) cache.
153+
self.cache = Arc::new(Cache::default());
154+
}
146155
}
147156
}
148157

compiler/rustc_mir_transform/src/simplify.rs

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ impl SimplifyCfg {
7474
}
7575

7676
pub(super) fn simplify_cfg<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
77-
CfgSimplifier::new(tcx, body).simplify();
77+
if CfgSimplifier::new(tcx, body).simplify() {
78+
// `simplify` returns that it changed something. We must invalidate the CFG caches as they
79+
// are not consistent with the modified CFG any more.
80+
body.basic_blocks.invalidate_cfg_cache();
81+
}
7882
remove_dead_blocks(body);
7983

8084
// FIXME: Should probably be moved into some kind of pass manager
@@ -121,19 +125,24 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
121125
// Preserve `SwitchInt` reads on built and analysis MIR, or if `-Zmir-preserve-ub`.
122126
let preserve_switch_reads = matches!(body.phase, MirPhase::Built | MirPhase::Analysis(_))
123127
|| tcx.sess.opts.unstable_opts.mir_preserve_ub;
124-
let basic_blocks = body.basic_blocks_mut();
128+
// Do not clear caches yet. The caller to `simplify` will do it if anything changed.
129+
let basic_blocks = body.basic_blocks.as_mut_preserves_cfg();
125130

126131
CfgSimplifier { preserve_switch_reads, basic_blocks, pred_count }
127132
}
128133

129-
fn simplify(mut self) {
134+
/// Returns whether we actually simplified anything. In that case, the caller *must* invalidate
135+
/// the CFG caches of the MIR body.
136+
#[must_use]
137+
fn simplify(mut self) -> bool {
130138
self.strip_nops();
131139

132140
// Vec of the blocks that should be merged. We store the indices here, instead of the
133141
// statements itself to avoid moving the (relatively) large statements twice.
134142
// We do not push the statements directly into the target block (`bb`) as that is slower
135143
// due to additional reallocations
136144
let mut merged_blocks = Vec::new();
145+
let mut outer_changed = false;
137146
loop {
138147
let mut changed = false;
139148

@@ -177,7 +186,11 @@ impl<'a, 'tcx> CfgSimplifier<'a, 'tcx> {
177186
if !changed {
178187
break;
179188
}
189+
190+
outer_changed = true;
180191
}
192+
193+
outer_changed
181194
}
182195

183196
/// This function will return `None` if

0 commit comments

Comments
 (0)