Auto merge of #113915 - cjgillot:ssa-call, r=tmiasko

Also consider call and yield as MIR SSA.

The SSA analysis on MIR only considered `Assign` statements as defining a SSA local.
This PR adds assignments as part of a `Call` or `Yield` terminator in that category.

This mainly allows to perform CopyProp on a call return place.

The only subtlety is in the dominance property: the assignment is only complete at the beginning of the target block.
This commit is contained in:
bors 2023-10-10 20:37:55 +00:00
commit d627cf07ce
13 changed files with 224 additions and 124 deletions

View File

@ -63,7 +63,7 @@ use rustc_middle::mir::*;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_target::abi::{VariantIdx, FIRST_VARIANT};
use crate::ssa::SsaLocals;
use crate::ssa::{AssignedValue, SsaLocals};
use crate::MirPass;
pub struct GVN;
@ -87,21 +87,28 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
let dominators = body.basic_blocks.dominators().clone();
let mut state = VnState::new(tcx, param_env, &ssa, &dominators, &body.local_decls);
for arg in body.args_iter() {
if ssa.is_ssa(arg) {
let value = state.new_opaque().unwrap();
state.assign(arg, value);
}
}
ssa.for_each_assignment_mut(&mut body.basic_blocks, |local, rvalue, location| {
let value = state.simplify_rvalue(rvalue, location).or_else(|| state.new_opaque()).unwrap();
// FIXME(#112651) `rvalue` may have a subtype to `local`. We can only mark `local` as
// reusable if we have an exact type match.
if state.local_decls[local].ty == rvalue.ty(state.local_decls, tcx) {
ssa.for_each_assignment_mut(
body.basic_blocks.as_mut_preserves_cfg(),
|local, value, location| {
let value = match value {
// We do not know anything of this assigned value.
AssignedValue::Arg | AssignedValue::Terminator(_) => None,
// Try to get some insight.
AssignedValue::Rvalue(rvalue) => {
let value = state.simplify_rvalue(rvalue, location);
// FIXME(#112651) `rvalue` may have a subtype to `local`. We can only mark `local` as
// reusable if we have an exact type match.
if state.local_decls[local].ty != rvalue.ty(state.local_decls, tcx) {
return;
}
value
}
};
// `next_opaque` is `Some`, so `new_opaque` must return `Some`.
let value = value.or_else(|| state.new_opaque()).unwrap();
state.assign(local, value);
}
});
},
);
// Stop creating opaques during replacement as it is useless.
state.next_opaque = None;

View File

