Skip to content

Commit

Permalink
Rollup merge of #73130 - wesleywiser:remove_const_prop_for_indirects,…
Browse files Browse the repository at this point in the history
… r=oli-obk

Remove const prop for indirects

This was only used by one mir-opt test and since it causes buggy behavior under `-Zmir-opt-level=2`, it seems like we should remove it.

This was split out from #71946.

Closes #72679
Closes #72372
Closes #72285
  • Loading branch information
RalfJung authored Jun 19, 2020
2 parents ea3c309 + 1e88f13 commit 098949b
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 96 deletions.
12 changes: 6 additions & 6 deletions src/librustc_mir/interpret/intern.rs
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,6 @@ pub enum InternKind {
Static(hir::Mutability),
Constant,
Promoted,
ConstProp,
}

/// Intern `ret` and everything it references.
Expand All @@ -314,9 +313,7 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
let base_intern_mode = match intern_kind {
InternKind::Static(mutbl) => InternMode::Static(mutbl),
// FIXME: what about array lengths, array initializers?
InternKind::Constant | InternKind::ConstProp | InternKind::Promoted => {
InternMode::ConstBase
}
InternKind::Constant | InternKind::Promoted => InternMode::ConstBase,
};

// Type based interning.
Expand Down Expand Up @@ -358,7 +355,10 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
Err(error) => {
ecx.tcx.sess.delay_span_bug(
ecx.tcx.span,
"error during interning should later cause validation failure",
&format!(
"error during interning should later cause validation failure: {}",
error
),
);
// Some errors shouldn't come up because creating them causes
// an allocation, which we should avoid. When that happens,
Expand Down Expand Up @@ -399,7 +399,7 @@ pub fn intern_const_alloc_recursive<M: CompileTimeMachine<'mir, 'tcx>>(
// immutability is so important.
alloc.mutability = Mutability::Not;
}
InternKind::Constant | InternKind::ConstProp => {
InternKind::Constant => {
// If it's a constant, we should not have any "leftovers" as everything
// is tracked by const-checking.
// FIXME: downgrade this to a warning? It rejects some legitimate consts,
Expand Down
11 changes: 3 additions & 8 deletions src/librustc_mir/transform/const_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ use rustc_trait_selection::traits;

use crate::const_eval::error_to_const_error;
use crate::interpret::{
self, compile_time_machine, intern_const_alloc_recursive, AllocId, Allocation, Frame, ImmTy,
Immediate, InternKind, InterpCx, LocalState, LocalValue, Memory, MemoryKind, OpTy,
Operand as InterpOperand, PlaceTy, Pointer, ScalarMaybeUninit, StackPopCleanup,
self, compile_time_machine, AllocId, Allocation, Frame, ImmTy, Immediate, InterpCx, LocalState,
LocalValue, Memory, MemoryKind, OpTy, Operand as InterpOperand, PlaceTy, Pointer,
ScalarMaybeUninit, StackPopCleanup,
};
use crate::transform::{MirPass, MirSource};

Expand Down Expand Up @@ -702,11 +702,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
ScalarMaybeUninit::Scalar(l),
ScalarMaybeUninit::Scalar(r),
)) => l.is_bits() && r.is_bits(),
interpret::Operand::Indirect(_) if mir_opt_level >= 2 => {
let mplace = op.assert_mem_place(&self.ecx);
intern_const_alloc_recursive(&mut self.ecx, InternKind::ConstProp, mplace, false);
true
}
_ => false,
}
}
Expand Down
5 changes: 5 additions & 0 deletions src/test/mir-opt/const_prop/discriminant.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
// compile-flags: -O

// FIXME(wesleywiser): Ideally, we could const-prop away all of this and just be left with
// `let x = 42` but that doesn't work because const-prop doesn't support `Operand::Indirect`
// and `InterpCx::eval_place()` always forces an allocation which creates the `Indirect`.
// Fixing either of those will allow us to const-prop this away.

