Skip to content

Commit cb7f116

Browse files
committed
optimize promote_consts by cache the validate check
1 parent 4799baa commit cb7f116

File tree

1 file changed

+70
-41
lines changed

1 file changed

+70
-41
lines changed

compiler/rustc_const_eval/src/transform/promote_consts.rs

+70-41
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,14 +77,21 @@ 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: Valid },
8181
/// Any other combination of assignments/uses.
8282
Unpromotable,
8383
/// This temp was part of an rvalue which got extracted
8484
/// during promotion and needs cleanup.
8585
PromotedOut,
8686
}
8787

88+
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
89+
pub enum Valid {
90+
Unknown,
91+
InValid,
92+
Validated,
93+
}
94+
8895
impl TempState {
8996
pub fn is_promotable(&self) -> bool {
9097
debug!("is_promotable: self={:?}", self);
@@ -133,7 +140,7 @@ impl<'tcx> Visitor<'tcx> for Collector<'_, 'tcx> {
133140
match context {
134141
PlaceContext::MutatingUse(MutatingUseContext::Store)
135142
| PlaceContext::MutatingUse(MutatingUseContext::Call) => {
136-
*temp = TempState::Defined { location, uses: 0 };
143+
*temp = TempState::Defined { location, uses: 0, valid: Valid::Unknown };
137144
return;
138145
}
139146
_ => { /* mark as unpromotable below */ }
@@ -188,7 +195,7 @@ pub fn collect_temps_and_candidates<'tcx>(
188195
/// This wraps an `Item`, and has access to all fields of that `Item` via `Deref` coercion.
189196
struct Validator<'a, 'tcx> {
190197
ccx: &'a ConstCx<'a, 'tcx>,
191-
temps: &'a IndexVec<Local, TempState>,
198+
temps: &'a mut IndexVec<Local, TempState>,
192199
}
193200

194201
impl<'a, 'tcx> std::ops::Deref for Validator<'a, 'tcx> {
@@ -202,7 +209,7 @@ impl<'a, 'tcx> std::ops::Deref for Validator<'a, 'tcx> {
202209
struct Unpromotable;
203210

204211
impl<'tcx> Validator<'_, 'tcx> {
205-
fn validate_candidate(&self, candidate: Candidate) -> Result<(), Unpromotable> {
212+
fn validate_candidate(&mut self, candidate: Candidate) -> Result<(), Unpromotable> {
206213
let loc = candidate.location;
207214
let statement = &self.body[loc.block].statements[loc.statement_index];
208215
match &statement.kind {
@@ -234,7 +241,7 @@ impl<'tcx> Validator<'_, 'tcx> {
234241
}
235242

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

@@ -272,40 +279,62 @@ impl<'tcx> Validator<'_, 'tcx> {
272279
}
273280
}
274281

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);
300-
}
282+
fn validate_local(&mut self, local: Local) -> Result<(), Unpromotable> {
283+
if let TempState::Defined { location: loc, uses, valid } = self.temps[local] {
284+
match valid {
285+
Valid::InValid => Err(Unpromotable),
286+
Valid::Validated => Ok(()),
287+
Valid::Unknown => {
288+
let ok = {
289+
let block = &self.body[loc.block];
290+
let num_stmts = block.statements.len();
291+
292+
if loc.statement_index < num_stmts {
293+
let statement = &block.statements[loc.statement_index];
294+
match &statement.kind {
295+
StatementKind::Assign(box (_, rhs)) => self.validate_rvalue(rhs),
296+
_ => {
297+
span_bug!(
298+
statement.source_info.span,
299+
"{:?} is not an assignment",
300+
statement
301+
);
302+
}
303+
}
304+
} else {
305+
let terminator = block.terminator();
306+
match &terminator.kind {
307+
TerminatorKind::Call { func, args, .. } => {
308+
self.validate_call(func, args)
309+
}
310+
TerminatorKind::Yield { .. } => Err(Unpromotable),
311+
kind => {
312+
span_bug!(
313+
terminator.source_info.span,
314+
"{:?} not promotable",
315+
kind
316+
);
317+
}
318+
}
319+
}
320+
};
321+
self.temps[local] = TempState::Defined {
322+
location: loc,
323+
uses,
324+
valid: match ok {
325+
Ok(()) => Valid::Validated,
326+
Err(_) => Valid::InValid,
327+
},
328+
};
329+
ok
301330
}
302331
}
303332
} else {
304333
Err(Unpromotable)
305334
}
306335
}
307336

308-
fn validate_place(&self, place: PlaceRef<'tcx>) -> Result<(), Unpromotable> {
337+
fn validate_place(&mut self, place: PlaceRef<'tcx>) -> Result<(), Unpromotable> {
309338
match place.last_projection() {
310339
None => self.validate_local(place.local),
311340
Some((place_base, elem)) => {
@@ -417,7 +446,7 @@ impl<'tcx> Validator<'_, 'tcx> {
417446
}
418447
}
419448

420-
fn validate_operand(&self, operand: &Operand<'tcx>) -> Result<(), Unpromotable> {
449+
fn validate_operand(&mut self, operand: &Operand<'tcx>) -> Result<(), Unpromotable> {
421450
match operand {
422451
Operand::Copy(place) | Operand::Move(place) => self.validate_place(place.as_ref()),
423452

@@ -447,7 +476,7 @@ impl<'tcx> Validator<'_, 'tcx> {
447476
}
448477
}
449478

450-
fn validate_ref(&self, kind: BorrowKind, place: &Place<'tcx>) -> Result<(), Unpromotable> {
479+
fn validate_ref(&mut self, kind: BorrowKind, place: &Place<'tcx>) -> Result<(), Unpromotable> {
451480
match kind {
452481
// Reject these borrow types just to be safe.
453482
// FIXME(RalfJung): could we allow them? Should we? No point in it until we have a usecase.
@@ -480,7 +509,7 @@ impl<'tcx> Validator<'_, 'tcx> {
480509
Ok(())
481510
}
482511

483-
fn validate_rvalue(&self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> {
512+
fn validate_rvalue(&mut self, rvalue: &Rvalue<'tcx>) -> Result<(), Unpromotable> {
484513
match rvalue {
485514
Rvalue::Use(operand) | Rvalue::Repeat(operand, _) => {
486515
self.validate_operand(operand)?;
@@ -623,7 +652,7 @@ impl<'tcx> Validator<'_, 'tcx> {
623652
}
624653

625654
fn validate_call(
626-
&self,
655+
&mut self,
627656
callee: &Operand<'tcx>,
628657
args: &[Operand<'tcx>],
629658
) -> Result<(), Unpromotable> {
@@ -665,10 +694,10 @@ impl<'tcx> Validator<'_, 'tcx> {
665694
// FIXME(eddyb) remove the differences for promotability in `static`, `const`, `const fn`.
666695
pub fn validate_candidates(
667696
ccx: &ConstCx<'_, '_>,
668-
temps: &IndexVec<Local, TempState>,
697+
temps: &mut IndexVec<Local, TempState>,
669698
candidates: &[Candidate],
670699
) -> Vec<Candidate> {
671-
let validator = Validator { ccx, temps };
700+
let mut validator = Validator { ccx, temps };
672701

673702
candidates
674703
.iter()
@@ -720,7 +749,7 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
720749
fn promote_temp(&mut self, temp: Local) -> Local {
721750
let old_keep_original = self.keep_original;
722751
let loc = match self.temps[temp] {
723-
TempState::Defined { location, uses } if uses > 0 => {
752+
TempState::Defined { location, uses, .. } if uses > 0 => {
724753
if uses > 1 {
725754
self.keep_original = true;
726755
}

0 commit comments

Comments
 (0)