Skip to content

Commit

Permalink
Auto merge of rust-lang#112002 - saethlin:enable-sroa, r=oli-obk,scot…
Browse files Browse the repository at this point in the history 
…tmcm

Enable ScalarReplacementOfAggregates in optimized builds

Like MatchBranchSimplification, this pass is known to produce significant runtime improvements in Cranelift artifacts, and I believe based on the perf runs here that the primary effect of this pass is to empower MatchBranchSimplification. ScalarReplacementOfAggregates on its own has little effect on anything, but when this was rebased up to include rust-lang#112001 we started seeing significant and majority-positive results.

Based on the fact that we see most of the regressions in debug builds (rust-lang#112002 (comment)) and some rather significant ones in cycles and wall time, I'm only enabling this in optimized builds at the moment.
  • Loading branch information
@bors
bors committed Jun 1, 2023
2 parents fabf929 + 79ba7b3 commit 642c92e
Show file tree
Hide file tree
Showing 13 changed files with 313 additions and 342 deletions.
50 changes: 45 additions & 5 deletions compiler/rustc_mir_transform/src/sroa.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,23 +6,29 @@ use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, Ty, TyCtxt};
use rustc_mir_dataflow::value_analysis::{excluded_locals, iter_fields};
use rustc_target::abi::FieldIdx;
use rustc_target::abi::{FieldIdx, ReprFlags, FIRST_VARIANT};

pub struct ScalarReplacementOfAggregates;

impl<'tcx> MirPass<'tcx> for ScalarReplacementOfAggregates {
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
sess.mir_opt_level() >= 3
sess.mir_opt_level() >= 2
}

#[instrument(level = "debug", skip(self, tcx, body))]
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
debug!(def_id = ?body.source.def_id());

// Avoid query cycles (generators require optimized MIR for layout).
if tcx.type_of(body.source.def_id()).subst_identity().is_generator() {
return;
}