// EMIT_MIR_FOR_EACH_BIT_WIDTH
// EMIT_MIR rustc.main.ConstProp.diff
fn main() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,100 +2,93 @@
+ // MIR for `main` after ConstProp

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/discriminant.rs:5:11: 5:11
let _1: i32; // in scope 0 at $DIR/discriminant.rs:6:9: 6:10
let mut _2: i32; // in scope 0 at $DIR/discriminant.rs:6:13: 6:64
let mut _3: std::option::Option<bool>; // in scope 0 at $DIR/discriminant.rs:6:34: 6:44
let mut _4: isize; // in scope 0 at $DIR/discriminant.rs:6:21: 6:31
let mut _0: (); // return place in scope 0 at $DIR/discriminant.rs:10:11: 10:11
let _1: i32; // in scope 0 at $DIR/discriminant.rs:11:9: 11:10
let mut _2: i32; // in scope 0 at $DIR/discriminant.rs:11:13: 11:64
let mut _3: std::option::Option<bool>; // in scope 0 at $DIR/discriminant.rs:11:34: 11:44
let mut _4: isize; // in scope 0 at $DIR/discriminant.rs:11:21: 11:31
scope 1 {
debug x => _1; // in scope 1 at $DIR/discriminant.rs:6:9: 6:10
debug x => _1; // in scope 1 at $DIR/discriminant.rs:11:9: 11:10
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/discriminant.rs:6:9: 6:10
StorageLive(_2); // scope 0 at $DIR/discriminant.rs:6:13: 6:64
StorageLive(_3); // scope 0 at $DIR/discriminant.rs:6:34: 6:44
- _3 = std::option::Option::<bool>::Some(const true); // scope 0 at $DIR/discriminant.rs:6:34: 6:44
+ _3 = const std::option::Option::<bool>::Some(true); // scope 0 at $DIR/discriminant.rs:6:34: 6:44
StorageLive(_1); // scope 0 at $DIR/discriminant.rs:11:9: 11:10
StorageLive(_2); // scope 0 at $DIR/discriminant.rs:11:13: 11:64
StorageLive(_3); // scope 0 at $DIR/discriminant.rs:11:34: 11:44
- _3 = std::option::Option::<bool>::Some(const true); // scope 0 at $DIR/discriminant.rs:11:34: 11:44
+ _3 = const std::option::Option::<bool>::Some(true); // scope 0 at $DIR/discriminant.rs:11:34: 11:44
// ty::Const
- // + ty: bool
+ // + ty: std::option::Option<bool>
// + val: Value(Scalar(0x01))
// mir::Constant
- // + span: $DIR/discriminant.rs:6:39: 6:43
- // + span: $DIR/discriminant.rs:11:39: 11:43
- // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
- _4 = discriminant(_3); // scope 0 at $DIR/discriminant.rs:6:21: 6:31
- switchInt(move _4) -> [1isize: bb2, otherwise: bb1]; // scope 0 at $DIR/discriminant.rs:6:21: 6:31
+ // + span: $DIR/discriminant.rs:6:34: 6:44
- _4 = discriminant(_3); // scope 0 at $DIR/discriminant.rs:11:21: 11:31
- switchInt(move _4) -> [1isize: bb2, otherwise: bb1]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+ // + span: $DIR/discriminant.rs:11:34: 11:44
+ // + literal: Const { ty: std::option::Option<bool>, val: Value(Scalar(0x01)) }
+ _4 = const 1isize; // scope 0 at $DIR/discriminant.rs:6:21: 6:31
+ _4 = const 1isize; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+ // ty::Const
+ // + ty: isize
+ // + val: Value(Scalar(0x00000001))
+ // mir::Constant
+ // + span: $DIR/discriminant.rs:6:21: 6:31
+ // + span: $DIR/discriminant.rs:11:21: 11:31
+ // + literal: Const { ty: isize, val: Value(Scalar(0x00000001)) }
+ switchInt(const 1isize) -> [1isize: bb2, otherwise: bb1]; // scope 0 at $DIR/discriminant.rs:6:21: 6:31
+ switchInt(const 1isize) -> [1isize: bb2, otherwise: bb1]; // scope 0 at $DIR/discriminant.rs:11:21: 11:31
+ // ty::Const
+ // + ty: isize
+ // + val: Value(Scalar(0x00000001))
+ // mir::Constant
+ // + span: $DIR/discriminant.rs:6:21: 6:31
+ // + span: $DIR/discriminant.rs:11:21: 11:31
+ // + literal: Const { ty: isize, val: Value(Scalar(0x00000001)) }
}

