Skip to content

Commit

Permalink
Allow Locals to be propagated into and from, but restricted to their …
Browse files Browse the repository at this point in the history
…own block
  • Loading branch information
felix91gr committed Apr 28, 2020
1 parent 3fd2d4a commit 86927ed
Show file tree
Hide file tree
Showing 10 changed files with 197 additions and 32 deletions.
59 changes: 46 additions & 13 deletions src/librustc_mir/transform/const_prop.rs
Expand Up @@ -326,6 +326,8 @@ struct ConstPropagator<'mir, 'tcx> {
// Because we have `MutVisitor` we can't obtain the `SourceInfo` from a `Location`. So we store
// the last known `SourceInfo` here and just keep revisiting it.
source_info: Option<SourceInfo>,
// Locals we need to forget at the end of the current block
locals_of_current_block: BitSet<Local>,
}

impl<'mir, 'tcx> LayoutOf for ConstPropagator<'mir, 'tcx> {
Expand Down Expand Up @@ -395,6 +397,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
//FIXME(wesleywiser) we can't steal this because `Visitor::super_visit_body()` needs it
local_decls: body.local_decls.clone(),
source_info: None,
locals_of_current_block: BitSet::new_empty(body.local_decls.len()),
}
}

Expand All @@ -409,8 +412,10 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}
}

fn remove_const(&mut self, local: Local) {
self.ecx.frame_mut().locals[local] =
/// Remove `local` from the pool of `Locals`. Allows writing to them,
/// but not reading from them anymore.
fn remove_const(ecx: &mut InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, local: Local) {
ecx.frame_mut().locals[local] =
LocalState { value: LocalValue::Uninitialized, layout: Cell::new(None) };
}

Expand Down Expand Up @@ -756,6 +761,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
enum ConstPropMode {
/// The `Local` can be propagated into and reads of this `Local` can also be propagated.
FullConstProp,
/// The `Local` can only be propagated into and from its own block.
OnlyInsideOwnBlock,
/// The `Local` can be propagated into but reads cannot be propagated.
OnlyPropagateInto,
/// No propagation is allowed at all.
Expand Down Expand Up @@ -787,10 +794,18 @@ impl CanConstProp {
// lint for x != y
// FIXME(oli-obk): lint variables until they are used in a condition
// FIXME(oli-obk): lint if return value is constant
if cpv.local_kinds[local] == LocalKind::Arg || cpv.local_kinds[local] == LocalKind::Var
{
if cpv.local_kinds[local] == LocalKind::Arg {
*val = ConstPropMode::OnlyPropagateInto;
trace!("local {:?} can't be const propagated because it's not a temporary", local);
trace!(
"local {:?} can't be const propagated because it's a function argument",
local
);
} else if cpv.local_kinds[local] == LocalKind::Var {
*val = ConstPropMode::OnlyInsideOwnBlock;
trace!(
"local {:?} will only be propagated inside its block, because it's a user variable",
local
);
}
}
cpv.visit_body(&body);
Expand Down Expand Up @@ -858,25 +873,35 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
if let Some(local) = place.as_local() {
let can_const_prop = self.can_const_prop[local];
if let Some(()) = self.const_prop(rval, place_layout, source_info, place) {
if can_const_prop == ConstPropMode::FullConstProp
|| can_const_prop == ConstPropMode::OnlyPropagateInto
{
if can_const_prop != ConstPropMode::NoPropagation {
// This will return None for Locals that are from other blocks,
// so it should be okay to propagate from here on down.
if let Some(value) = self.get_const(local) {
if self.should_const_prop(value) {
trace!("replacing {:?} with {:?}", rval, value);
self.replace_with_const(rval, value, statement.source_info);

if can_const_prop == ConstPropMode::FullConstProp {
if can_const_prop == ConstPropMode::FullConstProp
|| can_const_prop == ConstPropMode::OnlyInsideOwnBlock
{
trace!("propagated into {:?}", local);
}
}
if can_const_prop == ConstPropMode::OnlyInsideOwnBlock {
trace!(
"found local restricted to its block. Will remove it from const-prop after block is finished. Local: {:?}",
local
);
self.locals_of_current_block.insert(local);
}
}
}
}
if self.can_const_prop[local] != ConstPropMode::FullConstProp {
if self.can_const_prop[local] == ConstPropMode::OnlyPropagateInto
|| self.can_const_prop[local] == ConstPropMode::NoPropagation
{
trace!("can't propagate into {:?}", local);
if local != RETURN_PLACE {
self.remove_const(local);
Self::remove_const(&mut self.ecx, local);
}
}
}
Expand Down Expand Up @@ -915,7 +940,7 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
// doesn't use the invalid value
match cond {
Operand::Move(ref place) | Operand::Copy(ref place) => {
self.remove_const(place.local);
Self::remove_const(&mut self.ecx, place.local);
}
Operand::Constant(_) => {}
}
Expand Down Expand Up @@ -992,5 +1017,13 @@ impl<'mir, 'tcx> MutVisitor<'tcx> for ConstPropagator<'mir, 'tcx> {
//FIXME(wesleywiser) Call does have Operands that could be const-propagated
TerminatorKind::Call { .. } => {}
}
// We remove all Locals which are restricted in propagation to their containing blocks.
// We wouldn't need to clone, but the borrow checker can't see that we're not aliasing
// the locals_of_current_block field, so we need to clone it first.
// let ecx = &mut self.ecx;
for local in self.locals_of_current_block.iter() {
Self::remove_const(&mut self.ecx, local);
}
self.locals_of_current_block.clear();
}
}
Expand Up @@ -21,7 +21,14 @@
// + literal: Const { ty: u32, val: Value(Scalar(0x00000001)) }
StorageLive(_2); // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
StorageLive(_3); // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
_3 = _1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
- _3 = _1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
+ _3 = const 1u32; // scope 1 at $DIR/scalar_literal_propagation.rs:4:13: 4:14
+ // ty::Const
+ // + ty: u32
+ // + val: Value(Scalar(0x00000001))
+ // mir::Constant
+ // + span: $DIR/scalar_literal_propagation.rs:4:13: 4:14
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000001)) }
_2 = const consume(move _3) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:4:5: 4:15
// ty::Const
// + ty: fn(u32) {consume}
Expand Down
Expand Up @@ -29,7 +29,20 @@
// + literal: Const { ty: u32, val: Value(Scalar(0x00000002)) }
StorageLive(_2); // scope 1 at $DIR/tuple_literal_propagation.rs:5:5: 5:15
StorageLive(_3); // scope 1 at $DIR/tuple_literal_propagation.rs:5:13: 5:14
_3 = _1; // scope 1 at $DIR/tuple_literal_propagation.rs:5:13: 5:14
- _3 = _1; // scope 1 at $DIR/tuple_literal_propagation.rs:5:13: 5:14
+ _3 = (const 1u32, const 2u32); // scope 1 at $DIR/tuple_literal_propagation.rs:5:13: 5:14
+ // ty::Const
+ // + ty: u32
+ // + val: Value(Scalar(0x00000001))
+ // mir::Constant
+ // + span: $DIR/tuple_literal_propagation.rs:5:13: 5:14
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000001)) }
+ // ty::Const
+ // + ty: u32
+ // + val: Value(Scalar(0x00000002))
+ // mir::Constant
+ // + span: $DIR/tuple_literal_propagation.rs:5:13: 5:14
+ // + literal: Const { ty: u32, val: Value(Scalar(0x00000002)) }
_2 = const consume(move _3) -> bb1; // scope 1 at $DIR/tuple_literal_propagation.rs:5:5: 5:15
// ty::Const
// + ty: fn((u32, u32)) {consume}
Expand Down
1 change: 1 addition & 0 deletions src/test/mir-opt/simplify-arm-identity.rs
Expand Up @@ -2,6 +2,7 @@
// Regression test for issue #66856.
//
// compile-flags: -Zmir-opt-level=2
// EMIT_MIR_FOR_EACH_BIT_WIDTH