let mut excluded = excluded_locals(body);
let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());
loop {
debug!(?excluded);
let escaping = escaping_locals(&excluded, body);
let escaping = escaping_locals(tcx, param_env, &excluded, body);
debug!(?escaping);
let replacements = compute_flattening(tcx, param_env, body, escaping);
debug!(?replacements);
Expand All @@ -48,11 +54,45 @@ impl<'tcx> MirPass<'tcx> for ScalarReplacementOfAggregates {
/// - the locals is a union or an enum;
/// - the local's address is taken, and thus the relative addresses of the fields are observable to
/// client code.
fn escaping_locals(excluded: &BitSet<Local>, body: &Body<'_>) -> BitSet<Local> {
fn escaping_locals<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
excluded: &BitSet<Local>,
body: &Body<'tcx>,
) -> BitSet<Local> {
let is_excluded_ty = |ty: Ty<'tcx>| {
if ty.is_union() || ty.is_enum() {
return true;
}
if let ty::Adt(def, _substs) = ty.kind() {
if def.repr().flags.contains(ReprFlags::IS_SIMD) {
// Exclude #[repr(simd)] types so that they are not de-optimized into an array
return true;
}
// We already excluded unions and enums, so this ADT must have one variant
let variant = def.variant(FIRST_VARIANT);
if variant.fields.len() > 1 {
// If this has more than one field, it cannot be a wrapper that only provides a
// niche, so we do not want to automatically exclude it.
return false;
}
let Ok(layout) = tcx.layout_of(param_env.and(ty)) else {
// We can't get the layout
return true;
};
if layout.layout.largest_niche().is_some() {
// This type has a niche
return true;
}
}
// Default for non-ADTs
false
};

let mut set = BitSet::new_empty(body.local_decls.len());
set.insert_range(RETURN_PLACE..=Local::from_usize(body.arg_count));
for (local, decl) in body.local_decls().iter_enumerated() {
if decl.ty.is_union() || decl.ty.is_enum() || excluded.contains(local) {
if excluded.contains(local) || is_excluded_ty(decl.ty) {
set.insert(local);
}
}
Expand Down
7 changes: 4 additions & 3 deletions tests/codegen/issues/issue-105386-ub-in-debuginfo.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// compile-flags: --crate-type=lib -O -Cdebuginfo=2 -Cno-prepopulate-passes
// compile-flags: --crate-type=lib -O -Cdebuginfo=2 -Cno-prepopulate-passes -Zmir-enable-passes=-ScalarReplacementOfAggregates
// MIR SROA will decompose the closure
// min-llvm-version: 15.0 # this test uses opaque pointer notation
#![feature(stmt_expr_attributes)]

Expand All @@ -15,8 +16,8 @@ pub fn outer_function(x: S, y: S) -> usize {
// Check that we do not attempt to load from the spilled arg before it is assigned to
// when generating debuginfo.
// CHECK-LABEL: @outer_function
// CHECK: [[spill:%.*]] = alloca %"[closure@{{.*.rs}}:9:23: 9:25]"
// CHECK-NOT: [[ptr_tmp:%.*]] = getelementptr inbounds %"[closure@{{.*.rs}}:9:23: 9:25]", ptr [[spill]]
// CHECK: [[spill:%.*]] = alloca %"[closure@{{.*.rs}}:10:23: 10:25]"
// CHECK-NOT: [[ptr_tmp:%.*]] = getelementptr inbounds %"[closure@{{.*.rs}}:10:23: 10:25]", ptr [[spill]]
// CHECK-NOT: [[load:%.*]] = load ptr, ptr
// CHECK: call void @llvm.lifetime.start{{.*}}({{.*}}, ptr [[spill]])
// CHECK: [[inner:%.*]] = getelementptr inbounds %"{{.*}}", ptr [[spill]]
Expand Down
3 changes: 2 additions & 1 deletion tests/incremental/hashes/match_expressions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,8 +225,9 @@ pub fn change_mutability_of_binding_in_pattern(x: u32) -> u32 {
}
}

// Ignore optimized_mir in cfail2, the only change to optimized MIR is a span.
#[cfg(not(any(cfail1,cfail4)))]
#[rustc_clean(cfg="cfail2", except="hir_owner_nodes,optimized_mir,typeck")]
#[rustc_clean(cfg="cfail2", except="hir_owner_nodes,typeck")]
#[rustc_clean(cfg="cfail3")]
#[rustc_clean(cfg="cfail5", except="hir_owner_nodes,optimized_mir,typeck")]
#[rustc_clean(cfg="cfail6")]
Expand Down
96 changes: 44 additions & 52 deletions tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.diff
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,35 +11,33 @@
+ debug self => _3; // in scope 1 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
+ debug rhs => _4; // in scope 1 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
+ let mut _5: u16; // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ let mut _6: (u32,); // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ let mut _7: u32; // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ scope 2 {
+ scope 3 (inlined core::num::<impl u16>::unchecked_shl::conv) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ debug x => _7; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ let mut _8: std::option::Option<u16>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ let mut _9: std::result::Result<u16, std::num::TryFromIntError>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ debug x => _4; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ let mut _6: std::option::Option<u16>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ let mut _7: std::result::Result<u16, std::num::TryFromIntError>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ scope 4 {
+ scope 5 (inlined <u32 as TryInto<u16>>::try_into) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ debug self => _7; // in scope 5 at $SRC_DIR/core/src/convert/mod.rs:LL:COL
+ debug self => _4; // in scope 5 at $SRC_DIR/core/src/convert/mod.rs:LL:COL
+ scope 6 (inlined convert::num::<impl TryFrom<u32> for u16>::try_from) { // at $SRC_DIR/core/src/convert/mod.rs:LL:COL
+ debug u => _7; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ let mut _10: bool; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ let mut _11: u32; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ let mut _12: u16; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ debug u => _4; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ let mut _8: bool; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ let mut _9: u32; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ let mut _10: u16; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ }
+ }
+ scope 7 (inlined Result::<u16, TryFromIntError>::ok) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ debug self => _9; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ let mut _13: isize; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ let _14: u16; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ debug self => _7; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ let mut _11: isize; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ let _12: u16; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ scope 8 {
+ debug x => _14; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
+ debug x => _12; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
+ }
+ }
+ scope 9 (inlined #[track_caller] Option::<u16>::unwrap_unchecked) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ debug self => _8; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ let mut _15: &std::option::Option<u16>; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ let mut _16: isize; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ debug self => _6; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ let mut _13: &std::option::Option<u16>; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ let mut _14: isize; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ scope 10 {
+ debug val => _5; // in scope 10 at $SRC_DIR/core/src/option.rs:LL:COL
+ }
Expand All @@ -52,7 +50,7 @@
+ }
+ }
+ scope 12 (inlined Option::<u16>::is_some) { // at $SRC_DIR/core/src/option.rs:LL:COL
+ debug self => _15; // in scope 12 at $SRC_DIR/core/src/option.rs:LL:COL
+ debug self => _13; // in scope 12 at $SRC_DIR/core/src/option.rs:LL:COL
+ }
+ }
+ }
Expand All @@ -70,18 +68,14 @@
- // + span: $DIR/unchecked_shifts.rs:11:7: 11:20
- // + literal: Const { ty: unsafe fn(u16, u32) -> u16 {core::num::<impl u16>::unchecked_shl}, val: Value(<ZST>) }
+ StorageLive(_5); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_6); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ _6 = (_4,); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_7); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ _7 = move (_6.0: u32); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_8); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_9); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageLive(_11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _11 = const 65535_u32; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _10 = Gt(_7, move _11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageDead(_11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ switchInt(move _10) -> [0: bb3, otherwise: bb2]; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageLive(_6); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_7); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_8); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageLive(_9); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _9 = const 65535_u32; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _8 = Gt(_4, move _9); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageDead(_9); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ switchInt(move _8) -> [0: bb3, otherwise: bb2]; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
}

