From 57c41632949fbfad67207335ba16e7eba3b7adf1 Mon Sep 17 00:00:00 2001 From: Jakob Degen Date: Thu, 24 Feb 2022 09:47:13 -0500 Subject: [PATCH] Remove an unnecessary restriction in `dest_prop` --- compiler/rustc_mir_transform/src/dest_prop.rs | 33 ++++--------------- .../union.main.DestinationPropagation.diff | 18 ++++++---- src/test/mir-opt/dest-prop/union.rs | 2 +- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/compiler/rustc_mir_transform/src/dest_prop.rs b/compiler/rustc_mir_transform/src/dest_prop.rs index 237ead591a5..7878d6eaab1 100644 --- a/compiler/rustc_mir_transform/src/dest_prop.rs +++ b/compiler/rustc_mir_transform/src/dest_prop.rs @@ -38,12 +38,6 @@ //! It must also not contain any indexing projections, since those take an arbitrary `Local` as //! the index, and that local might only be initialized shortly before `dest` is used. //! -//! Subtle case: If `dest` is a, or projects through a union, then we have to make sure that there -//! remains an assignment to it, since that sets the "active field" of the union. But if `src` is -//! a ZST, it might not be initialized, so there might not be any use of it before the assignment, -//! and performing the optimization would simply delete the assignment, leaving `dest` -//! uninitialized. -//! //! * `src` must be a bare `Local` without any indirections or field projections (FIXME: Is this a //! fundamental restriction or just current impl state?). It can be copied or moved by the //! assignment. @@ -103,7 +97,6 @@ bit_set::{BitMatrix, BitSet}, vec::IndexVec, }; -use rustc_middle::mir::tcx::PlaceTy; use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor}; use rustc_middle::mir::{dump_mir, PassWhere}; use rustc_middle::mir::{ @@ -135,7 +128,7 @@ fn is_enabled(&self, sess: &rustc_session::Session) -> bool { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { let def_id = body.source.def_id(); - let candidates = find_candidates(tcx, body); + let candidates = find_candidates(body); if candidates.is_empty() { debug!("{:?}: no dest prop candidates, done", def_id); return; @@ -803,9 +796,8 @@ struct CandidateAssignment<'tcx> { /// comment) and also throw out assignments that involve a local that has its address taken or is /// otherwise ineligible (eg. locals used as array indices are ignored because we cannot propagate /// arbitrary places into array indices). -fn find_candidates<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> Vec> { +fn find_candidates<'tcx>(body: &Body<'tcx>) -> Vec> { let mut visitor = FindAssignments { - tcx, body, candidates: Vec::new(), ever_borrowed_locals: ever_borrowed_locals(body), @@ -816,7 +808,6 @@ fn find_candidates<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> Vec { - tcx: TyCtxt<'tcx>, body: &'a Body<'tcx>, candidates: Vec>, ever_borrowed_locals: BitSet, @@ -845,10 +836,11 @@ fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { return; } - // Can't optimize if both locals ever have their address taken (can introduce - // aliasing). - // FIXME: This can be smarter and take `StorageDead` into account (which - // invalidates borrows). + // Can't optimize if either local ever has their address taken. This optimization does + // liveness analysis only based on assignments, and a local can be live even if its + // never assigned to again, because a reference to it might be live. + // FIXME: This can be smarter and take `StorageDead` into account (which invalidates + // borrows). if self.ever_borrowed_locals.contains(dest.local) || self.ever_borrowed_locals.contains(src.local) { @@ -862,22 +854,11 @@ fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) { return; } - // Handle the "subtle case" described above by rejecting any `dest` that is or - // projects through a union. - let mut place_ty = PlaceTy::from_ty(self.body.local_decls[dest.local].ty); - if place_ty.ty.is_union() { - return; - } for elem in dest.projection { if let PlaceElem::Index(_) = elem { // `dest` contains an indexing projection. return; } - - place_ty = place_ty.projection_ty(self.tcx, elem); - if place_ty.ty.is_union() { - return; - } } self.candidates.push(CandidateAssignment { diff --git a/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff b/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff index 9be0738ea96..11776ed21e1 100644 --- a/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff +++ b/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff @@ -17,23 +17,29 @@ } bb0: { - StorageLive(_1); // scope 0 at $DIR/union.rs:13:9: 13:11 - StorageLive(_2); // scope 0 at $DIR/union.rs:13:23: 13:28 - _2 = val() -> bb1; // scope 0 at $DIR/union.rs:13:23: 13:28 +- StorageLive(_1); // scope 0 at $DIR/union.rs:13:9: 13:11 +- StorageLive(_2); // scope 0 at $DIR/union.rs:13:23: 13:28 +- _2 = val() -> bb1; // scope 0 at $DIR/union.rs:13:23: 13:28 ++ nop; // scope 0 at $DIR/union.rs:13:9: 13:11 ++ nop; // scope 0 at $DIR/union.rs:13:23: 13:28 ++ (_1.0: u32) = val() -> bb1; // scope 0 at $DIR/union.rs:13:23: 13:28 // mir::Constant // + span: $DIR/union.rs:13:23: 13:26 // + literal: Const { ty: fn() -> u32 {val}, val: Value(Scalar()) } } bb1: { - (_1.0: u32) = move _2; // scope 0 at $DIR/union.rs:13:14: 13:30 - StorageDead(_2); // scope 0 at $DIR/union.rs:13:29: 13:30 +- (_1.0: u32) = move _2; // scope 0 at $DIR/union.rs:13:14: 13:30 +- StorageDead(_2); // scope 0 at $DIR/union.rs:13:29: 13:30 ++ nop; // scope 0 at $DIR/union.rs:13:14: 13:30 ++ nop; // scope 0 at $DIR/union.rs:13:29: 13:30 StorageLive(_3); // scope 1 at $DIR/union.rs:15:5: 15:27 StorageLive(_4); // scope 1 at $DIR/union.rs:15:10: 15:26 _4 = (_1.0: u32); // scope 2 at $DIR/union.rs:15:19: 15:24 StorageDead(_4); // scope 1 at $DIR/union.rs:15:26: 15:27 StorageDead(_3); // scope 1 at $DIR/union.rs:15:27: 15:28 - StorageDead(_1); // scope 0 at $DIR/union.rs:16:1: 16:2 +- StorageDead(_1); // scope 0 at $DIR/union.rs:16:1: 16:2 ++ nop; // scope 0 at $DIR/union.rs:16:1: 16:2 return; // scope 0 at $DIR/union.rs:16:2: 16:2 } } diff --git a/src/test/mir-opt/dest-prop/union.rs b/src/test/mir-opt/dest-prop/union.rs index 0ac9661a66a..68c834dfbbf 100644 --- a/src/test/mir-opt/dest-prop/union.rs +++ b/src/test/mir-opt/dest-prop/union.rs @@ -1,4 +1,4 @@ -//! Tests that projections through unions cancel `DestinationPropagation`. +//! Tests that we can propogate into places that are projections into unions // compile-flags: -Zunsound-mir-opts fn val() -> u32 { 1