Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
cc @cjgillot @tmiasko

This revert is based on the following report of a regression caused by this PR:

rust-lang#121094

In accordance with the compiler team [revert policy], PRs that cause meaningful
regressions should be reverted and re-landed once the regression has been fixed
(and a regression test has been added, where appropriate).
[revert policy]: https://forge.rust-lang.org/compiler/reviews.html#reverts

Fear not! Regressions happen. Please rest assured that this does not
represent a negative judgment of your contribution or ability to contribute
positively to Rust in the future. We simply want to prioritize keeping existing
use cases working, and keep the compiler more stable for everyone.

r? compiler
  • Loading branch information
erickt committed Feb 15, 2024
1 parent eaff1af commit 20e8d54
Show file tree
Hide file tree
Showing 27 changed files with 1,286 additions and 466 deletions.
128 changes: 128 additions & 0 deletions compiler/rustc_mir_transform/src/const_goto.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
//! This pass optimizes the following sequence
//! ```rust,ignore (example)
//! bb2: {
//! _2 = const true;
//! goto -> bb3;
//! }
//!
//! bb3: {
//! switchInt(_2) -> [false: bb4, otherwise: bb5];
//! }
//! ```
//! into
//! ```rust,ignore (example)
//! bb2: {
//! _2 = const true;
//! goto -> bb5;
//! }
//! ```

use rustc_middle::mir::*;
use rustc_middle::ty::TyCtxt;
use rustc_middle::{mir::visit::Visitor, ty::ParamEnv};

use super::simplify::{simplify_cfg, simplify_locals};

pub struct ConstGoto;

impl<'tcx> MirPass<'tcx> for ConstGoto {
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
// This pass participates in some as-of-yet untested unsoundness found
// in https://github.com/rust-lang/rust/issues/112460
sess.mir_opt_level() >= 2 && sess.opts.unstable_opts.unsound_mir_opts
}

fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
trace!("Running ConstGoto on {:?}", body.source);
let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());
let mut opt_finder =
ConstGotoOptimizationFinder { tcx, body, optimizations: vec![], param_env };
opt_finder.visit_body(body);
let should_simplify = !opt_finder.optimizations.is_empty();
for opt in opt_finder.optimizations {
let block = &mut body.basic_blocks_mut()[opt.bb_with_goto];
block.statements.extend(opt.stmts_move_up);
let terminator = block.terminator_mut();
let new_goto = TerminatorKind::Goto { target: opt.target_to_use_in_goto };
debug!("SUCCESS: replacing `{:?}` with `{:?}`", terminator.kind, new_goto);
terminator.kind = new_goto;
}

// if we applied optimizations, we potentially have some cfg to cleanup to
// make it easier for further passes
if should_simplify {
simplify_cfg(body);
simplify_locals(body, tcx);
}
}
}

impl<'tcx> Visitor<'tcx> for ConstGotoOptimizationFinder<'_, 'tcx> {
fn visit_basic_block_data(&mut self, block: BasicBlock, data: &BasicBlockData<'tcx>) {
if data.is_cleanup {
// Because of the restrictions around control flow in cleanup blocks, we don't perform
// this optimization at all in such blocks.
return;
}
self.super_basic_block_data(block, data);
}

fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, location: Location) {
let _: Option<_> = try {
let target = terminator.kind.as_goto()?;
// We only apply this optimization if the last statement is a const assignment
let last_statement = self.body.basic_blocks[location.block].statements.last()?;

if let (place, Rvalue::Use(Operand::Constant(_const))) =
last_statement.kind.as_assign()?
{
// We found a constant being assigned to `place`.
// Now check that the target of this Goto switches on this place.
let target_bb = &self.body.basic_blocks[target];

// The `StorageDead(..)` statement does not affect the functionality of mir.
// We can move this part of the statement up to the predecessor.
let mut stmts_move_up = Vec::new();
for stmt in &target_bb.statements {
if let StatementKind::StorageDead(..) = stmt.kind {
stmts_move_up.push(stmt.clone())
} else {
None?;
}
}

let target_bb_terminator = target_bb.terminator();
let (discr, targets) = target_bb_terminator.kind.as_switch()?;
if discr.place() == Some(*place) {
let switch_ty = place.ty(self.body.local_decls(), self.tcx).ty;
debug_assert_eq!(switch_ty, _const.ty());
// We now know that the Switch matches on the const place, and it is statementless
// Now find which value in the Switch matches the const value.
let const_value = _const.const_.try_eval_bits(self.tcx, self.param_env)?;
let target_to_use_in_goto = targets.target_for_value(const_value);
self.optimizations.push(OptimizationToApply {
bb_with_goto: location.block,
target_to_use_in_goto,
stmts_move_up,
});
}
}
Some(())
};

self.super_terminator(terminator, location);
}
}

struct OptimizationToApply<'tcx> {
bb_with_goto: BasicBlock,
target_to_use_in_goto: BasicBlock,
stmts_move_up: Vec<Statement<'tcx>>,
}

pub struct ConstGotoOptimizationFinder<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
body: &'a Body<'tcx>,
param_env: ParamEnv<'tcx>,
optimizations: Vec<OptimizationToApply<'tcx>>,
}
2 changes: 1 addition & 1 deletion compiler/rustc_mir_transform/src/jump_threading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const MAX_PLACES: usize = 100;

impl<'tcx> MirPass<'tcx> for JumpThreading {
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
sess.mir_opt_level() >= 2
sess.mir_opt_level() >= 4
}

#[instrument(skip_all level = "debug")]
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ mod remove_place_mention;
mod add_subtyping_projections;
pub mod cleanup_post_borrowck;
mod const_debuginfo;
mod const_goto;
mod const_prop;
mod const_prop_lint;
mod copy_prop;
Expand Down Expand Up @@ -102,6 +103,7 @@ mod remove_unneeded_drops;
mod remove_zsts;
mod required_consts;
mod reveal_all;
mod separate_const_switch;
mod shim;
mod ssa;
// This pass is public to allow external drivers to perform MIR cleanup
Expand Down Expand Up @@ -588,6 +590,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {

// Has to run after `slice::len` lowering
&normalize_array_len::NormalizeArrayLen,
&const_goto::ConstGoto,
&ref_prop::ReferencePropagation,
&sroa::ScalarReplacementOfAggregates,
&match_branches::MatchBranchSimplification,
Expand All @@ -598,6 +601,10 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
&dead_store_elimination::DeadStoreElimination::Initial,
&gvn::GVN,
&simplify::SimplifyLocals::AfterGVN,
// Perform `SeparateConstSwitch` after SSA-based analyses, as cloning blocks may
// destroy the SSA property. It should still happen before const-propagation, so the
// latter pass will leverage the created opportunities.
&separate_const_switch::SeparateConstSwitch,
&dataflow_const_prop::DataflowConstProp,
&const_debuginfo::ConstDebugInfo,
&o1(simplify_branches::SimplifyConstCondition::AfterConstProp),
Expand Down
Loading

0 comments on commit 20e8d54

Please sign in to comment.