bb1: {
_2 = const 10i32; // scope 0 at $DIR/discriminant.rs:6:59: 6:61
_2 = const 10i32; // scope 0 at $DIR/discriminant.rs:11:59: 11:61
// ty::Const
// + ty: i32
// + val: Value(Scalar(0x0000000a))
// mir::Constant
// + span: $DIR/discriminant.rs:6:59: 6:61
// + span: $DIR/discriminant.rs:11:59: 11:61
// + literal: Const { ty: i32, val: Value(Scalar(0x0000000a)) }
goto -> bb4; // scope 0 at $DIR/discriminant.rs:6:13: 6:64
goto -> bb4; // scope 0 at $DIR/discriminant.rs:11:13: 11:64
}

bb2: {
- switchInt(((_3 as Some).0: bool)) -> [false: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:6:26: 6:30
+ switchInt(const true) -> [false: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:6:26: 6:30
+ // ty::Const
+ // + ty: bool
+ // + val: Value(Scalar(0x01))
+ // mir::Constant
+ // + span: $DIR/discriminant.rs:6:26: 6:30
+ // + literal: Const { ty: bool, val: Value(Scalar(0x01)) }
switchInt(((_3 as Some).0: bool)) -> [false: bb1, otherwise: bb3]; // scope 0 at $DIR/discriminant.rs:11:26: 11:30
}

bb3: {
_2 = const 42i32; // scope 0 at $DIR/discriminant.rs:6:47: 6:49
_2 = const 42i32; // scope 0 at $DIR/discriminant.rs:11:47: 11:49
// ty::Const
// + ty: i32
// + val: Value(Scalar(0x0000002a))
// mir::Constant
// + span: $DIR/discriminant.rs:6:47: 6:49
// + span: $DIR/discriminant.rs:11:47: 11:49
// + literal: Const { ty: i32, val: Value(Scalar(0x0000002a)) }
goto -> bb4; // scope 0 at $DIR/discriminant.rs:6:13: 6:64
goto -> bb4; // scope 0 at $DIR/discriminant.rs:11:13: 11:64
}

bb4: {
_1 = Add(move _2, const 0i32); // scope 0 at $DIR/discriminant.rs:6:13: 6:68
_1 = Add(move _2, const 0i32); // scope 0 at $DIR/discriminant.rs:11:13: 11:68
// ty::Const
// + ty: i32
// + val: Value(Scalar(0x00000000))
// mir::Constant
// + span: $DIR/discriminant.rs:6:67: 6:68
// + span: $DIR/discriminant.rs:11:67: 11:68
// + literal: Const { ty: i32, val: Value(Scalar(0x00000000)) }
StorageDead(_2); // scope 0 at $DIR/discriminant.rs:6:67: 6:68
StorageDead(_3); // scope 0 at $DIR/discriminant.rs:6:68: 6:69
_0 = const (); // scope 0 at $DIR/discriminant.rs:5:11: 7:2
StorageDead(_2); // scope 0 at $DIR/discriminant.rs:11:67: 11:68
StorageDead(_3); // scope 0 at $DIR/discriminant.rs:11:68: 11:69
_0 = const (); // scope 0 at $DIR/discriminant.rs:10:11: 12:2
// ty::Const
// + ty: ()
// + val: Value(Scalar(<ZST>))
// mir::Constant
// + span: $DIR/discriminant.rs:5:11: 7:2
// + span: $DIR/discriminant.rs:10:11: 12:2
// + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
StorageDead(_1); // scope 0 at $DIR/discriminant.rs:7:1: 7:2
return; // scope 0 at $DIR/discriminant.rs:7:2: 7:2
StorageDead(_1); // scope 0 at $DIR/discriminant.rs:12:1: 12:2
return; // scope 0 at $DIR/discriminant.rs:12:2: 12:2
}
}

Loading

0 comments on commit 098949b

Please sign in to comment.