enum Src {
Foo(u8),
Expand Down
@@ -0,0 +1,53 @@
- // MIR for `main` before SimplifyArmIdentity
+ // MIR for `main` after SimplifyArmIdentity

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/simplify-arm-identity.rs:17:11: 17:11
let _1: Src as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 0 at $DIR/simplify-arm-identity.rs:18:9: 18:10
let mut _2: Dst; // in scope 0 at $DIR/simplify-arm-identity.rs:19:18: 22:6
let mut _3: isize; // in scope 0 at $DIR/simplify-arm-identity.rs:20:9: 20:20
let mut _5: u8; // in scope 0 at $DIR/simplify-arm-identity.rs:20:33: 20:34
scope 1 {
debug e => _1; // in scope 1 at $DIR/simplify-arm-identity.rs:18:9: 18:10
let _4: u8; // in scope 1 at $DIR/simplify-arm-identity.rs:20:18: 20:19
scope 2 {
}
scope 3 {
debug x => _4; // in scope 3 at $DIR/simplify-arm-identity.rs:20:18: 20:19
}
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/simplify-arm-identity.rs:18:9: 18:10
((_1 as Foo).0: u8) = const 0u8; // scope 0 at $DIR/simplify-arm-identity.rs:18:18: 18:29
// ty::Const
// + ty: u8
// + val: Value(Scalar(0x00))
// mir::Constant
// + span: $DIR/simplify-arm-identity.rs:18:27: 18:28
// + literal: Const { ty: u8, val: Value(Scalar(0x00)) }
discriminant(_1) = 0; // scope 0 at $DIR/simplify-arm-identity.rs:18:18: 18:29
StorageLive(_2); // scope 1 at $DIR/simplify-arm-identity.rs:19:18: 22:6
_3 = const 0isize; // scope 1 at $DIR/simplify-arm-identity.rs:20:9: 20:20
// ty::Const
// + ty: isize
// + val: Value(Scalar(0x00000000))
// mir::Constant
// + span: $DIR/simplify-arm-identity.rs:20:9: 20:20
// + literal: Const { ty: isize, val: Value(Scalar(0x00000000)) }
_4 = ((_1 as Foo).0: u8); // scope 1 at $DIR/simplify-arm-identity.rs:20:18: 20:19
((_2 as Foo).0: u8) = move _4; // scope 3 at $DIR/simplify-arm-identity.rs:20:24: 20:35
discriminant(_2) = 0; // scope 3 at $DIR/simplify-arm-identity.rs:20:24: 20:35
StorageDead(_2); // scope 1 at $DIR/simplify-arm-identity.rs:22:6: 22:7
_0 = const (); // scope 0 at $DIR/simplify-arm-identity.rs:17:11: 23:2
// ty::Const
// + ty: ()
// + val: Value(Scalar(<ZST>))
// mir::Constant
// + span: $DIR/simplify-arm-identity.rs:17:11: 23:2
// + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
StorageDead(_1); // scope 0 at $DIR/simplify-arm-identity.rs:23:1: 23:2
return; // scope 0 at $DIR/simplify-arm-identity.rs:23:2: 23:2
}
}

