Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix ConstProp handling of written_only_inside_own_block_locals #102045

Merged
merged 1 commit into from
Sep 21, 2022
Merged

fix ConstProp handling of written_only_inside_own_block_locals #102045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 20 additions & 20 deletions compiler/rustc_mir_transform/src/const_prop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1066,32 +1066,32 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> {
let source_info = terminator.source_info;
self.source_info = Some(source_info);
self.super_terminator(terminator, location);
// Do NOT early return in this function, it does some crucial fixup of the state at the end!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably put the non-fixup logic into a separate function that can then early return...

Copy link
Member Author

@RalfJung RalfJung Sep 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of refactoring that could be done here... by someone who actually understands that code.^^

(I also still haven't quite figured out what check_binary_op actually checks and what its return value means. Few of the functions in this file have doc comments.)

match &mut terminator.kind {
TerminatorKind::Assert { expected, ref mut cond, .. } => {
if let Some(ref value) = self.eval_operand(&cond) {
trace!("assertion on {:?} should be {:?}", value, expected);
let expected = Scalar::from_bool(*expected);
let Ok(value_const) = self.ecx.read_scalar(&value) else {
// FIXME should be used use_ecx rather than a local match... but we have
// quite a few of these read_scalar/read_immediate that need fixing.
return
};
if expected != value_const {
// Poison all places this operand references so that further code
// doesn't use the invalid value
match cond {
Operand::Move(ref place) | Operand::Copy(ref place) => {
Self::remove_const(&mut self.ecx, place.local);
// FIXME should be used use_ecx rather than a local match... but we have
// quite a few of these read_scalar/read_immediate that need fixing.
if let Ok(value_const) = self.ecx.read_scalar(&value) {
if expected != value_const {
// Poison all places this operand references so that further code
// doesn't use the invalid value
match cond {
Operand::Move(ref place) | Operand::Copy(ref place) => {
Self::remove_const(&mut self.ecx, place.local);
}
Operand::Constant(_) => {}
}
} else {
if self.should_const_prop(value) {
*cond = self.operand_from_scalar(
value_const,
self.tcx.types.bool,
source_info.span,
);
}
Operand::Constant(_) => {}
}
} else {
if self.should_const_prop(value) {
*cond = self.operand_from_scalar(
value_const,
self.tcx.types.bool,
source_info.span,
);
}
}
}
Expand Down
20 changes: 20 additions & 0 deletions src/test/mir-opt/issue-101973.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// compile-flags: -O -C debug-assertions=on
// This needs inlining followed by ConstProp to reproduce, so we cannot use "unit-test".

#[inline]
pub fn imm8(x: u32) -> u32 {
let mut out = 0u32;
out |= (x >> 0) & 0xff;
out
}

// EMIT_MIR issue_101973.inner.ConstProp.diff
#[inline(never)]
pub fn inner(fields: u32) -> i64 {
imm8(fields).rotate_right(((fields >> 8) & 0xf) << 1) as i32 as i64
}

fn main() {
let val = inner(0xe32cf20f);
assert_eq!(val as u64, 0xfffffffff0000000);
}
100 changes: 100 additions & 0 deletions src/test/mir-opt/issue_101973.inner.ConstProp.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
- // MIR for `inner` before ConstProp
+ // MIR for `inner` after ConstProp

fn inner(_1: u32) -> i64 {
debug fields => _1; // in scope 0 at $DIR/issue-101973.rs:+0:14: +0:20
let mut _0: i64; // return place in scope 0 at $DIR/issue-101973.rs:+0:30: +0:33
let mut _2: i32; // in scope 0 at $DIR/issue-101973.rs:+1:5: +1:65
let mut _3: u32; // in scope 0 at $DIR/issue-101973.rs:+1:5: +1:58
let mut _4: u32; // in scope 0 at $DIR/issue-101973.rs:+1:5: +1:17
let mut _5: u32; // in scope 0 at $DIR/issue-101973.rs:+1:10: +1:16
let mut _6: u32; // in scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
let mut _7: u32; // in scope 0 at $DIR/issue-101973.rs:+1:31: +1:52
let mut _8: u32; // in scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
let mut _9: u32; // in scope 0 at $DIR/issue-101973.rs:+1:33: +1:39
let mut _10: (u32, bool); // in scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
let mut _11: (u32, bool); // in scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
scope 1 (inlined imm8) { // at $DIR/issue-101973.rs:14:5: 14:17
debug x => _5; // in scope 1 at $DIR/issue-101973.rs:5:13: 5:14
let mut _12: u32; // in scope 1 at $DIR/issue-101973.rs:7:12: 7:27
let mut _13: u32; // in scope 1 at $DIR/issue-101973.rs:7:12: 7:20
let mut _14: u32; // in scope 1 at $DIR/issue-101973.rs:7:13: 7:14
let mut _15: (u32, bool); // in scope 1 at $DIR/issue-101973.rs:7:12: 7:20
scope 2 {
debug out => _4; // in scope 2 at $DIR/issue-101973.rs:6:9: 6:16
}
}
scope 3 (inlined core::num::<impl u32>::rotate_right) { // at $DIR/issue-101973.rs:14:5: 14:58
debug self => _4; // in scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
debug n => _6; // in scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
let mut _16: u32; // in scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
let mut _17: u32; // in scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:65
StorageLive(_3); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:58
StorageLive(_4); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:17
StorageLive(_5); // scope 0 at $DIR/issue-101973.rs:+1:10: +1:16
_5 = _1; // scope 0 at $DIR/issue-101973.rs:+1:10: +1:16
_4 = const 0_u32; // scope 1 at $DIR/issue-101973.rs:6:19: 6:23
StorageLive(_12); // scope 2 at $DIR/issue-101973.rs:7:12: 7:27
StorageLive(_13); // scope 2 at $DIR/issue-101973.rs:7:12: 7:20
StorageLive(_14); // scope 2 at $DIR/issue-101973.rs:7:13: 7:14
_14 = _5; // scope 2 at $DIR/issue-101973.rs:7:13: 7:14
_15 = CheckedShr(_14, const 0_i32); // scope 2 at $DIR/issue-101973.rs:7:12: 7:20
assert(!move (_15.1: bool), "attempt to shift right by `{}`, which would overflow", const 0_i32) -> bb3; // scope 2 at $DIR/issue-101973.rs:7:12: 7:20
}

bb1: {
_8 = move (_10.0: u32); // scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
StorageDead(_9); // scope 0 at $DIR/issue-101973.rs:+1:44: +1:45
_7 = BitAnd(move _8, const 15_u32); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:52
StorageDead(_8); // scope 0 at $DIR/issue-101973.rs:+1:51: +1:52
_11 = CheckedShl(_7, const 1_i32); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
assert(!move (_11.1: bool), "attempt to shift left by `{}`, which would overflow", const 1_i32) -> bb2; // scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
}

bb2: {
_6 = move (_11.0: u32); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
StorageDead(_7); // scope 0 at $DIR/issue-101973.rs:+1:56: +1:57
StorageLive(_16); // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
_16 = _4; // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
StorageLive(_17); // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
_17 = _6; // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
_3 = rotate_right::<u32>(move _16, move _17) -> bb4; // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
// mir::Constant
// + span: $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
// + literal: Const { ty: extern "rust-intrinsic" fn(u32, u32) -> u32 {rotate_right::<u32>}, val: Value(<ZST>) }
}

bb3: {
_13 = move (_15.0: u32); // scope 2 at $DIR/issue-101973.rs:7:12: 7:20
StorageDead(_14); // scope 2 at $DIR/issue-101973.rs:7:19: 7:20
_12 = BitAnd(move _13, const 255_u32); // scope 2 at $DIR/issue-101973.rs:7:12: 7:27
StorageDead(_13); // scope 2 at $DIR/issue-101973.rs:7:26: 7:27
_4 = BitOr(_4, move _12); // scope 2 at $DIR/issue-101973.rs:7:5: 7:27
StorageDead(_12); // scope 2 at $DIR/issue-101973.rs:7:26: 7:27
StorageDead(_5); // scope 0 at $DIR/issue-101973.rs:+1:16: +1:17
StorageLive(_6); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:57
StorageLive(_7); // scope 0 at $DIR/issue-101973.rs:+1:31: +1:52
StorageLive(_8); // scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
StorageLive(_9); // scope 0 at $DIR/issue-101973.rs:+1:33: +1:39
_9 = _1; // scope 0 at $DIR/issue-101973.rs:+1:33: +1:39
_10 = CheckedShr(_9, const 8_i32); // scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
assert(!move (_10.1: bool), "attempt to shift right by `{}`, which would overflow", const 8_i32) -> bb1; // scope 0 at $DIR/issue-101973.rs:+1:32: +1:45
}

bb4: {
StorageDead(_17); // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
StorageDead(_16); // scope 3 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
StorageDead(_6); // scope 0 at $DIR/issue-101973.rs:+1:57: +1:58
StorageDead(_4); // scope 0 at $DIR/issue-101973.rs:+1:57: +1:58
_2 = move _3 as i32 (Misc); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:65
StorageDead(_3); // scope 0 at $DIR/issue-101973.rs:+1:64: +1:65
_0 = move _2 as i64 (Misc); // scope 0 at $DIR/issue-101973.rs:+1:5: +1:72
StorageDead(_2); // scope 0 at $DIR/issue-101973.rs:+1:71: +1:72
return; // scope 0 at $DIR/issue-101973.rs:+2:2: +2:2
}
}