From c16336a014176e82f1bcbba84f99a228b4a321f5 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 20 Sep 2018 11:57:45 +0200 Subject: [PATCH] move loop detector constants to the module that uses them; make lifetime order in ConstPropagator consistent with Memory --- src/librustc_mir/const_eval.rs | 20 +++++++++---------- src/librustc_mir/interpret/snapshot.rs | 25 ++++++++++++------------ src/librustc_mir/transform/const_prop.rs | 12 ++++++------ 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index e2a5d40a1236..bde0e95d372f 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -34,6 +34,13 @@ use interpret::{self, snapshot, }; +/// Number of steps until the detector even starts doing anything. +/// Also, a warning is shown to the user when this number is reached. +const STEPS_UNTIL_DETECTOR_ENABLED: isize = 1_000_000; +/// The number of steps between loop detector snapshots. +/// Should be a power of two for performance reasons. +const DETECTOR_SNAPSHOT_PERIOD: isize = 256; + pub fn mk_borrowck_eval_cx<'a, 'mir, 'tcx>( tcx: TyCtxt<'a, 'tcx, 'tcx>, instance: Instance<'tcx>, @@ -245,7 +252,7 @@ impl<'a, 'mir, 'tcx> CompileTimeInterpreter<'a, 'mir, 'tcx> { fn new() -> Self { CompileTimeInterpreter { loop_detector: Default::default(), - steps_since_detector_enabled: -snapshot::STEPS_UNTIL_DETECTOR_ENABLED, + steps_since_detector_enabled: -STEPS_UNTIL_DETECTOR_ENABLED, } } } @@ -349,22 +356,15 @@ impl<'a, 'mir, 'tcx> interpret::Machine<'a, 'mir, 'tcx> return Ok(()); } - *steps %= snapshot::DETECTOR_SNAPSHOT_PERIOD; + *steps %= DETECTOR_SNAPSHOT_PERIOD; if *steps != 0 { return Ok(()); } } - if ecx.machine.loop_detector.is_empty() { - // First run of the loop detector - - // FIXME(#49980): make this warning a lint - ecx.tcx.sess.span_warn(ecx.frame().span, - "Constant evaluating a complex constant, this might take some time"); - } - ecx.machine.loop_detector.observe_and_analyze( &ecx.tcx, + ecx.frame().span, &ecx.memory, &ecx.stack[..], ) diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index 388e0fd859d6..938f2e6db1cd 100644 --- a/src/librustc_mir/interpret/snapshot.rs +++ b/src/librustc_mir/interpret/snapshot.rs @@ -28,13 +28,6 @@ use super::eval_context::{LocalValue, StackPopCleanup}; use super::{Frame, Memory, Operand, MemPlace, Place, Value}; use const_eval::CompileTimeInterpreter; -/// Number of steps until the detector even starts doing anything. -/// Also, a warning is shown to the user when this number is reached. -pub(crate) const STEPS_UNTIL_DETECTOR_ENABLED: isize = 1_000_000; -/// The number of steps between loop detector snapshots. -/// Should be a power of two for performance reasons. -pub(crate) const DETECTOR_SNAPSHOT_PERIOD: isize = 256; - #[derive(Default)] pub(crate) struct InfiniteLoopDetector<'a, 'mir, 'tcx: 'a + 'mir> { /// The set of all `EvalSnapshot` *hashes* observed by this detector. @@ -53,28 +46,31 @@ pub(crate) struct InfiniteLoopDetector<'a, 'mir, 'tcx: 'a + 'mir> { impl<'a, 'mir, 'tcx> InfiniteLoopDetector<'a, 'mir, 'tcx> { - /// Returns `true` if the loop detector has not yet observed a snapshot. - pub fn is_empty(&self) -> bool { - self.hashes.is_empty() - } - pub fn observe_and_analyze<'b>( &mut self, tcx: &TyCtxt<'b, 'tcx, 'tcx>, + span: Span, memory: &Memory<'a, 'mir, 'tcx, CompileTimeInterpreter<'a, 'mir, 'tcx>>, stack: &[Frame<'mir, 'tcx>], ) -> EvalResult<'tcx, ()> { - + // Compute stack's hash before copying anything let mut hcx = tcx.get_stable_hashing_context(); let mut hasher = StableHasher::::new(); stack.hash_stable(&mut hcx, &mut hasher); let hash = hasher.finish(); + // Check if we know that hash already + if self.hashes.is_empty() { + // FIXME(#49980): make this warning a lint + tcx.sess.span_warn(span, + "Constant evaluating a complex constant, this might take some time"); + } if self.hashes.insert(hash) { // No collision return Ok(()) } + // We need to make a full copy. NOW things that to get really expensive. info!("snapshotting the state of the interpreter"); if self.snapshots.insert(EvalSnapshot::new(memory, stack)) { @@ -461,6 +457,9 @@ impl<'a, 'mir, 'tcx> Eq for EvalSnapshot<'a, 'mir, 'tcx> impl<'a, 'mir, 'tcx> PartialEq for EvalSnapshot<'a, 'mir, 'tcx> { fn eq(&self, other: &Self) -> bool { + // FIXME: This looks to be a *ridicolously expensive* comparison operation. + // Doesn't this make tons of copies? Either `snapshot` is very badly named, + // or it does! self.snapshot() == other.snapshot() } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 7cfa7de8138a..e2a6cee05497 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -68,9 +68,9 @@ impl MirPass for ConstProp { type Const<'tcx> = (OpTy<'tcx>, Span); /// Finds optimization opportunities on the MIR. -struct ConstPropagator<'b, 'a, 'tcx:'a+'b> { - ecx: EvalContext<'a, 'b, 'tcx, CompileTimeInterpreter<'a, 'b, 'tcx>>, - mir: &'b Mir<'tcx>, +struct ConstPropagator<'a, 'mir, 'tcx:'a+'mir> { + ecx: EvalContext<'a, 'mir, 'tcx, CompileTimeInterpreter<'a, 'mir, 'tcx>>, + mir: &'mir Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>, source: MirSource, places: IndexVec>>, @@ -101,12 +101,12 @@ impl<'a, 'b, 'tcx> HasTyCtxt<'tcx> for &'a ConstPropagator<'a, 'b, 'tcx> { } } -impl<'b, 'a, 'tcx:'b> ConstPropagator<'b, 'a, 'tcx> { +impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { fn new( - mir: &'b Mir<'tcx>, + mir: &'mir Mir<'tcx>, tcx: TyCtxt<'a, 'tcx, 'tcx>, source: MirSource, - ) -> ConstPropagator<'b, 'a, 'tcx> { + ) -> ConstPropagator<'a, 'mir, 'tcx> { let param_env = tcx.param_env(source.def_id); let substs = Substs::identity_for_item(tcx, source.def_id); let instance = Instance::new(source.def_id, substs);