From 74a270ac93710ef4ef2315cce3840486f92698b5 Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Sat, 3 Dec 2022 17:00:05 -0500 Subject: [PATCH] Re-enable removal of ZST writes to unions --- .../rustc_mir_transform/src/remove_zsts.rs | 27 +------------------ ...remove_zsts.get_union.PreCodegen.after.mir | 10 +++++++ .../remove_zsts.get_union.RemoveZsts.diff | 19 +++++++++++++ src/test/mir-opt/remove_zsts.rs | 14 ++++++++++ ...ouch_unions.get_union.RemoveZsts.after.mir | 15 ----------- .../mir-opt/remove_zsts_dont_touch_unions.rs | 19 ------------- 6 files changed, 44 insertions(+), 60 deletions(-) create mode 100644 src/test/mir-opt/remove_zsts.get_union.PreCodegen.after.mir create mode 100644 src/test/mir-opt/remove_zsts.get_union.RemoveZsts.diff create mode 100644 src/test/mir-opt/remove_zsts.rs delete mode 100644 src/test/mir-opt/remove_zsts_dont_touch_unions.get_union.RemoveZsts.after.mir delete mode 100644 src/test/mir-opt/remove_zsts_dont_touch_unions.rs diff --git a/compiler/rustc_mir_transform/src/remove_zsts.rs b/compiler/rustc_mir_transform/src/remove_zsts.rs index 40be4f146db98..569e783fee847 100644 --- a/compiler/rustc_mir_transform/src/remove_zsts.rs +++ b/compiler/rustc_mir_transform/src/remove_zsts.rs @@ -1,8 +1,7 @@ //! Removes assignments to ZST places. use crate::MirPass; -use rustc_middle::mir::tcx::PlaceTy; -use rustc_middle::mir::{Body, LocalDecls, Place, StatementKind}; +use rustc_middle::mir::{Body, StatementKind}; use rustc_middle::ty::{self, Ty, TyCtxt}; pub struct RemoveZsts; @@ -35,9 +34,6 @@ impl<'tcx> MirPass<'tcx> for RemoveZsts { if !layout.is_zst() { continue; } - if involves_a_union(place, local_decls, tcx) { - continue; - } if tcx.consider_optimizing(|| { format!( "RemoveZsts - Place: {:?} SourceInfo: {:?}", @@ -63,24 +59,3 @@ fn maybe_zst(ty: Ty<'_>) -> bool { _ => false, } } - -/// Miri lazily allocates memory for locals on assignment, -/// so we must preserve writes to unions and union fields, -/// or it will ICE on reads of those fields. -fn involves_a_union<'tcx>( - place: Place<'tcx>, - local_decls: &LocalDecls<'tcx>, - tcx: TyCtxt<'tcx>, -) -> bool { - let mut place_ty = PlaceTy::from_ty(local_decls[place.local].ty); - if place_ty.ty.is_union() { - return true; - } - for elem in place.projection { - place_ty = place_ty.projection_ty(tcx, elem); - if place_ty.ty.is_union() { - return true; - } - } - return false; -} diff --git a/src/test/mir-opt/remove_zsts.get_union.PreCodegen.after.mir b/src/test/mir-opt/remove_zsts.get_union.PreCodegen.after.mir new file mode 100644 index 0000000000000..12e914e25e0c8 --- /dev/null +++ b/src/test/mir-opt/remove_zsts.get_union.PreCodegen.after.mir @@ -0,0 +1,10 @@ +// MIR for `get_union` after PreCodegen + +fn get_union() -> Foo { + let mut _0: Foo; // return place in scope 0 at $DIR/remove_zsts.rs:+0:19: +0:22 + + bb0: { + Deinit(_0); // scope 0 at $DIR/remove_zsts.rs:+1:5: +1:18 + return; // scope 0 at $DIR/remove_zsts.rs:+2:2: +2:2 + } +} diff --git a/src/test/mir-opt/remove_zsts.get_union.RemoveZsts.diff b/src/test/mir-opt/remove_zsts.get_union.RemoveZsts.diff new file mode 100644 index 0000000000000..169b7b1054b47 --- /dev/null +++ b/src/test/mir-opt/remove_zsts.get_union.RemoveZsts.diff @@ -0,0 +1,19 @@ +- // MIR for `get_union` before RemoveZsts ++ // MIR for `get_union` after RemoveZsts + + fn get_union() -> Foo { + let mut _0: Foo; // return place in scope 0 at $DIR/remove_zsts.rs:+0:19: +0:22 + let mut _1: (); // in scope 0 at $DIR/remove_zsts.rs:+1:14: +1:16 + + bb0: { + StorageLive(_1); // scope 0 at $DIR/remove_zsts.rs:+1:14: +1:16 +- Deinit(_1); // scope 0 at $DIR/remove_zsts.rs:+1:14: +1:16 ++ nop; // scope 0 at $DIR/remove_zsts.rs:+1:14: +1:16 + Deinit(_0); // scope 0 at $DIR/remove_zsts.rs:+1:5: +1:18 +- (_0.0: ()) = move _1; // scope 0 at $DIR/remove_zsts.rs:+1:5: +1:18 ++ nop; // scope 0 at $DIR/remove_zsts.rs:+1:5: +1:18 + StorageDead(_1); // scope 0 at $DIR/remove_zsts.rs:+1:17: +1:18 + return; // scope 0 at $DIR/remove_zsts.rs:+2:2: +2:2 + } + } + diff --git a/src/test/mir-opt/remove_zsts.rs b/src/test/mir-opt/remove_zsts.rs new file mode 100644 index 0000000000000..1cf7ad6e3667c --- /dev/null +++ b/src/test/mir-opt/remove_zsts.rs @@ -0,0 +1,14 @@ +union Foo { + x: (), + y: u64, +} + +// EMIT_MIR remove_zsts.get_union.RemoveZsts.diff +// EMIT_MIR remove_zsts.get_union.PreCodegen.after.mir +fn get_union() -> Foo { + Foo { x: () } +} + +fn main() { + get_union(); +} diff --git a/src/test/mir-opt/remove_zsts_dont_touch_unions.get_union.RemoveZsts.after.mir b/src/test/mir-opt/remove_zsts_dont_touch_unions.get_union.RemoveZsts.after.mir deleted file mode 100644 index 7d9e6046202ca..0000000000000 --- a/src/test/mir-opt/remove_zsts_dont_touch_unions.get_union.RemoveZsts.after.mir +++ /dev/null @@ -1,15 +0,0 @@ -// MIR for `get_union` after RemoveZsts - -fn get_union() -> Foo { - let mut _0: Foo; // return place in scope 0 at $DIR/remove_zsts_dont_touch_unions.rs:+0:19: +0:22 - let mut _1: (); // in scope 0 at $DIR/remove_zsts_dont_touch_unions.rs:+1:14: +1:16 - - bb0: { - StorageLive(_1); // scope 0 at $DIR/remove_zsts_dont_touch_unions.rs:+1:14: +1:16 - nop; // scope 0 at $DIR/remove_zsts_dont_touch_unions.rs:+1:14: +1:16 - Deinit(_0); // scope 0 at $DIR/remove_zsts_dont_touch_unions.rs:+1:5: +1:18 - (_0.0: ()) = move _1; // scope 0 at $DIR/remove_zsts_dont_touch_unions.rs:+1:5: +1:18 - StorageDead(_1); // scope 0 at $DIR/remove_zsts_dont_touch_unions.rs:+1:17: +1:18 - return; // scope 0 at $DIR/remove_zsts_dont_touch_unions.rs:+2:2: +2:2 - } -} diff --git a/src/test/mir-opt/remove_zsts_dont_touch_unions.rs b/src/test/mir-opt/remove_zsts_dont_touch_unions.rs deleted file mode 100644 index 8b9de9b4d65a6..0000000000000 --- a/src/test/mir-opt/remove_zsts_dont_touch_unions.rs +++ /dev/null @@ -1,19 +0,0 @@ -// unit-test: RemoveZsts - -// Ensure RemoveZsts doesn't remove ZST assignments to union fields, -// which causes problems in Miri. - -union Foo { - x: (), - y: u64, -} - -// EMIT_MIR remove_zsts_dont_touch_unions.get_union.RemoveZsts.after.mir -fn get_union() -> Foo { - Foo { x: () } -} - - -fn main() { - get_union(); -}