@@ -0,0 +1,53 @@
- // MIR for `main` before SimplifyArmIdentity
+ // MIR for `main` after SimplifyArmIdentity

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/simplify-arm-identity.rs:17:11: 17:11
let _1: Src as UserTypeProjection { base: UserType(0), projs: [] }; // in scope 0 at $DIR/simplify-arm-identity.rs:18:9: 18:10
let mut _2: Dst; // in scope 0 at $DIR/simplify-arm-identity.rs:19:18: 22:6
let mut _3: isize; // in scope 0 at $DIR/simplify-arm-identity.rs:20:9: 20:20
let mut _5: u8; // in scope 0 at $DIR/simplify-arm-identity.rs:20:33: 20:34
scope 1 {
debug e => _1; // in scope 1 at $DIR/simplify-arm-identity.rs:18:9: 18:10
let _4: u8; // in scope 1 at $DIR/simplify-arm-identity.rs:20:18: 20:19
scope 2 {
}
scope 3 {
debug x => _4; // in scope 3 at $DIR/simplify-arm-identity.rs:20:18: 20:19
}
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/simplify-arm-identity.rs:18:9: 18:10
((_1 as Foo).0: u8) = const 0u8; // scope 0 at $DIR/simplify-arm-identity.rs:18:18: 18:29
// ty::Const
// + ty: u8
// + val: Value(Scalar(0x00))
// mir::Constant
// + span: $DIR/simplify-arm-identity.rs:18:27: 18:28
// + literal: Const { ty: u8, val: Value(Scalar(0x00)) }
discriminant(_1) = 0; // scope 0 at $DIR/simplify-arm-identity.rs:18:18: 18:29
StorageLive(_2); // scope 1 at $DIR/simplify-arm-identity.rs:19:18: 22:6
_3 = const 0isize; // scope 1 at $DIR/simplify-arm-identity.rs:20:9: 20:20
// ty::Const
// + ty: isize
// + val: Value(Scalar(0x0000000000000000))
// mir::Constant
// + span: $DIR/simplify-arm-identity.rs:20:9: 20:20
// + literal: Const { ty: isize, val: Value(Scalar(0x0000000000000000)) }
_4 = ((_1 as Foo).0: u8); // scope 1 at $DIR/simplify-arm-identity.rs:20:18: 20:19
((_2 as Foo).0: u8) = move _4; // scope 3 at $DIR/simplify-arm-identity.rs:20:24: 20:35
discriminant(_2) = 0; // scope 3 at $DIR/simplify-arm-identity.rs:20:24: 20:35
StorageDead(_2); // scope 1 at $DIR/simplify-arm-identity.rs:22:6: 22:7
_0 = const (); // scope 0 at $DIR/simplify-arm-identity.rs:17:11: 23:2
// ty::Const
// + ty: ()
// + val: Value(Scalar(<ZST>))
// mir::Constant
// + span: $DIR/simplify-arm-identity.rs:17:11: 23:2
// + literal: Const { ty: (), val: Value(Scalar(<ZST>)) }
StorageDead(_1); // scope 0 at $DIR/simplify-arm-identity.rs:23:1: 23:2
return; // scope 0 at $DIR/simplify-arm-identity.rs:23:2: 23:2
}
}

0 comments on commit 86927ed

Please sign in to comment.