1
Fork 0

Remove an unnecessary restriction in dest_prop

This commit is contained in:
Jakob Degen 2022-02-24 09:47:13 -05:00
parent 9ecd75b831
commit 57c4163294
3 changed files with 20 additions and 33 deletions

View file

@ -38,12 +38,6 @@
//! It must also not contain any indexing projections, since those take an arbitrary `Local` as //! 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. //! 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 //! * `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 //! fundamental restriction or just current impl state?). It can be copied or moved by the
//! assignment. //! assignment.
@ -103,7 +97,6 @@ use rustc_index::{
bit_set::{BitMatrix, BitSet}, bit_set::{BitMatrix, BitSet},
vec::IndexVec, vec::IndexVec,
}; };
use rustc_middle::mir::tcx::PlaceTy;
use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor}; use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor};
use rustc_middle::mir::{dump_mir, PassWhere}; use rustc_middle::mir::{dump_mir, PassWhere};
use rustc_middle::mir::{ use rustc_middle::mir::{
@ -135,7 +128,7 @@ impl<'tcx> MirPass<'tcx> for DestinationPropagation {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let def_id = body.source.def_id(); let def_id = body.source.def_id();
let candidates = find_candidates(tcx, body); let candidates = find_candidates(body);
if candidates.is_empty() { if candidates.is_empty() {
debug!("{:?}: no dest prop candidates, done", def_id); debug!("{:?}: no dest prop candidates, done", def_id);
return; 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 /// 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 /// otherwise ineligible (eg. locals used as array indices are ignored because we cannot propagate
/// arbitrary places into array indices). /// arbitrary places into array indices).
fn find_candidates<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> Vec<CandidateAssignment<'tcx>> { fn find_candidates<'tcx>(body: &Body<'tcx>) -> Vec<CandidateAssignment<'tcx>> {
let mut visitor = FindAssignments { let mut visitor = FindAssignments {
tcx,
body, body,
candidates: Vec::new(), candidates: Vec::new(),
ever_borrowed_locals: ever_borrowed_locals(body), ever_borrowed_locals: ever_borrowed_locals(body),
@ -816,7 +808,6 @@ fn find_candidates<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> Vec<CandidateA
} }
struct FindAssignments<'a, 'tcx> { struct FindAssignments<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
body: &'a Body<'tcx>, body: &'a Body<'tcx>,
candidates: Vec<CandidateAssignment<'tcx>>, candidates: Vec<CandidateAssignment<'tcx>>,
ever_borrowed_locals: BitSet<Local>, ever_borrowed_locals: BitSet<Local>,
@ -845,10 +836,11 @@ impl<'tcx> Visitor<'tcx> for FindAssignments<'_, 'tcx> {
return; return;
} }
// Can't optimize if both locals ever have their address taken (can introduce // Can't optimize if either local ever has their address taken. This optimization does
// aliasing). // liveness analysis only based on assignments, and a local can be live even if its
// FIXME: This can be smarter and take `StorageDead` into account (which // never assigned to again, because a reference to it might be live.
// invalidates borrows). // FIXME: This can be smarter and take `StorageDead` into account (which invalidates
// borrows).
if self.ever_borrowed_locals.contains(dest.local) if self.ever_borrowed_locals.contains(dest.local)
|| self.ever_borrowed_locals.contains(src.local) || self.ever_borrowed_locals.contains(src.local)
{ {
@ -862,22 +854,11 @@ impl<'tcx> Visitor<'tcx> for FindAssignments<'_, 'tcx> {
return; 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 { for elem in dest.projection {
if let PlaceElem::Index(_) = elem { if let PlaceElem::Index(_) = elem {
// `dest` contains an indexing projection. // `dest` contains an indexing projection.
return; return;
} }
place_ty = place_ty.projection_ty(self.tcx, elem);
if place_ty.ty.is_union() {
return;
}
} }
self.candidates.push(CandidateAssignment { self.candidates.push(CandidateAssignment {

View file

@ -17,23 +17,29 @@
} }
bb0: { bb0: {
StorageLive(_1); // scope 0 at $DIR/union.rs:13:9: 13:11 - StorageLive(_1); // scope 0 at $DIR/union.rs:13:9: 13:11
StorageLive(_2); // scope 0 at $DIR/union.rs:13:23: 13:28 - 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 - _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 // mir::Constant
// + span: $DIR/union.rs:13:23: 13:26 // + span: $DIR/union.rs:13:23: 13:26
// + literal: Const { ty: fn() -> u32 {val}, val: Value(Scalar(<ZST>)) } // + literal: Const { ty: fn() -> u32 {val}, val: Value(Scalar(<ZST>)) }
} }
bb1: { bb1: {
(_1.0: u32) = move _2; // scope 0 at $DIR/union.rs:13:14: 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 - 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(_3); // scope 1 at $DIR/union.rs:15:5: 15:27
StorageLive(_4); // scope 1 at $DIR/union.rs:15:10: 15:26 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 _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(_4); // scope 1 at $DIR/union.rs:15:26: 15:27
StorageDead(_3); // scope 1 at $DIR/union.rs:15:27: 15:28 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 return; // scope 0 at $DIR/union.rs:16:2: 16:2
} }
} }

View file

@ -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 // compile-flags: -Zunsound-mir-opts
fn val() -> u32 { fn val() -> u32 {
1 1