bb1: {
Expand All @@ -92,30 +86,30 @@
+ }
+
+ bb2: {
+ _9 = Result::<u16, TryFromIntError>::Err(const TryFromIntError(())); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _7 = Result::<u16, TryFromIntError>::Err(const TryFromIntError(())); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ // mir::Constant
+ // + span: no-location
+ // + literal: Const { ty: TryFromIntError, val: Value(<ZST>) }
+ goto -> bb4; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ }
+
+ bb3: {
+ StorageLive(_12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _12 = _7 as u16 (IntToInt); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _9 = Result::<u16, TryFromIntError>::Ok(move _12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageDead(_12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageLive(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _10 = _4 as u16 (IntToInt); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ _7 = Result::<u16, TryFromIntError>::Ok(move _10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageDead(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ goto -> bb4; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ }
+
+ bb4: {
+ StorageDead(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageLive(_14); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ _13 = discriminant(_9); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ switchInt(move _13) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ StorageDead(_8); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
+ StorageLive(_12); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ _11 = discriminant(_7); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ switchInt(move _11) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ }
+
+ bb5: {
+ _8 = Option::<u16>::None; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ _6 = Option::<u16>::None; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ goto -> bb8; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ }
+
Expand All @@ -124,25 +118,23 @@
+ }
+
+ bb7: {
+ _14 = move ((_9 as Ok).0: u16); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ _8 = Option::<u16>::Some(move _14); // scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
+ _12 = move ((_7 as Ok).0: u16); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ _6 = Option::<u16>::Some(move _12); // scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
+ goto -> bb8; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
+ }
+
+ bb8: {
+ StorageDead(_14); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageDead(_9); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_15); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ _16 = discriminant(_8); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ switchInt(move _16) -> [1: bb9, otherwise: bb6]; // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ StorageDead(_12); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageDead(_7); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageLive(_13); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ _14 = discriminant(_6); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ switchInt(move _14) -> [1: bb9, otherwise: bb6]; // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ }
+
+ bb9: {
+ _5 = move ((_8 as Some).0: u16); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ StorageDead(_15); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageDead(_8); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageDead(_7); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageDead(_6); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ _5 = move ((_6 as Some).0: u16); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
+ StorageDead(_13); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ StorageDead(_6); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
+ _0 = unchecked_shl::<u16>(_3, move _5) -> [return: bb1, unwind unreachable]; // scope 2 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
+ // mir::Constant
+ // + span: $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
Expand Down
Loading

0 comments on commit 642c92e

Please sign in to comment.