@ -5,7 +5,6 @@
//! As a consequence of rule 2, we consider that borrowed locals are not SSA, even if they are
//! `Freeze`, as we do not track that the assignment dominates all uses of the borrow.
use either::Either;
use rustc_data_structures::graph::dominators::Dominators;
use rustc_index::bit_set::BitSet;
use rustc_index::{IndexSlice, IndexVec};
@ -27,6 +26,12 @@ pub struct SsaLocals {
direct_uses: IndexVec<Local, u32>,
}
pub enum AssignedValue<'a, 'tcx> {
Arg,
Rvalue(&'a mut Rvalue<'tcx>),
Terminator(&'a mut TerminatorKind<'tcx>),
}
impl SsaLocals {
pub fn new<'tcx>(body: &Body<'tcx>) -> SsaLocals {
let assignment_order = Vec::with_capacity(body.local_decls.len());
@ -39,6 +44,7 @@ impl SsaLocals {
for local in body.args_iter() {
visitor.assignments[local] = Set1::One(DefLocation::Argument);
visitor.assignment_order.push(local);
}
// For SSA assignments, a RPO visit will see the assignment before it sees any use.
@ -105,8 +111,8 @@ impl SsaLocals {
) -> impl Iterator<Item = (Local, &'a Rvalue<'tcx>, Location)> + 'a {
self.assignment_order.iter().filter_map(|&local| {
if let Set1::One(DefLocation::Body(loc)) = self.assignments[local] {
let stmt = body.stmt_at(loc).left()?;
// `loc` must point to a direct assignment to `local`.
let Either::Left(stmt) = body.stmt_at(loc) else { bug!() };
let Some((target, rvalue)) = stmt.kind.as_assign() else { bug!() };
assert_eq!(target.as_local(), Some(local));
Some((local, rvalue, loc))
@ -118,18 +124,33 @@ impl SsaLocals {
pub fn for_each_assignment_mut<'tcx>(
&self,
basic_blocks: &mut BasicBlocks<'tcx>,
mut f: impl FnMut(Local, &mut Rvalue<'tcx>, Location),
basic_blocks: &mut IndexSlice<BasicBlock, BasicBlockData<'tcx>>,
mut f: impl FnMut(Local, AssignedValue<'_, 'tcx>, Location),
) {
for &local in &self.assignment_order {
if let Set1::One(DefLocation::Body(loc)) = self.assignments[local] {
// `loc` must point to a direct assignment to `local`.
let bbs = basic_blocks.as_mut_preserves_cfg();
let bb = &mut bbs[loc.block];
let stmt = &mut bb.statements[loc.statement_index];
let StatementKind::Assign(box (target, ref mut rvalue)) = stmt.kind else { bug!() };
assert_eq!(target.as_local(), Some(local));
f(local, rvalue, loc)
match self.assignments[local] {
Set1::One(DefLocation::Argument) => f(
local,
AssignedValue::Arg,
Location { block: START_BLOCK, statement_index: 0 },
),
Set1::One(DefLocation::Body(loc)) => {
let bb = &mut basic_blocks[loc.block];
let value = if loc.statement_index < bb.statements.len() {
// `loc` must point to a direct assignment to `local`.
let stmt = &mut bb.statements[loc.statement_index];
let StatementKind::Assign(box (target, ref mut rvalue)) = stmt.kind else {
bug!()
};
assert_eq!(target.as_local(), Some(local));
AssignedValue::Rvalue(rvalue)
} else {
let term = bb.terminator_mut();
AssignedValue::Terminator(&mut term.kind)
};
f(local, value, loc)
}
_ => {}
}
}
}
@ -228,8 +249,22 @@ impl<'tcx> Visitor<'tcx> for SsaVisitor<'_> {
}
fn visit_place(&mut self, place: &Place<'tcx>, ctxt: PlaceContext, loc: Location) {
if place.projection.first() == Some(&PlaceElem::Deref) {
// Do not do anything for storage statements and debuginfo.
let location = match ctxt {
PlaceContext::MutatingUse(
MutatingUseContext::Store | MutatingUseContext::Call | MutatingUseContext::Yield,
) => Some(DefLocation::Body(loc)),
_ => None,
};
if let Some(location) = location
&& let Some(local) = place.as_local()
{
self.assignments[local].insert(location);
if let Set1::One(_) = self.assignments[local] {
// Only record if SSA-like, to avoid growing the vector needlessly.
self.assignment_order.push(local);
}
} else if place.projection.first() == Some(&PlaceElem::Deref) {
// Do not do anything for debuginfo.
if ctxt.is_use() {
// Only change the context if it is a real use, not a "use" in debuginfo.
let new_ctxt = PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy);
@ -237,25 +272,11 @@ impl<'tcx> Visitor<'tcx> for SsaVisitor<'_> {
self.visit_projection(place.as_ref(), new_ctxt, loc);
self.check_dominates(place.local, loc);
}
return;
} else {
self.visit_projection(place.as_ref(), ctxt, loc);
self.visit_local(place.local, ctxt, loc);
}
}
fn visit_assign(&mut self, place: &Place<'tcx>, rvalue: &Rvalue<'tcx>, loc: Location) {
if let Some(local) = place.as_local() {
self.assignments[local].insert(DefLocation::Body(loc));
if let Set1::One(_) = self.assignments[local] {
// Only record if SSA-like, to avoid growing the vector needlessly.
self.assignment_order.push(local);
}
} else {
self.visit_place(place, PlaceContext::MutatingUse(MutatingUseContext::Store), loc);
}
self.visit_rvalue(rvalue, loc);
}
}
#[instrument(level = "trace", skip(ssa, body))]

View File

@ -0,0 +1,21 @@
- // MIR for `multiple_edges` before CopyProp
+ // MIR for `multiple_edges` after CopyProp
fn multiple_edges(_1: bool) -> u8 {
let mut _0: u8;
let mut _2: u8;
bb0: {
switchInt(_1) -> [1: bb1, otherwise: bb2];
}
bb1: {
_2 = dummy(const 13_u8) -> [return: bb2, unwind continue];
}
bb2: {
_0 = _2;
return;
}
}

View File

@ -0,0 +1,24 @@
- // MIR for `nrvo` before CopyProp
+ // MIR for `nrvo` after CopyProp
fn nrvo() -> u8 {
let mut _0: u8;
let _1: u8;
scope 1 {
- debug y => _1;
+ debug y => _0;
}
bb0: {
- StorageLive(_1);
- _1 = dummy(const 5_u8) -> [return: bb1, unwind continue];
+ _0 = dummy(const 5_u8) -> [return: bb1, unwind continue];
}
bb1: {
- _0 = _1;
- StorageDead(_1);
return;
}
}

View File

@ -0,0 +1,43 @@
// Check that CopyProp does propagate return values of call terminators.
// unit-test: CopyProp
// needs-unwind
#![feature(custom_mir, core_intrinsics)]
use std::intrinsics::mir::*;
#[inline(never)]
fn dummy(x: u8) -> u8 {
x
}
// EMIT_MIR calls.nrvo.CopyProp.diff
fn nrvo() -> u8 {
let y = dummy(5); // this should get NRVO
y
}
// EMIT_MIR calls.multiple_edges.CopyProp.diff
#[custom_mir(dialect = "runtime", phase = "initial")]
fn multiple_edges(t: bool) -> u8 {
mir! {
let x: u8;
{
match t { true => bbt, _ => ret }
}
bbt = {
Call(x = dummy(13), ret)
}
ret = {
// `x` is not assigned on the `bb0 -> ret` edge,
// so should not be marked as SSA for merging with `_0`.
RET = x;
Return()
}
}
}
fn main() {
// Make sure the function actually gets instantiated.
nrvo();
multiple_edges(false);
}

View File

@ -8,11 +8,11 @@
+ scope 1 (inlined call_twice::<!, fn() -> ! {sleep}>) {
+ debug f => _2;
+ let mut _3: &fn() -> ! {sleep};
+ let mut _4: !;
+ let _4: !;
+ let mut _5: &fn() -> ! {sleep};
+ let mut _6: !;
+ scope 2 {
+ debug a => _4;
+ let _6: !;
+ scope 3 {
+ debug b => _6;
+ }

View File

@ -10,10 +10,10 @@
+ let mut _3: &fn() -> ! {sleep};
+ let _4: !;
+ let mut _5: &fn() -> ! {sleep};
+ let mut _6: !;
+ let mut _7: !;
+ scope 2 {
+ debug a => _4;
+ let _6: !;
+ scope 3 {
+ debug b => _6;
+ }

View File

@ -6,21 +6,20 @@ fn filter_mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> Option<U>) -> ()
let mut _0: ();
let mut _3: std::iter::FilterMap<impl Iterator<Item = T>, impl Fn(T) -> Option<U>>;
let mut _4: std::iter::FilterMap<impl Iterator<Item = T>, impl Fn(T) -> Option<U>>;
let mut _5: std::iter::FilterMap<impl Iterator<Item = T>, impl Fn(T) -> Option<U>>;
let mut _6: &mut std::iter::FilterMap<impl Iterator<Item = T>, impl Fn(T) -> Option<U>>;
let mut _9: std::option::Option<U>;
let mut _10: isize;
let _12: ();
let mut _5: &mut std::iter::FilterMap<impl Iterator<Item = T>, impl Fn(T) -> Option<U>>;
let mut _8: std::option::Option<U>;
let mut _9: isize;
let _11: ();
scope 1 {
debug iter => _5;
let _11: U;
debug iter => _4;
let _10: U;
scope 2 {
debug x => _11;
debug x => _10;
}
scope 4 (inlined <FilterMap<impl Iterator<Item = T>, impl Fn(T) -> Option<U>> as Iterator>::next) {
debug self => _6;
let mut _7: &mut impl Iterator<Item = T>;
let mut _8: &mut impl Fn(T) -> Option<U>;
debug self => _5;
let mut _6: &mut impl Iterator<Item = T>;
let mut _7: &mut impl Fn(T) -> Option<U>;
}
}
scope 3 (inlined <FilterMap<impl Iterator<Item = T>, impl Fn(T) -> Option<U>> as IntoIterator>::into_iter) {
@ -28,54 +27,49 @@ fn filter_mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> Option<U>) -> ()
}
bb0: {
StorageLive(_4);
StorageLive(_3);
_3 = <impl Iterator<Item = T> as Iterator>::filter_map::<U, impl Fn(T) -> Option<U>>(move _1, move _2) -> [return: bb1, unwind continue];
}
bb1: {
StorageLive(_4);
_4 = move _3;
StorageDead(_3);
StorageLive(_5);
_5 = move _4;
goto -> bb2;
}
bb2: {
StorageLive(_9);
_6 = &mut _5;
StorageLive(_7);
_7 = &mut (_5.0: impl Iterator<Item = T>);
StorageLive(_8);
_8 = &mut (_5.1: impl Fn(T) -> Option<U>);
_9 = <impl Iterator<Item = T> as Iterator>::find_map::<U, &mut impl Fn(T) -> Option<U>>(move _7, move _8) -> [return: bb3, unwind: bb9];
_5 = &mut _4;
StorageLive(_6);
_6 = &mut (_4.0: impl Iterator<Item = T>);
StorageLive(_7);
_7 = &mut (_4.1: impl Fn(T) -> Option<U>);
_8 = <impl Iterator<Item = T> as Iterator>::find_map::<U, &mut impl Fn(T) -> Option<U>>(move _6, move _7) -> [return: bb3, unwind: bb9];
}
bb3: {
StorageDead(_8);
StorageDead(_7);
_10 = discriminant(_9);
switchInt(move _10) -> [0: bb4, 1: bb6, otherwise: bb8];
StorageDead(_6);
_9 = discriminant(_8);
switchInt(move _9) -> [0: bb4, 1: bb6, otherwise: bb8];
}
bb4: {
StorageDead(_9);
drop(_5) -> [return: bb5, unwind continue];
StorageDead(_8);
drop(_4) -> [return: bb5, unwind continue];
}
bb5: {
StorageDead(_5);
StorageDead(_4);
return;
}
bb6: {
_11 = move ((_9 as Some).0: U);
_12 = opaque::<U>(move _11) -> [return: bb7, unwind: bb9];
_10 = move ((_8 as Some).0: U);
_11 = opaque::<U>(move _10) -> [return: bb7, unwind: bb9];
}
bb7: {
StorageDead(_9);
StorageDead(_8);
goto -> bb2;
}
@ -84,7 +78,7 @@ fn filter_mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> Option<U>) -> ()
}
bb9 (cleanup): {
drop(_5) -> [return: bb10, unwind terminate(cleanup)];
drop(_4) -> [return: bb10, unwind terminate(cleanup)];
}
bb10 (cleanup): {

View File

@ -6,16 +6,15 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () {
let mut _0: ();
let mut _3: std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>;
let mut _4: std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>;
let mut _5: std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>;
let mut _6: &mut std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>;
let mut _7: std::option::Option<U>;
let mut _8: isize;
let _10: ();
let mut _5: &mut std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>;
let mut _6: std::option::Option<U>;
let mut _7: isize;
let _9: ();
scope 1 {
debug iter => _5;
let _9: U;
debug iter => _4;
let _8: U;
scope 2 {
debug x => _9;
debug x => _8;
}
}
scope 3 (inlined <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as IntoIterator>::into_iter) {
@ -23,50 +22,45 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () {
}
bb0: {
StorageLive(_4);
StorageLive(_3);
_3 = <impl Iterator<Item = T> as Iterator>::map::<U, impl Fn(T) -> U>(move _1, move _2) -> [return: bb1, unwind continue];
}
bb1: {
StorageLive(_4);
_4 = move _3;
StorageDead(_3);
StorageLive(_5);
_5 = move _4;
goto -> bb2;
}
bb2: {
StorageLive(_7);
StorageLive(_6);
_6 = &mut _5;
_7 = <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as Iterator>::next(move _6) -> [return: bb3, unwind: bb9];
StorageLive(_5);
_5 = &mut _4;
_6 = <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as Iterator>::next(move _5) -> [return: bb3, unwind: bb9];
}
bb3: {
StorageDead(_6);
_8 = discriminant(_7);
switchInt(move _8) -> [0: bb4, 1: bb6, otherwise: bb8];
StorageDead(_5);
_7 = discriminant(_6);
switchInt(move _7) -> [0: bb4, 1: bb6, otherwise: bb8];
}
bb4: {
StorageDead(_7);
drop(_5) -> [return: bb5, unwind continue];
StorageDead(_6);
drop(_4) -> [return: bb5, unwind continue];
}
bb5: {
StorageDead(_5);
StorageDead(_4);
return;
}
bb6: {
_9 = move ((_7 as Some).0: U);
_10 = opaque::<U>(move _9) -> [return: bb7, unwind: bb9];
_8 = move ((_6 as Some).0: U);
_9 = opaque::<U>(move _8) -> [return: bb7, unwind: bb9];
}
bb7: {
StorageDead(_7);
StorageDead(_6);
goto -> bb2;
}
@ -75,7 +69,7 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () {
}
bb9 (cleanup): {
drop(_5) -> [return: bb10, unwind terminate(cleanup)];
drop(_4) -> [return: bb10, unwind terminate(cleanup)];
}
bb10 (cleanup): {

View File

@ -7,17 +7,13 @@ fn slice_index_range(_1: &[u32], _2: std::ops::Range<usize>) -> &[u32] {
scope 1 (inlined #[track_caller] core::slice::index::<impl Index<std::ops::Range<usize>> for [u32]>::index) {
debug self => _1;
debug index => _2;
let _3: &[u32];
}
bb0: {
StorageLive(_3);
_3 = <std::ops::Range<usize> as SliceIndex<[u32]>>::index(move _2, move _1) -> [return: bb1, unwind unreachable];
_0 = <std::ops::Range<usize> as SliceIndex<[u32]>>::index(move _2, move _1) -> [return: bb1, unwind unreachable];
}
bb1: {
_0 = _3;
StorageDead(_3);
return;
}
}

View File

@ -7,17 +7,13 @@ fn slice_index_range(_1: &[u32], _2: std::ops::Range<usize>) -> &[u32] {
scope 1 (inlined #[track_caller] core::slice::index::<impl Index<std::ops::Range<usize>> for [u32]>::index) {
debug self => _1;
debug index => _2;
let _3: &[u32];
}
bb0: {
StorageLive(_3);
_3 = <std::ops::Range<usize> as SliceIndex<[u32]>>::index(move _2, move _1) -> [return: bb1, unwind continue];
_0 = <std::ops::Range<usize> as SliceIndex<[u32]>>::index(move _2, move _1) -> [return: bb1, unwind continue];
}
bb1: {
_0 = _3;
StorageDead(_3);
return;
}
}

View File

@ -92,8 +92,8 @@
StorageDead(_7);
- StorageDead(_6);
- StorageLive(_10);
StorageLive(_11);
StorageLive(_12);
- StorageLive(_11);
- StorageLive(_12);
StorageLive(_13);
_26 = const _;
_13 = &(*_26);
@ -105,8 +105,9 @@
bb5: {
StorageDead(_15);
StorageDead(_13);
_11 = &(*_12);
_16 = Len((*_11));
- _11 = &(*_12);
- _16 = Len((*_11));
+ _16 = Len((*_12));
_17 = const 3_usize;
_18 = Ge(move _16, move _17);
switchInt(move _18) -> [0: bb7, otherwise: bb6];
@ -114,12 +115,15 @@
bb6: {
StorageLive(_19);
_19 = &(*_11)[1 of 3];
- _19 = &(*_11)[1 of 3];
+ _19 = &(*_12)[1 of 3];
StorageLive(_20);
_20 = &(*_11)[2:-1];
- _20 = &(*_11)[2:-1];
+ _20 = &(*_12)[2:-1];
StorageLive(_21);
_21 = &(*_11)[-1 of 3];
- _21 = &(*_11)[-1 of 3];
- _10 = const ();
+ _21 = &(*_12)[-1 of 3];
StorageDead(_21);
StorageDead(_20);
StorageDead(_19);
@ -132,8 +136,8 @@
}
bb8: {
StorageDead(_12);
StorageDead(_11);
- StorageDead(_12);
- StorageDead(_11);
- StorageDead(_10);
StorageLive(_22);
StorageLive(_23);

View File

@ -58,7 +58,7 @@ fn test_stable_mir(_tcx: TyCtxt<'_>) -> ControlFlow<()> {
let foo_bar = get_item(&items, (DefKind::Fn, "foo_bar")).unwrap();
let body = foo_bar.body();
assert_eq!(body.locals.len(), 7);
assert_eq!(body.locals.len(), 5);
assert_eq!(body.blocks.len(), 4);
let block = &body.blocks[0];
match &block.terminator.kind {