Skip to content

Commit 642c92e

Browse files
borsbors
committed
Auto merge of #112002 - saethlin:enable-sroa, r=oli-obk,scottmcm
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 #112001 we started seeing significant and majority-positive results. Based on the fact that we see most of the regressions in debug builds (#112002 (comment)) and some rather significant ones in cycles and wall time, I'm only enabling this in optimized builds at the moment.
2 parents fabf929 + 79ba7b3 commit 642c92e

13 files changed

+313
-342
lines changed

‎compiler/rustc_mir_transform/src/sroa.rs

+45-5
Original file line numberDiff line numberDiff line change
@@ -6,23 +6,29 @@ use rustc_middle::mir::visit::*;
66
use rustc_middle::mir::*;
77
use rustc_middle::ty::{self, Ty, TyCtxt};
88
use rustc_mir_dataflow::value_analysis::{excluded_locals, iter_fields};
9-
use rustc_target::abi::FieldIdx;
9+
use rustc_target::abi::{FieldIdx, ReprFlags, FIRST_VARIANT};
1010

1111
pub struct ScalarReplacementOfAggregates;
1212

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

1818
#[instrument(level = "debug", skip(self, tcx, body))]
1919
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
2020
debug!(def_id = ?body.source.def_id());
21+
22+
// Avoid query cycles (generators require optimized MIR for layout).
23+
if tcx.type_of(body.source.def_id()).subst_identity().is_generator() {
24+
return;
25+
}
26+
2127
let mut excluded = excluded_locals(body);
2228
let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());
2329
loop {
2430
debug!(?excluded);
25-
let escaping = escaping_locals(&excluded, body);
31+
let escaping = escaping_locals(tcx, param_env, &excluded, body);
2632
debug!(?escaping);
2733
let replacements = compute_flattening(tcx, param_env, body, escaping);
2834
debug!(?replacements);
@@ -48,11 +54,45 @@ impl<'tcx> MirPass<'tcx> for ScalarReplacementOfAggregates {
4854
/// - the locals is a union or an enum;
4955
/// - the local's address is taken, and thus the relative addresses of the fields are observable to
5056
/// client code.
51-
fn escaping_locals(excluded: &BitSet<Local>, body: &Body<'_>) -> BitSet<Local> {
57+
fn escaping_locals<'tcx>(
58+
tcx: TyCtxt<'tcx>,
59+
param_env: ty::ParamEnv<'tcx>,
60+
excluded: &BitSet<Local>,
61+
body: &Body<'tcx>,
62+
) -> BitSet<Local> {
63+
let is_excluded_ty = |ty: Ty<'tcx>| {
64+
if ty.is_union() || ty.is_enum() {
65+
return true;
66+
}
67+
if let ty::Adt(def, _substs) = ty.kind() {
68+
if def.repr().flags.contains(ReprFlags::IS_SIMD) {
69+
// Exclude #[repr(simd)] types so that they are not de-optimized into an array
70+
return true;
71+
}
72+
// We already excluded unions and enums, so this ADT must have one variant
73+
let variant = def.variant(FIRST_VARIANT);
74+
if variant.fields.len() > 1 {
75+
// If this has more than one field, it cannot be a wrapper that only provides a
76+
// niche, so we do not want to automatically exclude it.
77+
return false;
78+
}
79+
let Ok(layout) = tcx.layout_of(param_env.and(ty)) else {
80+
// We can't get the layout
81+
return true;
82+
};
83+
if layout.layout.largest_niche().is_some() {
84+
// This type has a niche
85+
return true;
86+
}
87+
}
88+
// Default for non-ADTs
89+
false
90+
};
91+
5292
let mut set = BitSet::new_empty(body.local_decls.len());
5393
set.insert_range(RETURN_PLACE..=Local::from_usize(body.arg_count));
5494
for (local, decl) in body.local_decls().iter_enumerated() {
55-
if decl.ty.is_union() || decl.ty.is_enum() || excluded.contains(local) {
95+
if excluded.contains(local) || is_excluded_ty(decl.ty) {
5696
set.insert(local);
5797
}
5898
}

‎tests/codegen/issues/issue-105386-ub-in-debuginfo.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
// compile-flags: --crate-type=lib -O -Cdebuginfo=2 -Cno-prepopulate-passes
1+
// compile-flags: --crate-type=lib -O -Cdebuginfo=2 -Cno-prepopulate-passes -Zmir-enable-passes=-ScalarReplacementOfAggregates
2+
// MIR SROA will decompose the closure
23
// min-llvm-version: 15.0 # this test uses opaque pointer notation
34
#![feature(stmt_expr_attributes)]
45

@@ -15,8 +16,8 @@ pub fn outer_function(x: S, y: S) -> usize {
1516
// Check that we do not attempt to load from the spilled arg before it is assigned to
1617
// when generating debuginfo.
1718
// CHECK-LABEL: @outer_function
18-
// CHECK: [[spill:%.*]] = alloca %"[closure@{{.*.rs}}:9:23: 9:25]"
19-
// CHECK-NOT: [[ptr_tmp:%.*]] = getelementptr inbounds %"[closure@{{.*.rs}}:9:23: 9:25]", ptr [[spill]]
19+
// CHECK: [[spill:%.*]] = alloca %"[closure@{{.*.rs}}:10:23: 10:25]"
20+
// CHECK-NOT: [[ptr_tmp:%.*]] = getelementptr inbounds %"[closure@{{.*.rs}}:10:23: 10:25]", ptr [[spill]]
2021
// CHECK-NOT: [[load:%.*]] = load ptr, ptr
2122
// CHECK: call void @llvm.lifetime.start{{.*}}({{.*}}, ptr [[spill]])
2223
// CHECK: [[inner:%.*]] = getelementptr inbounds %"{{.*}}", ptr [[spill]]

‎tests/incremental/hashes/match_expressions.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -225,8 +225,9 @@ pub fn change_mutability_of_binding_in_pattern(x: u32) -> u32 {
225225
}
226226
}
227227

228+
// Ignore optimized_mir in cfail2, the only change to optimized MIR is a span.
228229
#[cfg(not(any(cfail1,cfail4)))]
229-
#[rustc_clean(cfg="cfail2", except="hir_owner_nodes,optimized_mir,typeck")]
230+
#[rustc_clean(cfg="cfail2", except="hir_owner_nodes,typeck")]
230231
#[rustc_clean(cfg="cfail3")]
231232
#[rustc_clean(cfg="cfail5", except="hir_owner_nodes,optimized_mir,typeck")]
232233
#[rustc_clean(cfg="cfail6")]

‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.diff

+44-52
Original file line numberDiff line numberDiff line change
@@ -11,35 +11,33 @@
1111
+ debug self => _3; // in scope 1 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
1212
+ debug rhs => _4; // in scope 1 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
1313
+ let mut _5: u16; // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
14-
+ let mut _6: (u32,); // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
15-
+ let mut _7: u32; // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
1614
+ scope 2 {
1715
+ scope 3 (inlined core::num::<impl u16>::unchecked_shl::conv) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
18-
+ debug x => _7; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
19-
+ let mut _8: std::option::Option<u16>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
20-
+ let mut _9: std::result::Result<u16, std::num::TryFromIntError>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
16+
+ debug x => _4; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
17+
+ let mut _6: std::option::Option<u16>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
18+
+ let mut _7: std::result::Result<u16, std::num::TryFromIntError>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
2119
+ scope 4 {
2220
+ scope 5 (inlined <u32 as TryInto<u16>>::try_into) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
23-
+ debug self => _7; // in scope 5 at $SRC_DIR/core/src/convert/mod.rs:LL:COL
21+
+ debug self => _4; // in scope 5 at $SRC_DIR/core/src/convert/mod.rs:LL:COL
2422
+ scope 6 (inlined convert::num::<impl TryFrom<u32> for u16>::try_from) { // at $SRC_DIR/core/src/convert/mod.rs:LL:COL
25-
+ debug u => _7; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
26-
+ let mut _10: bool; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
27-
+ let mut _11: u32; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
28-
+ let mut _12: u16; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
23+
+ debug u => _4; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
24+
+ let mut _8: bool; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
25+
+ let mut _9: u32; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
26+
+ let mut _10: u16; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
2927
+ }
3028
+ }
3129
+ scope 7 (inlined Result::<u16, TryFromIntError>::ok) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
32-
+ debug self => _9; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
33-
+ let mut _13: isize; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
34-
+ let _14: u16; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
30+
+ debug self => _7; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
31+
+ let mut _11: isize; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
32+
+ let _12: u16; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
3533
+ scope 8 {
36-
+ debug x => _14; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
34+
+ debug x => _12; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
3735
+ }
3836
+ }
3937
+ scope 9 (inlined #[track_caller] Option::<u16>::unwrap_unchecked) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
40-
+ debug self => _8; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
41-
+ let mut _15: &std::option::Option<u16>; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
42-
+ let mut _16: isize; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
38+
+ debug self => _6; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
39+
+ let mut _13: &std::option::Option<u16>; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
40+
+ let mut _14: isize; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
4341
+ scope 10 {
4442
+ debug val => _5; // in scope 10 at $SRC_DIR/core/src/option.rs:LL:COL
4543
+ }
@@ -52,7 +50,7 @@
5250
+ }
5351
+ }
5452
+ scope 12 (inlined Option::<u16>::is_some) { // at $SRC_DIR/core/src/option.rs:LL:COL
55-
+ debug self => _15; // in scope 12 at $SRC_DIR/core/src/option.rs:LL:COL
53+
+ debug self => _13; // in scope 12 at $SRC_DIR/core/src/option.rs:LL:COL
5654
+ }
5755
+ }
5856
+ }
@@ -70,18 +68,14 @@
7068
- // + span: $DIR/unchecked_shifts.rs:11:7: 11:20
7169
- // + literal: Const { ty: unsafe fn(u16, u32) -> u16 {core::num::<impl u16>::unchecked_shl}, val: Value(<ZST>) }
7270
+ StorageLive(_5); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
73-
+ StorageLive(_6); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
74-
+ _6 = (_4,); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
75-
+ StorageLive(_7); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
76-
+ _7 = move (_6.0: u32); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
77-
+ StorageLive(_8); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
78-
+ StorageLive(_9); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
79-
+ StorageLive(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
80-
+ StorageLive(_11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
81-
+ _11 = const 65535_u32; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
82-
+ _10 = Gt(_7, move _11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
83-
+ StorageDead(_11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
84-
+ switchInt(move _10) -> [0: bb3, otherwise: bb2]; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
71+
+ StorageLive(_6); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
72+
+ StorageLive(_7); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
73+
+ StorageLive(_8); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
74+
+ StorageLive(_9); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
75+
+ _9 = const 65535_u32; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
76+
+ _8 = Gt(_4, move _9); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
77+
+ StorageDead(_9); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
78+
+ switchInt(move _8) -> [0: bb3, otherwise: bb2]; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
8579
}
8680

8781
bb1: {
@@ -92,30 +86,30 @@
9286
+ }
9387
+
9488
+ bb2: {
95-
+ _9 = Result::<u16, TryFromIntError>::Err(const TryFromIntError(())); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
89+
+ _7 = Result::<u16, TryFromIntError>::Err(const TryFromIntError(())); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
9690
+ // mir::Constant
9791
+ // + span: no-location
9892
+ // + literal: Const { ty: TryFromIntError, val: Value(<ZST>) }
9993
+ goto -> bb4; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
10094
+ }
10195
+
10296
+ bb3: {
103-
+ StorageLive(_12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
104-
+ _12 = _7 as u16 (IntToInt); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
105-
+ _9 = Result::<u16, TryFromIntError>::Ok(move _12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
106-
+ StorageDead(_12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
97+
+ StorageLive(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
98+
+ _10 = _4 as u16 (IntToInt); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
99+
+ _7 = Result::<u16, TryFromIntError>::Ok(move _10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
100+
+ StorageDead(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
107101
+ goto -> bb4; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
108102
+ }
109103
+
110104
+ bb4: {
111-
+ StorageDead(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
112-
+ StorageLive(_14); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
113-
+ _13 = discriminant(_9); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
114-
+ switchInt(move _13) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
105+
+ StorageDead(_8); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
106+
+ StorageLive(_12); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
107+
+ _11 = discriminant(_7); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
108+
+ switchInt(move _11) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
115109
+ }
116110
+
117111
+ bb5: {
118-
+ _8 = Option::<u16>::None; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
112+
+ _6 = Option::<u16>::None; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
119113
+ goto -> bb8; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
120114
+ }
121115
+
@@ -124,25 +118,23 @@
124118
+ }
125119
+
126120
+ bb7: {
127-
+ _14 = move ((_9 as Ok).0: u16); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
128-
+ _8 = Option::<u16>::Some(move _14); // scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
121+
+ _12 = move ((_7 as Ok).0: u16); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
122+
+ _6 = Option::<u16>::Some(move _12); // scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
129123
+ goto -> bb8; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
130124
+ }
131125
+
132126
+ bb8: {
133-
+ StorageDead(_14); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
134-
+ StorageDead(_9); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
135-
+ StorageLive(_15); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
136-
+ _16 = discriminant(_8); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
137-
+ switchInt(move _16) -> [1: bb9, otherwise: bb6]; // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
127+
+ StorageDead(_12); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
128+
+ StorageDead(_7); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
129+
+ StorageLive(_13); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
130+
+ _14 = discriminant(_6); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
131+
+ switchInt(move _14) -> [1: bb9, otherwise: bb6]; // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
138132
+ }
139133
+
140134
+ bb9: {
141-
+ _5 = move ((_8 as Some).0: u16); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
142-
+ StorageDead(_15); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
143-
+ StorageDead(_8); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
144-
+ StorageDead(_7); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
145-
+ StorageDead(_6); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
135+
+ _5 = move ((_6 as Some).0: u16); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
136+
+ StorageDead(_13); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
137+
+ StorageDead(_6); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
146138
+ _0 = unchecked_shl::<u16>(_3, move _5) -> [return: bb1, unwind unreachable]; // scope 2 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
147139
+ // mir::Constant
148140
+ // + span: $SRC_DIR/core/src/num/uint_macros.rs:LL:COL

0 commit comments

Comments
 (0)