Skip to content

Commit e013f9e

Browse files
committed
Auto merge of #96815 - SparrowLii:promote_const, r=oli-obk
optimize `promote_consts` by caching the results of `validate_local` From the FIXME in the impl of `promote_consts`. Early return the `validate_local` should save some compile time. `qualif_local` is similar to this, but requires futher changing because there are different types of qualif checks. If this PR is effective, I will do it as well.
2 parents 8a2fe75 + b890037 commit e013f9e

File tree

1 file changed

+52
-42
lines changed

1 file changed

+52
-42
lines changed

compiler/rustc_const_eval/src/transform/promote_consts.rs

+52-42
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ impl<'tcx> MirPass<'tcx> for PromoteTemps<'tcx> {
6060

6161
let mut rpo = traversal::reverse_postorder(body);
6262
let ccx = ConstCx::new(tcx, body);
63-
let (temps, all_candidates) = collect_temps_and_candidates(&ccx, &mut rpo);
63+
let (mut temps, all_candidates) = collect_temps_and_candidates(&ccx, &mut rpo);
6464

65-
let promotable_candidates = validate_candidates(&ccx, &temps, &all_candidates);
65+
let promotable_candidates = validate_candidates(&ccx, &mut temps, &all_candidates);
6666

6767
let promoted = promote_candidates(body, tcx, temps, promotable_candidates);
6868
self.promoted_fragments.set(promoted);
@@ -77,7 +77,7 @@ pub enum TempState {
7777
/// One direct assignment and any number of direct uses.
7878
/// A borrow of this temp is promotable if the assigned
7979
/// value is qualified as constant.
80-
Defined { location: Location, uses: usize },
80+
Defined { location: Location, uses: usize, valid: Result<(), ()> },
8181
/// Any other combination of assignments/uses.
8282
Unpromotable,
8383
/// This temp was part of an rvalue which got extracted
@@ -133,7 +133,7 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
133133
match context {
134134
PlaceContext::MutatingUse(MutatingUseContext::Store)
135135
| PlaceContext::MutatingUse(MutatingUseContext::Call) => {
136-
*temp = TempState::Defined { location, uses: 0 };
136+
*temp = TempState::Defined { location, uses: 0, valid: Err(()) };
137137
return;
138138
}
139139
_ => { /* mark as unpromotable below */ }
@@ -188,7 +188,7 @@ pub fn collect_temps_and_candidates<'tcx>(
188188
/// This wraps an `Item`, and has access to all fields of that `Item` via `Deref` coercion.
189189
struct Validator<'a, 'tcx> {
190190
ccx: &'a ConstCx<'a, 'tcx>,
191-
temps: &'a IndexVec<Local, TempState>,
191+
temps: &'a mut IndexVec<Local, TempState>,
192192
}
193193

194194
impl<'a, 'tcx> std::ops::Deref for Validator<'a, 'tcx> {
@@ -202,7 +202,7 @@ impl<'a, 'tcx> std::ops::Deref for Validator<'a, 'tcx> {
202202
struct Unpromotable;
203203

204204
impl<'tcx> Validator<'_, 'tcx> {
205-
fn validate_candidate(&self, candidate: Candidate) -> Result<(), Unpromotable> {
205+
fn validate_candidate(&mut self, candidate: Candidate) -> Result<(), Unpromotable> {
206206
let loc = candidate.location;
207207
let statement = &self.body[loc.block].statements[loc.statement_index];
208208
match &statement.kind {
@@ -234,7 +234,7 @@ impl<'tcx> Validator<'_, 'tcx> {
234234
}
235235

236236
// FIXME(eddyb) maybe cache this?
237-
fn qualif_local<Q: qualifs::Qualif>(&self, local: Local) -> bool {
237+
fn qualif_local<Q: qualifs::Qualif>(&mut self, local: Local) -> bool {
238238
if let TempState::Defined { location: loc, .. } = self.temps[local] {
239239
let num_stmts = self.body[loc.block].statements.len();
240240

@@ -272,40 +272,50 @@ impl<'tcx> Validator<'_, 'tcx> {
272272
}
273273
}
274274

275-
// FIXME(eddyb) maybe cache this?
276-
fn validate_local(&self, local: Local) -> Result<(), Unpromotable> {
277-
if let TempState::Defined { location: loc, .. } = self.temps[local] {
278-
let block = &self.body[loc.block];
279-
let num_stmts = block.statements.len();
280-
281-
if loc.statement_index < num_stmts {
282-
let statement = &block.statements[loc.statement_index];
283-
match &statement.kind {
284-
StatementKind::Assign(box (_, rhs)) => self.validate_rvalue(rhs),
285-
_ => {
286-
span_bug!(
287-
statement.source_info.span,
288-
"{:?} is not an assignment",
289-
statement
290-
);
291-
}
292-
}
293-
} else {
294-
let terminator = block.terminator();
295-
match &terminator.kind {
296-
TerminatorKind::Call { func, args, .. } => self.validate_call(func, args),
297-
TerminatorKind::Yield { .. } => Err(Unpromotable),
298-
kind => {
299-
span_bug!(terminator.source_info.span, "{:?} not promotable", kind);
275+
fn validate_local(&mut self, local: Local) -> Result<(), Unpromotable> {
276+
if let TempState::Defined { location: loc, uses, valid } = self.temps[local] {
277+
valid.or_else(|_| {
278+
let ok = {
279+
let block = &self.body[loc.block];
280+
let num_stmts = block.statements.len();
281+
282+
if loc.statement_index < num_stmts {
283+
let statement = &block.statements[loc.statement_index];
284+
match &statement.kind {
285+
StatementKind::Assign(box (_, rhs)) => self.validate_rvalue(rhs),
286+
_ => {
287+
span_bug!(
288+
statement.source_info.span,
289+
"{:?} is not an assignment",
290+
statement
291+
);
292+
}
293+
}
294+
} else {
295+
let terminator = block.terminator();
296+
match &terminator.kind {
297+
TerminatorKind::Call { func, args, .. } => {
298+
self.validate_call(func, args)
299+
}
300+
TerminatorKind::Yield { .. } => Err(Unpromotable),
301+
kind => {
302+
span_bug!(terminator.source_info.span, "{:?} not promotable", kind);
303+
}
304+
}
300305
}
301-
}
302-
}
306+
};
307+
self.temps[local] = match ok {
308+
Ok(()) => TempState::Defined { location: loc, uses, valid: Ok(()) },
309+
Err(_) => TempState::Unpromotable,
310+
};
311+
ok
312+
})
303313
} else {
304314
Err(Unpromotable)
305315
}
306316
}
307317

308-
fn validate_place(&self, place: PlaceRef<'tcx>) -> Result<(), Unpromotable> {
318+
fn validate_place(&mut self, place: PlaceRef<'tcx>) -> Result<(), Unpromotable> {
309319
match place.last_projection() {
310320
None => self.validate_local(place.local),
311321
Some((place_base, elem)) => {
@@ -417,7 +427,7 @@ impl<'tcx> Validator<'_, 'tcx> {
417427
}
418428
}
419429

420-
fn validate_operand(&self, operand: &Operand<'tcx>) -> Result<(), Unpromotable> {
430+
fn validate_operand(&mut self, operand: &Operand<'tcx>) -> Result<(), Unpromotable> {
421431
match operand {
422432
Operand::Copy(place) | Operand::Move(place) => self.validate_place(place.as_ref()),
423433

@@ -447,7 +457,7 @@ impl<'tcx> Validator<'_, 'tcx> {
447457
}
448458
}
449459

450-
fn validate_ref(&self, kind: BorrowKind, place: &Place<'tcx>) -> Result<(), Unpromotable> {
460+
fn validate_ref(&mut self, kind: BorrowKind, place: &Place<'tcx>) -> Result<(), Unpromotable> {
451461
match kind {
452462
// Reject these borrow types just to be safe.
453463
// FIXME(RalfJung): could we allow them? Should we? No point in it until we have a usecase.
@@ -480,7 +490,7 @@ impl<'tcx> Validator<'_, 'tcx> {
480490
Ok(())
481491
}
482492

483-
fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> {
493+
fn validate_rvalue(&mut self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> {
484494
match rvalue {
485495
Rvalue::Use(operand) | Rvalue::Repeat(operand, _) => {
486496
self.validate_operand(operand)?;
@@ -623,7 +633,7 @@ impl<'tcx> Validator<'_, 'tcx> {
623633
}
624634

625635
fn validate_call(
626-
&self,
636+
&mut self,
627637
callee: &Operand<'tcx>,
628638
args: &[Operand<'tcx>],
629639
) -> Result<(), Unpromotable> {
@@ -665,10 +675,10 @@ impl<'tcx> Validator<'_, 'tcx> {
665675
// FIXME(eddyb) remove the differences for promotability in `static`, `const`, `const fn`.
666676
pub fn validate_candidates(
667677
ccx: &ConstCx<'_, '_>,
668-
temps: &IndexVec<Local, TempState>,
678+
temps: &mut IndexVec<Local, TempState>,
669679
candidates: &[Candidate],
670680
) -> Vec<Candidate> {
671-
let validator = Validator { ccx, temps };
681+
let mut validator = Validator { ccx, temps };
672682

673683
candidates
674684
.iter()
@@ -720,7 +730,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
720730
fn promote_temp(&mut self, temp: Local) -> Local {
721731
let old_keep_original = self.keep_original;
722732
let loc = match self.temps[temp] {
723-
TempState::Defined { location, uses } if uses > 0 => {
733+
TempState::Defined { location, uses, .. } if uses > 0 => {
724734
if uses > 1 {
725735
self.keep_original = true;
726736
}

0 commit comments

Comments
 (0)