From 63f3dcefb950d9d9f63b8a71de39d990a742b675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 11 Sep 2020 00:00:00 +0000 Subject: [PATCH 1/4] copy-prop: Add test case for missed optimization due const prop failure Currently a failure to completely propagate a constant inhibits propagation of any subsequent locals. Add a test case demonstrating the problem before resolving it. --- ...propagation_arg.maybe.CopyPropagation.diff | 204 ++++++++++++++++++ src/test/mir-opt/copy_propagation_arg.rs | 15 ++ 2 files changed, 219 insertions(+) create mode 100644 src/test/mir-opt/copy_propagation_arg.maybe.CopyPropagation.diff diff --git a/src/test/mir-opt/copy_propagation_arg.maybe.CopyPropagation.diff b/src/test/mir-opt/copy_propagation_arg.maybe.CopyPropagation.diff new file mode 100644 index 0000000000000..7ef7c3c47f38f --- /dev/null +++ b/src/test/mir-opt/copy_propagation_arg.maybe.CopyPropagation.diff @@ -0,0 +1,204 @@ +- // MIR for `maybe` before CopyPropagation ++ // MIR for `maybe` after CopyPropagation + + fn maybe(_1: &[bool], _2: String) -> () { + debug m => _1; // in scope 0 at $DIR/copy_propagation_arg.rs:34:10: 34:11 + debug s1 => _2; // in scope 0 at $DIR/copy_propagation_arg.rs:34:22: 34:24 + let mut _0: (); // return place in scope 0 at $DIR/copy_propagation_arg.rs:34:34: 34:34 + let mut _3: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + let _4: usize; // in scope 0 at $DIR/copy_propagation_arg.rs:35:10: 35:11 + let mut _5: usize; // in scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + let mut _6: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + let _7: std::string::String; // in scope 0 at $DIR/copy_propagation_arg.rs:36:13: 36:15 + let mut _8: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:37:12: 37:16 + let _9: usize; // in scope 0 at $DIR/copy_propagation_arg.rs:37:14: 37:15 + let mut _10: usize; // in scope 0 at $DIR/copy_propagation_arg.rs:37:12: 37:16 + let mut _11: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:37:12: 37:16 + let mut _13: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:39:16: 39:20 + let _14: usize; // in scope 0 at $DIR/copy_propagation_arg.rs:39:18: 39:19 + let mut _15: usize; // in scope 0 at $DIR/copy_propagation_arg.rs:39:16: 39:20 + let mut _16: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:39:16: 39:20 + let _18: &std::string::String; // in scope 0 at $DIR/copy_propagation_arg.rs:41:17: 41:20 + let mut _19: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:45:1: 45:2 + let mut _20: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + let mut _21: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + scope 1 { + debug s2 => _7; // in scope 1 at $DIR/copy_propagation_arg.rs:36:13: 36:15 + let _12: std::string::String; // in scope 1 at $DIR/copy_propagation_arg.rs:38:17: 38:19 + scope 2 { + debug s3 => _12; // in scope 2 at $DIR/copy_propagation_arg.rs:38:17: 38:19 + let _17: std::string::String; // in scope 2 at $DIR/copy_propagation_arg.rs:40:21: 40:23 + scope 3 { + debug s4 => _17; // in scope 3 at $DIR/copy_propagation_arg.rs:40:21: 40:23 + } + } + } + + bb0: { + _21 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + _19 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + _20 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + _19 = const true; // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + StorageLive(_3); // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 +- StorageLive(_4); // scope 0 at $DIR/copy_propagation_arg.rs:35:10: 35:11 ++ nop; // scope 0 at $DIR/copy_propagation_arg.rs:35:10: 35:11 + _4 = const 0_usize; // scope 0 at $DIR/copy_propagation_arg.rs:35:10: 35:11 + _5 = Len((*_1)); // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + _6 = Lt(const 0_usize, _5); // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + assert(move _6, "index out of bounds: the len is {} but the index is {}", move _5, const 0_usize) -> [success: bb2, unwind: bb17]; // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 + } + + bb1 (cleanup): { + resume; // scope 0 at $DIR/copy_propagation_arg.rs:34:1: 45:2 + } + + bb2: { + _3 = (*_1)[_4]; // scope 0 at $DIR/copy_propagation_arg.rs:35:8: 35:12 +- StorageDead(_4); // scope 0 at $DIR/copy_propagation_arg.rs:35:11: 35:12 ++ nop; // scope 0 at $DIR/copy_propagation_arg.rs:35:11: 35:12 + switchInt(_3) -> [false: bb3, otherwise: bb4]; // scope 0 at $DIR/copy_propagation_arg.rs:35:5: 44:6 + } + + bb3: { + _0 = const (); // scope 0 at $DIR/copy_propagation_arg.rs:35:5: 44:6 + goto -> bb14; // scope 0 at $DIR/copy_propagation_arg.rs:35:5: 44:6 + } + + bb4: { + StorageLive(_7); // scope 0 at $DIR/copy_propagation_arg.rs:36:13: 36:15 + _19 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:36:18: 36:20 + _20 = const true; // scope 0 at $DIR/copy_propagation_arg.rs:36:18: 36:20 + _7 = move _2; // scope 0 at $DIR/copy_propagation_arg.rs:36:18: 36:20 + StorageLive(_8); // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 + StorageLive(_9); // scope 1 at $DIR/copy_propagation_arg.rs:37:14: 37:15 + _9 = const 1_usize; // scope 1 at $DIR/copy_propagation_arg.rs:37:14: 37:15 + _10 = Len((*_1)); // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 + _11 = Lt(const 1_usize, _10); // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 + assert(move _11, "index out of bounds: the len is {} but the index is {}", move _10, const 1_usize) -> [success: bb5, unwind: bb19]; // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 + } + + bb5: { + _8 = (*_1)[_9]; // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 + StorageDead(_9); // scope 1 at $DIR/copy_propagation_arg.rs:37:15: 37:16 + switchInt(_8) -> [false: bb6, otherwise: bb7]; // scope 1 at $DIR/copy_propagation_arg.rs:37:9: 43:10 + } + + bb6: { + _0 = const (); // scope 1 at $DIR/copy_propagation_arg.rs:37:9: 43:10 + goto -> bb25; // scope 1 at $DIR/copy_propagation_arg.rs:37:9: 43:10 + } + + bb7: { + StorageLive(_12); // scope 1 at $DIR/copy_propagation_arg.rs:38:17: 38:19 + _20 = const false; // scope 1 at $DIR/copy_propagation_arg.rs:38:22: 38:24 + _21 = const true; // scope 1 at $DIR/copy_propagation_arg.rs:38:22: 38:24 + _12 = move _7; // scope 1 at $DIR/copy_propagation_arg.rs:38:22: 38:24 + StorageLive(_13); // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 + StorageLive(_14); // scope 2 at $DIR/copy_propagation_arg.rs:39:18: 39:19 + _14 = const 2_usize; // scope 2 at $DIR/copy_propagation_arg.rs:39:18: 39:19 + _15 = Len((*_1)); // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 + _16 = Lt(const 2_usize, _15); // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 + assert(move _16, "index out of bounds: the len is {} but the index is {}", move _15, const 2_usize) -> [success: bb8, unwind: bb21]; // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 + } + + bb8: { + _13 = (*_1)[_14]; // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 + StorageDead(_14); // scope 2 at $DIR/copy_propagation_arg.rs:39:19: 39:20 + switchInt(_13) -> [false: bb9, otherwise: bb10]; // scope 2 at $DIR/copy_propagation_arg.rs:39:13: 42:14 + } + + bb9: { + _0 = const (); // scope 2 at $DIR/copy_propagation_arg.rs:39:13: 42:14 + goto -> bb23; // scope 2 at $DIR/copy_propagation_arg.rs:39:13: 42:14 + } + + bb10: { + StorageLive(_17); // scope 2 at $DIR/copy_propagation_arg.rs:40:21: 40:23 + _21 = const false; // scope 2 at $DIR/copy_propagation_arg.rs:40:26: 40:28 + _17 = move _12; // scope 2 at $DIR/copy_propagation_arg.rs:40:26: 40:28 + StorageLive(_18); // scope 3 at $DIR/copy_propagation_arg.rs:41:17: 41:20 + _18 = &_17; // scope 3 at $DIR/copy_propagation_arg.rs:41:17: 41:20 + StorageDead(_18); // scope 3 at $DIR/copy_propagation_arg.rs:41:20: 41:21 + _0 = const (); // scope 2 at $DIR/copy_propagation_arg.rs:39:21: 42:14 + drop(_17) -> [return: bb11, unwind: bb21]; // scope 2 at $DIR/copy_propagation_arg.rs:42:13: 42:14 + } + + bb11: { + StorageDead(_17); // scope 2 at $DIR/copy_propagation_arg.rs:42:13: 42:14 + goto -> bb23; // scope 2 at $DIR/copy_propagation_arg.rs:39:13: 42:14 + } + + bb12: { + _21 = const false; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + StorageDead(_12); // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + StorageDead(_13); // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + goto -> bb25; // scope 1 at $DIR/copy_propagation_arg.rs:37:9: 43:10 + } + + bb13: { + _20 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + StorageDead(_7); // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + StorageDead(_8); // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + goto -> bb14; // scope 0 at $DIR/copy_propagation_arg.rs:35:5: 44:6 + } + + bb14: { + StorageDead(_3); // scope 0 at $DIR/copy_propagation_arg.rs:45:1: 45:2 + switchInt(_19) -> [false: bb15, otherwise: bb26]; // scope 0 at $DIR/copy_propagation_arg.rs:45:1: 45:2 + } + + bb15: { + return; // scope 0 at $DIR/copy_propagation_arg.rs:45:2: 45:2 + } + + bb16 (cleanup): { + _19 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:45:1: 45:2 + drop(_2) -> bb1; // scope 0 at $DIR/copy_propagation_arg.rs:45:1: 45:2 + } + + bb17 (cleanup): { + switchInt(_19) -> [false: bb1, otherwise: bb16]; // scope 0 at $DIR/copy_propagation_arg.rs:45:1: 45:2 + } + + bb18 (cleanup): { + _20 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + drop(_7) -> bb17; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + } + + bb19 (cleanup): { + switchInt(_20) -> [false: bb17, otherwise: bb18]; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + } + + bb20 (cleanup): { + _21 = const false; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + drop(_12) -> bb19; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + } + + bb21 (cleanup): { + switchInt(_21) -> [false: bb19, otherwise: bb20]; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + } + + bb22: { + _21 = const false; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + drop(_12) -> [return: bb12, unwind: bb19]; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + } + + bb23: { + switchInt(_21) -> [false: bb12, otherwise: bb22]; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 + } + + bb24: { + _20 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + drop(_7) -> [return: bb13, unwind: bb17]; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + } + + bb25: { + switchInt(_20) -> [false: bb13, otherwise: bb24]; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 + } + + bb26: { + _19 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:45:1: 45:2 + drop(_2) -> [return: bb15, unwind: bb1]; // scope 0 at $DIR/copy_propagation_arg.rs:45:1: 45:2 + } + } + diff --git a/src/test/mir-opt/copy_propagation_arg.rs b/src/test/mir-opt/copy_propagation_arg.rs index 3a00fc58a4ea8..94f5a6535ba71 100644 --- a/src/test/mir-opt/copy_propagation_arg.rs +++ b/src/test/mir-opt/copy_propagation_arg.rs @@ -30,10 +30,25 @@ fn arg_src(mut x: i32) -> i32 { y } +// EMIT_MIR copy_propagation_arg.maybe.CopyPropagation.diff +fn maybe(m: &[bool], s1: String) { + if m[0] { + let s2 = s1; + if m[1] { + let s3 = s2; + if m[2] { + let s4 = s3; + &s4; + } + } + } +} + fn main() { // Make sure the function actually gets instantiated. foo(0); bar(0); baz(0); arg_src(0); + maybe(&[true, false, true], "Hello!".to_owned()); } From 2ee57d752f3bd1ce3bf386465de9e60b6ebbfa52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 11 Sep 2020 00:00:00 +0000 Subject: [PATCH 2/4] copy-prop: Attempt to propagate each local only once The copy propagation as currently implemented does not introduce new copy propagation opportunities. Examine each local for possible optimization only once. Additionally, this also fixes the issue where copy propagation optimization would stop without examining all locals after failing to completely propagate a constant. --- compiler/rustc_mir/src/transform/copy_prop.rs | 169 +++++++++--------- ...propagation_arg.maybe.CopyPropagation.diff | 66 ++++--- 2 files changed, 127 insertions(+), 108 deletions(-) diff --git a/compiler/rustc_mir/src/transform/copy_prop.rs b/compiler/rustc_mir/src/transform/copy_prop.rs index ba406c72df848..c14e61096195d 100644 --- a/compiler/rustc_mir/src/transform/copy_prop.rs +++ b/compiler/rustc_mir/src/transform/copy_prop.rs @@ -38,114 +38,111 @@ impl<'tcx> MirPass<'tcx> for CopyPropagation { } let mut def_use_analysis = DefUseAnalysis::new(body); - loop { - def_use_analysis.analyze(body); + let mut changed = true; - if eliminate_self_assignments(body, &def_use_analysis) { + for dest_local in body.local_decls.indices() { + if changed { def_use_analysis.analyze(body); + if eliminate_self_assignments(body, &def_use_analysis) { + def_use_analysis.analyze(body); + } + changed = false; } - let mut changed = false; - for dest_local in body.local_decls.indices() { - debug!("considering destination local: {:?}", dest_local); - - let action; - let location; - { - // The destination must have exactly one def. - let dest_use_info = def_use_analysis.local_info(dest_local); - let dest_def_count = dest_use_info.def_count_not_including_drop(); - if dest_def_count == 0 { - debug!(" Can't copy-propagate local: dest {:?} undefined", dest_local); - continue; - } - if dest_def_count > 1 { - debug!( - " Can't copy-propagate local: dest {:?} defined {} times", - dest_local, - dest_use_info.def_count() - ); - continue; - } - if dest_use_info.use_count() == 0 { - debug!(" Can't copy-propagate local: dest {:?} unused", dest_local); - continue; - } - // Conservatively gives up if the dest is an argument, - // because there may be uses of the original argument value. - // Also gives up on the return place, as we cannot propagate into its implicit - // use by `return`. - if matches!( - body.local_kind(dest_local), - LocalKind::Arg | LocalKind::ReturnPointer - ) { - debug!(" Can't copy-propagate local: dest {:?} (argument)", dest_local); + debug!("considering destination local: {:?}", dest_local); + + let action; + let location; + { + // The destination must have exactly one def. + let dest_use_info = def_use_analysis.local_info(dest_local); + let dest_def_count = dest_use_info.def_count_not_including_drop(); + if dest_def_count == 0 { + debug!(" Can't copy-propagate local: dest {:?} undefined", dest_local); + continue; + } + if dest_def_count > 1 { + debug!( + " Can't copy-propagate local: dest {:?} defined {} times", + dest_local, + dest_use_info.def_count() + ); + continue; + } + if dest_use_info.use_count() == 0 { + debug!(" Can't copy-propagate local: dest {:?} unused", dest_local); + continue; + } + // Conservatively gives up if the dest is an argument, + // because there may be uses of the original argument value. + // Also gives up on the return place, as we cannot propagate into its implicit + // use by `return`. + if matches!( + body.local_kind(dest_local), + LocalKind::Arg | LocalKind::ReturnPointer + ) { + debug!(" Can't copy-propagate local: dest {:?} (argument)", dest_local); + continue; + } + let dest_place_def = dest_use_info.defs_not_including_drop().next().unwrap(); + location = dest_place_def.location; + + let basic_block = &body[location.block]; + let statement_index = location.statement_index; + let statement = match basic_block.statements.get(statement_index) { + Some(statement) => statement, + None => { + debug!(" Can't copy-propagate local: used in terminator"); continue; } - let dest_place_def = dest_use_info.defs_not_including_drop().next().unwrap(); - location = dest_place_def.location; - - let basic_block = &body[location.block]; - let statement_index = location.statement_index; - let statement = match basic_block.statements.get(statement_index) { - Some(statement) => statement, - None => { - debug!(" Can't copy-propagate local: used in terminator"); - continue; - } - }; - - // That use of the source must be an assignment. - match &statement.kind { - StatementKind::Assign(box (place, Rvalue::Use(operand))) => { - if let Some(local) = place.as_local() { - if local == dest_local { - let maybe_action = match operand { - Operand::Copy(src_place) | Operand::Move(src_place) => { - Action::local_copy(&body, &def_use_analysis, *src_place) - } - Operand::Constant(ref src_constant) => { - Action::constant(src_constant) - } - }; - match maybe_action { - Some(this_action) => action = this_action, - None => continue, + }; + + // That use of the source must be an assignment. + match &statement.kind { + StatementKind::Assign(box (place, Rvalue::Use(operand))) => { + if let Some(local) = place.as_local() { + if local == dest_local { + let maybe_action = match operand { + Operand::Copy(src_place) | Operand::Move(src_place) => { + Action::local_copy(&body, &def_use_analysis, *src_place) } - } else { - debug!( - " Can't copy-propagate local: source use is not an \ - assignment" - ); - continue; + Operand::Constant(ref src_constant) => { + Action::constant(src_constant) + } + }; + match maybe_action { + Some(this_action) => action = this_action, + None => continue, } } else { debug!( " Can't copy-propagate local: source use is not an \ - assignment" + assignment" ); continue; } - } - _ => { + } else { debug!( " Can't copy-propagate local: source use is not an \ - assignment" + assignment" ); continue; } } + _ => { + debug!( + " Can't copy-propagate local: source use is not an \ + assignment" + ); + continue; + } } - - changed = - action.perform(body, &def_use_analysis, dest_local, location, tcx) || changed; - // FIXME(pcwalton): Update the use-def chains to delete the instructions instead of - // regenerating the chains. - break; - } - if !changed { - break; } + + // FIXME(pcwalton): Update the use-def chains to delete the instructions instead of + // regenerating the chains. + action.perform(body, &def_use_analysis, dest_local, location, tcx); + changed = true; } } } diff --git a/src/test/mir-opt/copy_propagation_arg.maybe.CopyPropagation.diff b/src/test/mir-opt/copy_propagation_arg.maybe.CopyPropagation.diff index 7ef7c3c47f38f..9e83670df4d37 100644 --- a/src/test/mir-opt/copy_propagation_arg.maybe.CopyPropagation.diff +++ b/src/test/mir-opt/copy_propagation_arg.maybe.CopyPropagation.diff @@ -23,13 +23,16 @@ let mut _20: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 let mut _21: bool; // in scope 0 at $DIR/copy_propagation_arg.rs:43:9: 43:10 scope 1 { - debug s2 => _7; // in scope 1 at $DIR/copy_propagation_arg.rs:36:13: 36:15 +- debug s2 => _7; // in scope 1 at $DIR/copy_propagation_arg.rs:36:13: 36:15 ++ debug s2 => _2; // in scope 1 at $DIR/copy_propagation_arg.rs:36:13: 36:15 let _12: std::string::String; // in scope 1 at $DIR/copy_propagation_arg.rs:38:17: 38:19 scope 2 { - debug s3 => _12; // in scope 2 at $DIR/copy_propagation_arg.rs:38:17: 38:19 +- debug s3 => _12; // in scope 2 at $DIR/copy_propagation_arg.rs:38:17: 38:19 ++ debug s3 => _2; // in scope 2 at $DIR/copy_propagation_arg.rs:38:17: 38:19 let _17: std::string::String; // in scope 2 at $DIR/copy_propagation_arg.rs:40:21: 40:23 scope 3 { - debug s4 => _17; // in scope 3 at $DIR/copy_propagation_arg.rs:40:21: 40:23 +- debug s4 => _17; // in scope 3 at $DIR/copy_propagation_arg.rs:40:21: 40:23 ++ debug s4 => _2; // in scope 3 at $DIR/copy_propagation_arg.rs:40:21: 40:23 } } } @@ -65,12 +68,15 @@ } bb4: { - StorageLive(_7); // scope 0 at $DIR/copy_propagation_arg.rs:36:13: 36:15 +- StorageLive(_7); // scope 0 at $DIR/copy_propagation_arg.rs:36:13: 36:15 ++ nop; // scope 0 at $DIR/copy_propagation_arg.rs:36:13: 36:15 _19 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:36:18: 36:20 _20 = const true; // scope 0 at $DIR/copy_propagation_arg.rs:36:18: 36:20 - _7 = move _2; // scope 0 at $DIR/copy_propagation_arg.rs:36:18: 36:20 +- _7 = move _2; // scope 0 at $DIR/copy_propagation_arg.rs:36:18: 36:20 ++ nop; // scope 0 at $DIR/copy_propagation_arg.rs:36:18: 36:20 StorageLive(_8); // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 - StorageLive(_9); // scope 1 at $DIR/copy_propagation_arg.rs:37:14: 37:15 +- StorageLive(_9); // scope 1 at $DIR/copy_propagation_arg.rs:37:14: 37:15 ++ nop; // scope 1 at $DIR/copy_propagation_arg.rs:37:14: 37:15 _9 = const 1_usize; // scope 1 at $DIR/copy_propagation_arg.rs:37:14: 37:15 _10 = Len((*_1)); // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 _11 = Lt(const 1_usize, _10); // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 @@ -79,7 +85,8 @@ bb5: { _8 = (*_1)[_9]; // scope 1 at $DIR/copy_propagation_arg.rs:37:12: 37:16 - StorageDead(_9); // scope 1 at $DIR/copy_propagation_arg.rs:37:15: 37:16 +- StorageDead(_9); // scope 1 at $DIR/copy_propagation_arg.rs:37:15: 37:16 ++ nop; // scope 1 at $DIR/copy_propagation_arg.rs:37:15: 37:16 switchInt(_8) -> [false: bb6, otherwise: bb7]; // scope 1 at $DIR/copy_propagation_arg.rs:37:9: 43:10 } @@ -89,12 +96,15 @@ } bb7: { - StorageLive(_12); // scope 1 at $DIR/copy_propagation_arg.rs:38:17: 38:19 +- StorageLive(_12); // scope 1 at $DIR/copy_propagation_arg.rs:38:17: 38:19 ++ nop; // scope 1 at $DIR/copy_propagation_arg.rs:38:17: 38:19 _20 = const false; // scope 1 at $DIR/copy_propagation_arg.rs:38:22: 38:24 _21 = const true; // scope 1 at $DIR/copy_propagation_arg.rs:38:22: 38:24 - _12 = move _7; // scope 1 at $DIR/copy_propagation_arg.rs:38:22: 38:24 +- _12 = move _7; // scope 1 at $DIR/copy_propagation_arg.rs:38:22: 38:24 ++ nop; // scope 1 at $DIR/copy_propagation_arg.rs:38:22: 38:24 StorageLive(_13); // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 - StorageLive(_14); // scope 2 at $DIR/copy_propagation_arg.rs:39:18: 39:19 +- StorageLive(_14); // scope 2 at $DIR/copy_propagation_arg.rs:39:18: 39:19 ++ nop; // scope 2 at $DIR/copy_propagation_arg.rs:39:18: 39:19 _14 = const 2_usize; // scope 2 at $DIR/copy_propagation_arg.rs:39:18: 39:19 _15 = Len((*_1)); // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 _16 = Lt(const 2_usize, _15); // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 @@ -103,7 +113,8 @@ bb8: { _13 = (*_1)[_14]; // scope 2 at $DIR/copy_propagation_arg.rs:39:16: 39:20 - StorageDead(_14); // scope 2 at $DIR/copy_propagation_arg.rs:39:19: 39:20 +- StorageDead(_14); // scope 2 at $DIR/copy_propagation_arg.rs:39:19: 39:20 ++ nop; // scope 2 at $DIR/copy_propagation_arg.rs:39:19: 39:20 switchInt(_13) -> [false: bb9, otherwise: bb10]; // scope 2 at $DIR/copy_propagation_arg.rs:39:13: 42:14 } @@ -113,31 +124,38 @@ } bb10: { - StorageLive(_17); // scope 2 at $DIR/copy_propagation_arg.rs:40:21: 40:23 +- StorageLive(_17); // scope 2 at $DIR/copy_propagation_arg.rs:40:21: 40:23 ++ nop; // scope 2 at $DIR/copy_propagation_arg.rs:40:21: 40:23 _21 = const false; // scope 2 at $DIR/copy_propagation_arg.rs:40:26: 40:28 - _17 = move _12; // scope 2 at $DIR/copy_propagation_arg.rs:40:26: 40:28 +- _17 = move _12; // scope 2 at $DIR/copy_propagation_arg.rs:40:26: 40:28 ++ nop; // scope 2 at $DIR/copy_propagation_arg.rs:40:26: 40:28 StorageLive(_18); // scope 3 at $DIR/copy_propagation_arg.rs:41:17: 41:20 - _18 = &_17; // scope 3 at $DIR/copy_propagation_arg.rs:41:17: 41:20 +- _18 = &_17; // scope 3 at $DIR/copy_propagation_arg.rs:41:17: 41:20 ++ _18 = &_2; // scope 3 at $DIR/copy_propagation_arg.rs:41:17: 41:20 StorageDead(_18); // scope 3 at $DIR/copy_propagation_arg.rs:41:20: 41:21 _0 = const (); // scope 2 at $DIR/copy_propagation_arg.rs:39:21: 42:14 - drop(_17) -> [return: bb11, unwind: bb21]; // scope 2 at $DIR/copy_propagation_arg.rs:42:13: 42:14 +- drop(_17) -> [return: bb11, unwind: bb21]; // scope 2 at $DIR/copy_propagation_arg.rs:42:13: 42:14 ++ drop(_2) -> [return: bb11, unwind: bb21]; // scope 2 at $DIR/copy_propagation_arg.rs:42:13: 42:14 } bb11: { - StorageDead(_17); // scope 2 at $DIR/copy_propagation_arg.rs:42:13: 42:14 +- StorageDead(_17); // scope 2 at $DIR/copy_propagation_arg.rs:42:13: 42:14 ++ nop; // scope 2 at $DIR/copy_propagation_arg.rs:42:13: 42:14 goto -> bb23; // scope 2 at $DIR/copy_propagation_arg.rs:39:13: 42:14 } bb12: { _21 = const false; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 - StorageDead(_12); // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 +- StorageDead(_12); // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 ++ nop; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 StorageDead(_13); // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 goto -> bb25; // scope 1 at $DIR/copy_propagation_arg.rs:37:9: 43:10 } bb13: { _20 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 - StorageDead(_7); // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 +- StorageDead(_7); // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 ++ nop; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 StorageDead(_8); // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 goto -> bb14; // scope 0 at $DIR/copy_propagation_arg.rs:35:5: 44:6 } @@ -162,7 +180,8 @@ bb18 (cleanup): { _20 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 - drop(_7) -> bb17; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 +- drop(_7) -> bb17; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 ++ drop(_2) -> bb17; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 } bb19 (cleanup): { @@ -171,7 +190,8 @@ bb20 (cleanup): { _21 = const false; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 - drop(_12) -> bb19; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 +- drop(_12) -> bb19; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 ++ drop(_2) -> bb19; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 } bb21 (cleanup): { @@ -180,7 +200,8 @@ bb22: { _21 = const false; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 - drop(_12) -> [return: bb12, unwind: bb19]; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 +- drop(_12) -> [return: bb12, unwind: bb19]; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 ++ drop(_2) -> [return: bb12, unwind: bb19]; // scope 1 at $DIR/copy_propagation_arg.rs:43:9: 43:10 } bb23: { @@ -189,7 +210,8 @@ bb24: { _20 = const false; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 - drop(_7) -> [return: bb13, unwind: bb17]; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 +- drop(_7) -> [return: bb13, unwind: bb17]; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 ++ drop(_2) -> [return: bb13, unwind: bb17]; // scope 0 at $DIR/copy_propagation_arg.rs:44:5: 44:6 } bb25: { From 35c8b0e78a02dc1f42a27cf009003be9b86df314 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 11 Sep 2020 00:00:00 +0000 Subject: [PATCH 3/4] copy-prop: Remove unused self-assignment elimination The assignments that are starting point for the copy propagation are removed separately. The copy propagation does not introduce self-assignments otherwise, so the code for self-assignments elimination is functionally inert. Remove it. No functional changes intended. --- compiler/rustc_mir/src/transform/copy_prop.rs | 44 ------------------- 1 file changed, 44 deletions(-) diff --git a/compiler/rustc_mir/src/transform/copy_prop.rs b/compiler/rustc_mir/src/transform/copy_prop.rs index c14e61096195d..df518d730c895 100644 --- a/compiler/rustc_mir/src/transform/copy_prop.rs +++ b/compiler/rustc_mir/src/transform/copy_prop.rs @@ -43,9 +43,6 @@ impl<'tcx> MirPass<'tcx> for CopyPropagation { for dest_local in body.local_decls.indices() { if changed { def_use_analysis.analyze(body); - if eliminate_self_assignments(body, &def_use_analysis) { - def_use_analysis.analyze(body); - } changed = false; } @@ -147,47 +144,6 @@ impl<'tcx> MirPass<'tcx> for CopyPropagation { } } -fn eliminate_self_assignments(body: &mut Body<'_>, def_use_analysis: &DefUseAnalysis) -> bool { - let mut changed = false; - - for dest_local in body.local_decls.indices() { - let dest_use_info = def_use_analysis.local_info(dest_local); - - for def in dest_use_info.defs_not_including_drop() { - let location = def.location; - if let Some(stmt) = body[location.block].statements.get(location.statement_index) { - match &stmt.kind { - StatementKind::Assign(box ( - place, - Rvalue::Use(Operand::Copy(src_place) | Operand::Move(src_place)), - )) => { - if let (Some(local), Some(src_local)) = - (place.as_local(), src_place.as_local()) - { - if local == dest_local && dest_local == src_local { - } else { - continue; - } - } else { - continue; - } - } - _ => { - continue; - } - } - } else { - continue; - } - debug!("deleting a self-assignment for {:?}", dest_local); - body.make_statement_nop(location); - changed = true; - } - } - - changed -} - enum Action<'tcx> { PropagateLocalCopy(Local), PropagateConstant(Constant<'tcx>), From 232adee6169ed779faddcf5ddf491c058d93c782 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mi=C4=85sko?= Date: Fri, 11 Sep 2020 00:00:00 +0000 Subject: [PATCH 4/4] copy-prop: Online def-use analysis for copy propagation pass Implement an online def-use analysis for the copy propagation pass. The analysis is updated in an incremental fashion as changes are made to the MIR body instead of being recomputed from scratch after each change. The analysis is intended to be equivalent to the previous one, but it also offers a new methods to: remove specific statement, remove storage markers, and replace the uses of a local with another local or a constant, all while keeping the analysis up-to date. No functional changes intended. --- compiler/rustc_mir/src/transform/copy_prop.rs | 321 ++++++-------- compiler/rustc_mir/src/util/def_use.rs | 405 ++++++++++++++---- 2 files changed, 457 insertions(+), 269 deletions(-) diff --git a/compiler/rustc_mir/src/transform/copy_prop.rs b/compiler/rustc_mir/src/transform/copy_prop.rs index df518d730c895..5edcfd4d53901 100644 --- a/compiler/rustc_mir/src/transform/copy_prop.rs +++ b/compiler/rustc_mir/src/transform/copy_prop.rs @@ -21,7 +21,6 @@ use crate::transform::{MirPass, MirSource}; use crate::util::def_use::DefUseAnalysis; -use rustc_middle::mir::visit::MutVisitor; use rustc_middle::mir::{ Body, Constant, Local, LocalKind, Location, Operand, Place, Rvalue, StatementKind, }; @@ -30,7 +29,7 @@ use rustc_middle::ty::TyCtxt; pub struct CopyPropagation; impl<'tcx> MirPass<'tcx> for CopyPropagation { - fn run_pass(&self, tcx: TyCtxt<'tcx>, _source: MirSource<'tcx>, body: &mut Body<'tcx>) { + fn run_pass(&self, tcx: TyCtxt<'tcx>, source: MirSource<'tcx>, body: &mut Body<'tcx>) { // We only run when the MIR optimization level is > 1. // This avoids a slow pass, and messing up debug info. if tcx.sess.opts.debugging_opts.mir_opt_level <= 1 { @@ -38,122 +37,143 @@ impl<'tcx> MirPass<'tcx> for CopyPropagation { } let mut def_use_analysis = DefUseAnalysis::new(body); - let mut changed = true; + let mut propagated_local = 0; + let mut propagated_const = 0; - for dest_local in body.local_decls.indices() { - if changed { - def_use_analysis.analyze(body); - changed = false; + for dest_local in def_use_analysis.body().local_decls.indices() { + let action = match prepare_action(&mut def_use_analysis, dest_local) { + None => continue, + Some(action) => action, + }; + if action.perform(tcx, &mut def_use_analysis) { + match action { + Action::PropagateLocalCopy { .. } => propagated_local += 1, + Action::PropagateConstant { .. } => propagated_const += 1, + } } + } - debug!("considering destination local: {:?}", dest_local); + let propagated = propagated_local + propagated_const; + info!( + "{:?} propagated={} local={} const={}", + source.instance.def_id(), + propagated, + propagated_local, + propagated_const + ); + } +} - let action; - let location; - { - // The destination must have exactly one def. - let dest_use_info = def_use_analysis.local_info(dest_local); - let dest_def_count = dest_use_info.def_count_not_including_drop(); - if dest_def_count == 0 { - debug!(" Can't copy-propagate local: dest {:?} undefined", dest_local); - continue; - } - if dest_def_count > 1 { - debug!( - " Can't copy-propagate local: dest {:?} defined {} times", - dest_local, - dest_use_info.def_count() - ); - continue; - } - if dest_use_info.use_count() == 0 { - debug!(" Can't copy-propagate local: dest {:?} unused", dest_local); - continue; - } - // Conservatively gives up if the dest is an argument, - // because there may be uses of the original argument value. - // Also gives up on the return place, as we cannot propagate into its implicit - // use by `return`. - if matches!( - body.local_kind(dest_local), - LocalKind::Arg | LocalKind::ReturnPointer - ) { - debug!(" Can't copy-propagate local: dest {:?} (argument)", dest_local); - continue; - } - let dest_place_def = dest_use_info.defs_not_including_drop().next().unwrap(); - location = dest_place_def.location; +fn prepare_action<'tcx, 'a>( + def_use_analysis: &mut DefUseAnalysis<'tcx, 'a>, + dest_local: Local, +) -> Option> { + debug!("considering destination local: {:?}", dest_local); - let basic_block = &body[location.block]; - let statement_index = location.statement_index; - let statement = match basic_block.statements.get(statement_index) { - Some(statement) => statement, - None => { - debug!(" Can't copy-propagate local: used in terminator"); - continue; - } - }; + // The destination must have exactly one def. + let dest_use_info = def_use_analysis.local_info(dest_local); + let dest_def_count = dest_use_info.def_count_not_including_drop(); + if dest_def_count == 0 { + debug!(" Can't copy-propagate local: dest {:?} undefined", dest_local); + return None; + } + if dest_def_count > 1 { + debug!( + " Can't copy-propagate local: dest {:?} defined {} times", + dest_local, + dest_use_info.def_count() + ); + return None; + } + if dest_use_info.use_count() == 0 { + debug!(" Can't copy-propagate local: dest {:?} unused", dest_local); + return None; + } + // Conservatively gives up if the dest is an argument, + // because there may be uses of the original argument value. + // Also gives up on the return place, as we cannot propagate into its implicit + // use by `return`. + if matches!( + def_use_analysis.body().local_kind(dest_local), + LocalKind::Arg | LocalKind::ReturnPointer + ) { + debug!(" Can't copy-propagate local: dest {:?} (argument)", dest_local); + return None; + } - // That use of the source must be an assignment. - match &statement.kind { - StatementKind::Assign(box (place, Rvalue::Use(operand))) => { - if let Some(local) = place.as_local() { - if local == dest_local { - let maybe_action = match operand { - Operand::Copy(src_place) | Operand::Move(src_place) => { - Action::local_copy(&body, &def_use_analysis, *src_place) - } - Operand::Constant(ref src_constant) => { - Action::constant(src_constant) - } - }; - match maybe_action { - Some(this_action) => action = this_action, - None => continue, - } - } else { - debug!( - " Can't copy-propagate local: source use is not an \ - assignment" - ); - continue; + let mut defs = dest_use_info.defs(); + loop { + let location = match defs.next() { + None => return None, + Some(def) => def, + }; + let basic_block = &def_use_analysis.body()[location.block]; + let statement = match basic_block.statements.get(location.statement_index) { + Some(statement) => statement, + None => { + debug!(" Can't copy-propagate local: used in terminator"); + continue; + } + }; + // That use of the source must be an assignment. + match &statement.kind { + StatementKind::Assign(box (place, Rvalue::Use(operand))) => { + if let Some(local) = place.as_local() { + if local == dest_local { + let maybe_action = match operand { + Operand::Copy(src_place) | Operand::Move(src_place) => { + Action::local_copy( + def_use_analysis, + *src_place, + dest_local, + location, + ) } - } else { - debug!( - " Can't copy-propagate local: source use is not an \ - assignment" - ); - continue; + Operand::Constant(ref src_constant) => { + Action::constant(src_constant, dest_local, location) + } + }; + match maybe_action { + Some(action) => return Some(action), + None => continue, } - } - _ => { + } else { debug!( " Can't copy-propagate local: source use is not an \ - assignment" + assignment" ); continue; } + } else { + debug!( + " Can't copy-propagate local: source use is not an \ + assignment" + ); + continue; } } - - // FIXME(pcwalton): Update the use-def chains to delete the instructions instead of - // regenerating the chains. - action.perform(body, &def_use_analysis, dest_local, location, tcx); - changed = true; + _ => { + debug!( + " Can't copy-propagate local: source use is not an \ + assignment" + ); + continue; + } } } } enum Action<'tcx> { - PropagateLocalCopy(Local), - PropagateConstant(Constant<'tcx>), + PropagateLocalCopy { src_local: Local, dest_local: Local, location: Location }, + PropagateConstant { src_constant: Constant<'tcx>, dest_local: Local, location: Location }, } impl<'tcx> Action<'tcx> { - fn local_copy( - body: &Body<'tcx>, - def_use_analysis: &DefUseAnalysis, + fn local_copy<'a>( + def_use_analysis: &DefUseAnalysis<'tcx, 'a>, src_place: Place<'tcx>, + dest_local: Local, + location: Location, ) -> Option> { // The source must be a local. let src_local = if let Some(local) = src_place.as_local() { @@ -190,7 +210,7 @@ impl<'tcx> Action<'tcx> { // USE(SRC); let src_def_count = src_use_info.def_count_not_including_drop(); // allow function arguments to be propagated - let is_arg = body.local_kind(src_local) == LocalKind::Arg; + let is_arg = def_use_analysis.body().local_kind(src_local) == LocalKind::Arg; if (is_arg && src_def_count != 0) || (!is_arg && src_def_count != 1) { debug!( " Can't copy-propagate local: {} defs of src{}", @@ -200,50 +220,44 @@ impl<'tcx> Action<'tcx> { return None; } - Some(Action::PropagateLocalCopy(src_local)) + Some(Action::PropagateLocalCopy { src_local, dest_local, location }) } - fn constant(src_constant: &Constant<'tcx>) -> Option> { - Some(Action::PropagateConstant(*src_constant)) - } - - fn perform( - self, - body: &mut Body<'tcx>, - def_use_analysis: &DefUseAnalysis, + fn constant( + src_constant: &Constant<'tcx>, dest_local: Local, location: Location, + ) -> Option> { + Some(Action::PropagateConstant { src_constant: *src_constant, dest_local, location }) + } + + fn perform<'a>( + &self, tcx: TyCtxt<'tcx>, + def_use_analysis: &mut DefUseAnalysis<'tcx, 'a>, ) -> bool { - match self { - Action::PropagateLocalCopy(src_local) => { + match *self { + Action::PropagateLocalCopy { src_local, dest_local, location } => { // Eliminate the destination and the assignment. // // First, remove all markers. // // FIXME(pcwalton): Don't do this. Merge live ranges instead. debug!(" Replacing all uses of {:?} with {:?} (local)", dest_local, src_local); - for place_use in &def_use_analysis.local_info(dest_local).defs_and_uses { - if place_use.context.is_storage_marker() { - body.make_statement_nop(place_use.location) - } - } - for place_use in &def_use_analysis.local_info(src_local).defs_and_uses { - if place_use.context.is_storage_marker() { - body.make_statement_nop(place_use.location) - } - } + def_use_analysis.remove_storage_markers(dest_local); + def_use_analysis.remove_storage_markers(src_local); // Replace all uses of the destination local with the source local. - def_use_analysis.replace_all_defs_and_uses_with(dest_local, body, src_local, tcx); + def_use_analysis.replace_with_local(tcx, dest_local, src_local); // Finally, zap the now-useless assignment instruction. debug!(" Deleting assignment"); - body.make_statement_nop(location); + def_use_analysis.remove_statement(location); true } - Action::PropagateConstant(src_constant) => { + Action::PropagateConstant { src_constant, dest_local, location } => { + let old_use_count = def_use_analysis.local_info(dest_local).use_count(); // First, remove all markers. // // FIXME(pcwalton): Don't do this. Merge live ranges instead. @@ -251,37 +265,30 @@ impl<'tcx> Action<'tcx> { " Replacing all uses of {:?} with {:?} (constant)", dest_local, src_constant ); - let dest_local_info = def_use_analysis.local_info(dest_local); - for place_use in &dest_local_info.defs_and_uses { - if place_use.context.is_storage_marker() { - body.make_statement_nop(place_use.location) - } - } + def_use_analysis.remove_storage_markers(dest_local); // Replace all uses of the destination local with the constant. - let mut visitor = ConstantPropagationVisitor::new(dest_local, src_constant, tcx); - for dest_place_use in &dest_local_info.defs_and_uses { - visitor.visit_location(body, dest_place_use.location) - } + def_use_analysis.replace_with_constant(tcx, dest_local, src_constant); // Zap the assignment instruction if we eliminated all the uses. We won't have been // able to do that if the destination was used in a projection, because projections // must have places on their LHS. - let use_count = dest_local_info.use_count(); - if visitor.uses_replaced == use_count { + let new_use_count = def_use_analysis.local_info(dest_local).use_count(); + let uses_replaced = old_use_count - new_use_count; + if new_use_count == 0 { debug!( " {} of {} use(s) replaced; deleting assignment", - visitor.uses_replaced, use_count + uses_replaced, old_use_count ); - body.make_statement_nop(location); + def_use_analysis.remove_statement(location); true - } else if visitor.uses_replaced == 0 { + } else if uses_replaced == 0 { debug!(" No uses replaced; not deleting assignment"); false } else { debug!( " {} of {} use(s) replaced; not deleting assignment", - visitor.uses_replaced, use_count + uses_replaced, old_use_count ); true } @@ -289,47 +296,3 @@ impl<'tcx> Action<'tcx> { } } } - -struct ConstantPropagationVisitor<'tcx> { - dest_local: Local, - constant: Constant<'tcx>, - tcx: TyCtxt<'tcx>, - uses_replaced: usize, -} - -impl<'tcx> ConstantPropagationVisitor<'tcx> { - fn new( - dest_local: Local, - constant: Constant<'tcx>, - tcx: TyCtxt<'tcx>, - ) -> ConstantPropagationVisitor<'tcx> { - ConstantPropagationVisitor { dest_local, constant, tcx, uses_replaced: 0 } - } -} - -impl<'tcx> MutVisitor<'tcx> for ConstantPropagationVisitor<'tcx> { - fn tcx(&self) -> TyCtxt<'tcx> { - self.tcx - } - - fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) { - self.super_operand(operand, location); - - match operand { - Operand::Copy(place) | Operand::Move(place) => { - if let Some(local) = place.as_local() { - if local == self.dest_local { - } else { - return; - } - } else { - return; - } - } - _ => return, - } - - *operand = Operand::Constant(box self.constant); - self.uses_replaced += 1 - } -} diff --git a/compiler/rustc_mir/src/util/def_use.rs b/compiler/rustc_mir/src/util/def_use.rs index b4448ead8eb81..5f5d1cd089a7b 100644 --- a/compiler/rustc_mir/src/util/def_use.rs +++ b/compiler/rustc_mir/src/util/def_use.rs @@ -1,158 +1,383 @@ //! Def-use analysis. use rustc_index::vec::IndexVec; -use rustc_middle::mir::visit::{MutVisitor, PlaceContext, Visitor}; -use rustc_middle::mir::{Body, Local, Location, VarDebugInfo}; +use rustc_middle::mir::visit::{ + MutVisitor, NonMutatingUseContext, NonUseContext, PlaceContext, Visitor, +}; +use rustc_middle::mir::{Body, Constant, Local, Location, Operand, StatementKind, VarDebugInfo}; use rustc_middle::ty::TyCtxt; -use std::mem; +use std::collections::btree_map::{self, BTreeMap}; -pub struct DefUseAnalysis { +pub(crate) struct DefUseAnalysis<'tcx, 'a> { + body: &'a mut Body<'tcx>, info: IndexVec, + debug_info_uses: Vec, + statement_uses: Vec, } -#[derive(Clone)] -pub struct Info { - // FIXME(eddyb) use smallvec where possible. - pub defs_and_uses: Vec, - var_debug_info_indices: Vec, +#[derive(Clone, Default)] +pub(crate) struct Info { + drop_uses: usize, + non_mutating_uses: MultiSet, + mutating_uses: MultiSet, + storage_uses: MultiSet, + debug_info_uses: MultiSet, } -#[derive(Clone)] -pub struct Use { - pub context: PlaceContext, - pub location: Location, -} +impl DefUseAnalysis<'tcx, 'a> { + pub(crate) fn new(body: &'a mut Body<'tcx>) -> Self { + let info = IndexVec::from_elem_n(Info::default(), body.local_decls.len()); + let mut this = + DefUseAnalysis { body, info, debug_info_uses: Vec::new(), statement_uses: Vec::new() }; + let mut visitor = UseFinder::insert_into(&mut this.info); + visitor.visit_body(&this.body); + this + } -impl DefUseAnalysis { - pub fn new(body: &Body<'_>) -> DefUseAnalysis { - DefUseAnalysis { info: IndexVec::from_elem_n(Info::new(), body.local_decls.len()) } + pub(crate) fn body(&self) -> &Body<'tcx> { + self.body } - pub fn analyze(&mut self, body: &Body<'_>) { - self.clear(); + pub(crate) fn local_info(&self, local: Local) -> &Info { + &self.info[local] + } - let mut finder = DefUseFinder { - info: mem::take(&mut self.info), - var_debug_info_index: 0, - in_var_debug_info: false, + /// Removes the statement at given location, replacing it with a nop. + pub(crate) fn remove_statement(&mut self, location: Location) { + let block = &mut self.body[location.block]; + if let Some(statement) = block.statements.get_mut(location.statement_index) { + let mut visitor = UseFinder::remove_from(&mut self.info); + visitor.visit_statement(statement, location); + statement.make_nop(); + } else { + panic!("cannot remove terminator"); }; - finder.visit_body(&body); - self.info = finder.info } - fn clear(&mut self) { - for info in &mut self.info { - info.clear(); + /// Removes all storage markers associated with given local. + pub(crate) fn remove_storage_markers(&mut self, local: Local) { + let local_info = &mut self.info[local]; + for location in local_info.storage_uses.iter().copied() { + let block = &mut self.body[location.block]; + let statement = &mut block.statements[location.statement_index]; + assert!( + statement.kind == StatementKind::StorageLive(local) + || statement.kind == StatementKind::StorageDead(local) + ); + statement.make_nop(); } + local_info.storage_uses.clear(); } - pub fn local_info(&self, local: Local) -> &Info { - &self.info[local] + /// Replaces all uses of old local with a new local. + pub(crate) fn replace_with_local( + &mut self, + tcx: TyCtxt<'tcx>, + old_local: Local, + new_local: Local, + ) { + let local_info = &self.info[old_local]; + + self.debug_info_uses.clear(); + self.debug_info_uses.extend(&local_info.debug_info_uses); + + self.statement_uses.clear(); + self.statement_uses.extend(&local_info.non_mutating_uses); + self.statement_uses.extend(&local_info.mutating_uses); + self.statement_uses.extend(&local_info.storage_uses); + self.statement_uses.sort(); + self.statement_uses.dedup(); + + let mut visitor = ReplaceWithLocalVisitor::new(tcx, &mut self.info, old_local, new_local); + + for index in self.debug_info_uses.iter().copied() { + visitor.var_debug_info_index = index; + visitor.visit_var_debug_info(&mut self.body.var_debug_info[index]); + } + + for location in self.statement_uses.iter().copied() { + visitor.visit_location(&mut self.body, location); + } } - fn mutate_defs_and_uses( - &self, - local: Local, - body: &mut Body<'tcx>, - new_local: Local, + /// Replaces uses of a local with a constant whenever possible. + pub(crate) fn replace_with_constant( + &mut self, tcx: TyCtxt<'tcx>, + old_local: Local, + new_const: Constant<'tcx>, ) { - let mut visitor = MutateUseVisitor::new(local, new_local, tcx); - let info = &self.info[local]; - for place_use in &info.defs_and_uses { - visitor.visit_location(body, place_use.location) + let local_info = &self.info[old_local]; + + self.statement_uses.clear(); + self.statement_uses.extend(&local_info.non_mutating_uses); + + let mut visitor = ReplaceWithConstVisitor::new(tcx, &mut self.info, old_local, new_const); + + for location in self.statement_uses.iter().copied() { + visitor.visit_location(&mut self.body, location); } - // Update debuginfo as well, alongside defs/uses. - for &i in &info.var_debug_info_indices { - visitor.visit_var_debug_info(&mut body.var_debug_info[i]); + + // FIXME: Replace debug info uses once they support constants. + } +} + +impl Info { + pub(crate) fn def_count(&self) -> usize { + self.mutating_uses.len() + } + + pub(crate) fn def_count_not_including_drop(&self) -> usize { + self.mutating_uses.len() - self.drop_uses + } + + pub(crate) fn use_count(&self) -> usize { + self.non_mutating_uses.len() + } + + pub(crate) fn defs(&self) -> impl Iterator + '_ { + self.mutating_uses.iter().copied() + } + + fn insert_use( + &mut self, + context: PlaceContext, + location: Location, + var_debug_info_index: usize, + ) { + match context { + PlaceContext::NonMutatingUse(_) => { + self.non_mutating_uses.insert(location); + } + PlaceContext::MutatingUse(_) => { + if context.is_drop() { + self.drop_uses += 1; + } + self.mutating_uses.insert(location); + } + PlaceContext::NonUse(NonUseContext::VarDebugInfo) => { + self.debug_info_uses.insert(var_debug_info_index); + } + PlaceContext::NonUse(_) => { + assert!(context.is_storage_marker()); + self.storage_uses.insert(location); + } } } - // FIXME(pcwalton): this should update the def-use chains. - pub fn replace_all_defs_and_uses_with( - &self, - local: Local, - body: &mut Body<'tcx>, - new_local: Local, - tcx: TyCtxt<'tcx>, + fn remove_use( + self: &mut Info, + context: PlaceContext, + location: Location, + var_debug_info_index: usize, ) { - self.mutate_defs_and_uses(local, body, new_local, tcx) + match context { + PlaceContext::NonMutatingUse(_) => { + assert!(self.non_mutating_uses.remove(location)); + } + PlaceContext::MutatingUse(_) => { + if context.is_drop() { + assert!(self.drop_uses > 0); + self.drop_uses -= 1; + } + assert!(self.mutating_uses.remove(location)); + } + PlaceContext::NonUse(NonUseContext::VarDebugInfo) => { + assert!(self.debug_info_uses.remove(var_debug_info_index)); + } + PlaceContext::NonUse(_) => { + assert!(context.is_storage_marker()); + assert!(self.storage_uses.remove(location)); + } + } } } -struct DefUseFinder { - info: IndexVec, +/// A visitor that updates locals use information. +struct UseFinder<'a> { + insert: bool, + info: &'a mut IndexVec, var_debug_info_index: usize, - in_var_debug_info: bool, } -impl Visitor<'_> for DefUseFinder { +impl UseFinder<'a> { + fn insert_into(info: &'a mut IndexVec) -> UseFinder<'a> { + UseFinder { insert: true, info, var_debug_info_index: 0 } + } + + fn remove_from(info: &'a mut IndexVec) -> UseFinder<'a> { + UseFinder { insert: false, info, var_debug_info_index: 0 } + } +} + +impl Visitor<'_> for UseFinder<'a> { fn visit_local(&mut self, &local: &Local, context: PlaceContext, location: Location) { - let info = &mut self.info[local]; - if self.in_var_debug_info { - info.var_debug_info_indices.push(self.var_debug_info_index); + let local_info = &mut self.info[local]; + if self.insert { + local_info.insert_use(context, location, self.var_debug_info_index); } else { - info.defs_and_uses.push(Use { context, location }); + local_info.remove_use(context, location, self.var_debug_info_index); } } + fn visit_var_debug_info(&mut self, var_debug_info: &VarDebugInfo<'tcx>) { - assert!(!self.in_var_debug_info); - self.in_var_debug_info = true; self.super_var_debug_info(var_debug_info); - self.in_var_debug_info = false; self.var_debug_info_index += 1; } } -impl Info { - fn new() -> Info { - Info { defs_and_uses: vec![], var_debug_info_indices: vec![] } - } - - fn clear(&mut self) { - self.defs_and_uses.clear(); - self.var_debug_info_indices.clear(); - } +/// A visitor that replaces one local with another while keeping the def-use analysis up-to date. +struct ReplaceWithLocalVisitor<'tcx, 'a> { + tcx: TyCtxt<'tcx>, + info: &'a mut IndexVec, + old_local: Local, + new_local: Local, + var_debug_info_index: usize, +} - pub fn def_count(&self) -> usize { - self.defs_and_uses.iter().filter(|place_use| place_use.context.is_mutating_use()).count() +impl ReplaceWithLocalVisitor<'tcx, 'a> { + /// Replaces all uses of old local with new local. + fn new( + tcx: TyCtxt<'tcx>, + info: &'a mut IndexVec, + old_local: Local, + new_local: Local, + ) -> Self { + ReplaceWithLocalVisitor { tcx, info, old_local, new_local, var_debug_info_index: 0 } } +} - pub fn def_count_not_including_drop(&self) -> usize { - self.defs_not_including_drop().count() +impl<'tcx, 'a> MutVisitor<'tcx> for ReplaceWithLocalVisitor<'tcx, 'a> { + fn tcx(&self) -> TyCtxt<'tcx> { + self.tcx } - pub fn defs_not_including_drop(&self) -> impl Iterator { - self.defs_and_uses - .iter() - .filter(|place_use| place_use.context.is_mutating_use() && !place_use.context.is_drop()) + fn visit_local(&mut self, local: &mut Local, context: PlaceContext, location: Location) { + if *local != self.old_local { + return; + } + self.info[self.old_local].remove_use(context, location, self.var_debug_info_index); + *local = self.new_local; + self.info[self.new_local].insert_use(context, location, self.var_debug_info_index); } - pub fn use_count(&self) -> usize { - self.defs_and_uses.iter().filter(|place_use| place_use.context.is_nonmutating_use()).count() + fn visit_var_debug_info(&mut self, var_debug_info: &mut VarDebugInfo<'tcx>) { + self.super_var_debug_info(var_debug_info); } } -struct MutateUseVisitor<'tcx> { - query: Local, - new_local: Local, +/// A visitor that replaces local with constant while keeping the def-use analysis up-to date. +struct ReplaceWithConstVisitor<'tcx, 'a> { tcx: TyCtxt<'tcx>, + info: &'a mut IndexVec, + old_local: Local, + new_const: Constant<'tcx>, + var_debug_info_index: usize, } -impl MutateUseVisitor<'tcx> { - fn new(query: Local, new_local: Local, tcx: TyCtxt<'tcx>) -> MutateUseVisitor<'tcx> { - MutateUseVisitor { query, new_local, tcx } +impl<'tcx, 'a> ReplaceWithConstVisitor<'tcx, 'a> { + fn new( + tcx: TyCtxt<'tcx>, + info: &'a mut IndexVec, + old_local: Local, + new_const: Constant<'tcx>, + ) -> Self { + ReplaceWithConstVisitor { tcx, info, old_local, new_const, var_debug_info_index: 0 } } } -impl MutVisitor<'tcx> for MutateUseVisitor<'tcx> { +impl<'tcx, 'a> MutVisitor<'tcx> for ReplaceWithConstVisitor<'tcx, 'a> { fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } - fn visit_local(&mut self, local: &mut Local, _context: PlaceContext, _location: Location) { - if *local == self.query { - *local = self.new_local; + fn visit_operand(&mut self, operand: &mut Operand<'tcx>, location: Location) { + self.super_operand(operand, location); + + match operand { + Operand::Copy(place) | Operand::Move(place) => { + if let Some(local) = place.as_local() { + if local == self.old_local { + } else { + return; + } + } else { + return; + } + } + _ => return, } + + self.info[self.old_local].remove_use( + match operand { + Operand::Copy(_) => PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy), + Operand::Move(_) => PlaceContext::NonMutatingUse(NonMutatingUseContext::Move), + _ => unreachable!(), + }, + location, + self.var_debug_info_index, + ); + *operand = Operand::Constant(box self.new_const); + } +} + +struct MultiSet { + len: u32, + items: BTreeMap, +} + +impl Default for MultiSet { + fn default() -> Self { + MultiSet { len: 0, items: BTreeMap::new() } + } +} + +impl Clone for MultiSet { + fn clone(&self) -> Self { + MultiSet { len: self.len, items: self.items.clone() } + } +} + +impl<'a, T: Ord> IntoIterator for &'a MultiSet { + type Item = &'a T; + type IntoIter = btree_map::Keys<'a, T, u16>; + + fn into_iter(self) -> Self::IntoIter { + self.items.keys() + } +} + +impl MultiSet { + fn len(&self) -> usize { + self.len as usize + } + + fn clear(&mut self) { + self.len = 0; + self.items.clear(); + } + + fn insert(&mut self, value: T) { + self.len += 1; + *self.items.entry(value).or_insert(0) += 1; + } + + fn remove(&mut self, value: T) -> bool { + match self.items.entry(value) { + btree_map::Entry::Vacant(..) => false, + btree_map::Entry::Occupied(mut entry) => { + if *entry.get() == 1 { + entry.remove(); + } else { + *entry.get_mut() -= 1; + } + self.len -= 1; + true + } + } + } + + fn iter(&self) -> impl Iterator { + self.items.keys() } }