Skip to content

Commit

Permalink
Auto merge of #125916 - tesuji:mir-opt-const-array-locals, r=<try>
Browse files Browse the repository at this point in the history
[WIP] mir-opt: promoting const read-only arrays

Modified from a copy of PromoteTemps. It's kind of a hack so nothing fancy or easy to follow and review.
I'll  to reuse structures from PromoteTemps when there is [consensus for this pass][zulip].

Compiler is doing more work now with this opt. So I don't think this pass improves compiler performance.
But anyway, for statistics, can I get a perf run?

cc #73825

r? ghost

### Current status
- [ ] Waiting for [consensus][zulip].
- [ ] Maybe rewrite to [use GVN with mentor from oli][mentor]
- [x] ~ICE on unstable feature:  tests/assembly/simd-intrinsic-mask-load.rs#x86-avx512.~
  In particular `Simd([literal array])` now transformed to `Simd(array_var)`. Maybe I should ignore array in constructor.
- [x] *~Fail test on nested arrays~*

[zulip]: https://rust-lang.zulipchat.com/#narrow/stream/136281-t-opsem/topic/Could.20const.20read-only.20arrays.20be.20const.20promoted.3F
[mentor]: #125916 (comment)
  • Loading branch information
bors committed Jun 10, 2024
2 parents 06194ca + dca7207 commit 63ac52a
Show file tree
Hide file tree
Showing 6 changed files with 1,339 additions and 15 deletions.
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,
&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);
(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

0 comments on commit 63ac52a

Please sign in to comment.