Skip to content

Enable ScalarReplacementOfAggregates in optimized builds #112002

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 1, 2023
Merged

Enable ScalarReplacementOfAggregates in optimized builds #112002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 45 additions & 5 deletions compiler/rustc_mir_transform/src/sroa.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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