Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] mir-opt: promoting const read-only arrays #125916

Closed
wants to merge 17 commits into from
13 changes: 11 additions & 2 deletions compiler/rustc_mir_transform/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ mod normalize_array_len;
mod nrvo;
mod prettify;
mod promote_consts;
mod promote_consts_local_arrays;
mod ref_prop;
mod remove_noop_landing_pads;
mod remove_storage_markers;
Expand Down Expand Up @@ -342,14 +343,22 @@ fn mir_promoted(

// What we need to run borrowck etc.
let promote_pass = promote_consts::PromoteTemps::default();
let promote_array = promote_consts_local_arrays::PromoteArraysOpt::default();
pm::run_passes(
tcx,
&mut body,
&[&promote_pass, &simplify::SimplifyCfg::PromoteConsts, &coverage::InstrumentCoverage],
&[
&promote_pass,
&promote_array,
Copy link
Contributor

Choose a reason for hiding this comment

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

since this is something that should not have user-visible effects (e.g. affecting dropck, const eval UB or borrowck), it should be run as part of the regular runtime optimization pipeline

&simplify::SimplifyCfg::PromoteConsts,
&coverage::InstrumentCoverage,
],
Some(MirPhase::Analysis(AnalysisPhase::Initial)),
);

let promoted = promote_pass.promoted_fragments.into_inner();
let mut promoted = promote_pass.promoted_fragments.into_inner();
let array_promoted = promote_array.promoted_fragments.into_inner();
promoted.extend(array_promoted);
Comment on lines +360 to +361
Copy link
Contributor

Choose a reason for hiding this comment

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

which does mean you won't be able to use the existing promotion scheme, but would need to start looking into create_def and query feeding, which is probably not ready to support this use case yet. I have not yet given it much thought what is needed to fully support that, but if you want we can look into this together.

Copy link
Contributor

Choose a reason for hiding this comment

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

then again, if all we're doing is creating non-generic static items, that already has precedent (we do that for nested statics), so likely you can do the same in an optimization.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though in that case I would expect this to fall out of GVN or some similar optimization, not be its own separate path

Copy link
Contributor

Choose a reason for hiding this comment

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

With GVN or similar you don't even need to create new constants and MIR bodies, you can just stick the fully evaluated constant into a MIR constant

(tcx.alloc_steal_mir(body), tcx.alloc_steal_promoted(promoted))
}

Expand Down
33 changes: 21 additions & 12 deletions compiler/rustc_mir_transform/src/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,33 +98,36 @@ struct Collector<'a, 'tcx> {
}

impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
#[instrument(level = "debug", skip(self))]
fn visit_local(&mut self, index: Local, context: PlaceContext, location: Location) {
debug!("visit_local: index={:?} context={:?} location={:?}", index, context, location);
// We're only interested in temporaries and the return place
match self.ccx.body.local_kind(index) {
LocalKind::Arg => return,
LocalKind::Temp if self.ccx.body.local_decls[index].is_user_variable() => return,
LocalKind::Temp
if {
let is_user_variable = self.ccx.body.local_decls[index].is_user_variable();
debug!(?is_user_variable);
is_user_variable
} =>
{
return;
}
LocalKind::ReturnPointer | LocalKind::Temp => {}
}

// Ignore drops, if the temp gets promoted,
// then it's constant and thus drop is noop.
// Non-uses are also irrelevant.
if context.is_drop() || !context.is_use() {
debug!(
"visit_local: context.is_drop={:?} context.is_use={:?}",
context.is_drop(),
context.is_use(),
);
debug!(is_drop = context.is_drop(), is_use = context.is_use());
return;
}

let temp = &mut self.temps[index];
debug!("visit_local: temp={:?}", temp);
debug!(?temp);
*temp = match *temp {
TempState::Undefined => match context {
PlaceContext::MutatingUse(MutatingUseContext::Store)
| PlaceContext::MutatingUse(MutatingUseContext::Call) => {
PlaceContext::MutatingUse(MutatingUseContext::Store | MutatingUseContext::Call) => {
TempState::Defined { location, uses: 0, valid: Err(()) }
}
_ => TempState::Unpromotable,
Expand All @@ -137,7 +140,7 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
| PlaceContext::NonMutatingUse(_) => true,
PlaceContext::MutatingUse(_) | PlaceContext::NonUse(_) => false,
};
debug!("visit_local: allowed_use={:?}", allowed_use);
debug!(?allowed_use);
if allowed_use {
*uses += 1;
return;
Expand All @@ -146,6 +149,7 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
}
TempState::Unpromotable | TempState::PromotedOut => TempState::Unpromotable,
};
debug!(?temp);
}

fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
Expand Down Expand Up @@ -972,7 +976,12 @@ fn promote_candidates<'tcx>(
candidates: Vec<Candidate>,
) -> IndexVec<Promoted, Body<'tcx>> {
// Visit candidates in reverse, in case they're nested.
debug!("promote_candidates({:?})", candidates);
debug!(promote_candidates = ?candidates);

// eagerly fail fast
if candidates.is_empty() {
return IndexVec::new();
}

let mut promotions = IndexVec::new();

Expand Down
Loading
Loading