Skip to content

Commit 0075bb4

Browse files
borsbors
committed
Auto merge of #91743 - cjgillot:enable_mir_inlining_inline_all, r=oli-obk
Enable MIR inlining Continuation of #82280 by `@wesleywiser.` #82280 has shown nice compile time wins could be obtained by enabling MIR inlining. Most of the issues in #81567 are now fixed, except the interaction with polymorphization which is worked around specifically. I believe we can proceed with enabling MIR inlining in the near future (preferably just after beta branching, in case we discover new issues). Steps before merging: - [x] figure out the interaction with polymorphization; - [x] figure out how miri should deal with extern types; - [x] silence the extra arithmetic overflow warnings; - [x] remove the codegen fulfilment ICE; - [x] remove the type normalization ICEs while compiling nalgebra; - [ ] tweak the inlining threshold.
2 parents aedf78e + cbbf06b commit 0075bb4

36 files changed

+252
-170
lines changed

‎compiler/rustc_codegen_cranelift/src/base.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,7 @@ fn codegen_stmt<'tcx>(
686686
substs,
687687
ty::ClosureKind::FnOnce,
688688
)
689+
.expect("failed to normalize and resolve closure during codegen")
689690
.polymorphize(fx.tcx);
690691
let func_ref = fx.get_function_ref(instance);
691692
let func_addr = fx.bcx.ins().func_addr(fx.pointer_type, func_ref);

‎compiler/rustc_codegen_ssa/src/mir/rvalue.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
213213
substs,
214214
ty::ClosureKind::FnOnce,
215215
)
216+
.expect("failed to normalize and resolve closure during codegen")
216217
.polymorphize(bx.cx().tcx());
217218
OperandValue::Immediate(bx.cx().get_fn_addr(instance))
218219
}

‎compiler/rustc_const_eval/src/interpret/cast.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
100100
def_id,
101101
substs,
102102
ty::ClosureKind::FnOnce,
103-
);
103+
)
104+
.ok_or_else(|| err_inval!(TooGeneric))?;
104105
let fn_ptr = self.create_fn_alloc_ptr(FnVal::Instance(instance));
105106
self.write_pointer(fn_ptr, dest)?;
106107
}

‎compiler/rustc_const_eval/src/interpret/util.rs

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -44,22 +44,10 @@ where
4444
let is_used = unused_params.contains(index).map_or(true, |unused| !unused);
4545
// Only recurse when generic parameters in fns, closures and generators
4646
// are used and require substitution.
47-
match (is_used, subst.needs_subst()) {
48-
// Just in case there are closures or generators within this subst,
49-
// recurse.
50-
(true, true) => return subst.visit_with(self),
51-
// Confirm that polymorphization replaced the parameter with
52-
// `ty::Param`/`ty::ConstKind::Param`.
53-
(false, true) if cfg!(debug_assertions) => match subst.unpack() {
54-
ty::subst::GenericArgKind::Type(ty) => {
55-
assert!(matches!(ty.kind(), ty::Param(_)))
56-
}
57-
ty::subst::GenericArgKind::Const(ct) => {
58-
assert!(matches!(ct.kind(), ty::ConstKind::Param(_)))
59-
}
60-
ty::subst::GenericArgKind::Lifetime(..) => (),
61-
},
62-
_ => {}
47+
// Just in case there are closures or generators within this subst,
48+
// recurse.
49+
if is_used && subst.needs_subst() {
50+
return subst.visit_with(self);
6351
}
6452
}
6553
ControlFlow::CONTINUE

‎compiler/rustc_middle/src/ty/instance.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -496,12 +496,12 @@ impl<'tcx> Instance<'tcx> {
496496
def_id: DefId,
497497
substs: ty::SubstsRef<'tcx>,
498498
requested_kind: ty::ClosureKind,
499-
) -> Instance<'tcx> {
499+
) -> Option<Instance<'tcx>> {
500500
let actual_kind = substs.as_closure().kind();
501501

502502
match needs_fn_once_adapter_shim(actual_kind, requested_kind) {
503503
Ok(true) => Instance::fn_once_adapter_instance(tcx, def_id, substs),
504-
_ => Instance::new(def_id, substs),
504+
_ => Some(Instance::new(def_id, substs)),
505505
}
506506
}
507507

@@ -515,7 +515,7 @@ impl<'tcx> Instance<'tcx> {
515515
tcx: TyCtxt<'tcx>,
516516
closure_did: DefId,
517517
substs: ty::SubstsRef<'tcx>,
518-
) -> Instance<'tcx> {
518+
) -> Option<Instance<'tcx>> {
519519
debug!("fn_once_adapter_shim({:?}, {:?})", closure_did, substs);
520520
let fn_once = tcx.require_lang_item(LangItem::FnOnce, None);
521521
let call_once = tcx
@@ -531,12 +531,13 @@ impl<'tcx> Instance<'tcx> {
531531
let self_ty = tcx.mk_closure(closure_did, substs);
532532

533533
let sig = substs.as_closure().sig();
534-
let sig = tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), sig);
534+
let sig =
535+
tcx.try_normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), sig).ok()?;
535536
assert_eq!(sig.inputs().len(), 1);
536537
let substs = tcx.mk_substs_trait(self_ty, &[sig.inputs()[0].into()]);
537538

538539
debug!("fn_once_adapter_shim: self_ty={:?} sig={:?}", self_ty, sig);
539-
Instance { def, substs }
540+
Some(Instance { def, substs })
540541
}
541542

542543
/// Depending on the kind of `InstanceDef`, the MIR body associated with an

‎compiler/rustc_middle/src/ty/normalize_erasing_regions.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,26 @@ impl<'tcx> TyCtxt<'tcx> {
112112
self.normalize_erasing_regions(param_env, value)
113113
}
114114

115+
/// If you have a `Binder<'tcx, T>`, you can do this to strip out the
116+
/// late-bound regions and then normalize the result, yielding up
117+
/// a `T` (with regions erased). This is appropriate when the
118+
/// binder is being instantiated at the call site.
119+
///
120+
/// N.B., currently, higher-ranked type bounds inhibit
121+
/// normalization. Therefore, each time we erase them in
122+
/// codegen, we need to normalize the contents.
123+
pub fn try_normalize_erasing_late_bound_regions<T>(
124+
self,
125+
param_env: ty::ParamEnv<'tcx>,
126+
value: ty::Binder<'tcx, T>,
127+
) -> Result<T, NormalizationError<'tcx>>
128+
where
129+
T: TypeFoldable<'tcx>,
130+
{
131+
let value = self.erase_late_bound_regions(value);
132+
self.try_normalize_erasing_regions(param_env, value)
133+
}
134+
115135
/// Monomorphizes a type from the AST by first applying the
116136
/// in-scope substitutions and then normalizing any associated
117137
/// types.

‎compiler/rustc_mir_transform/src/inline.rs

Lines changed: 68 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
//! Inlining pass for MIR functions
22
use crate::deref_separator::deref_finder;
33
use rustc_attr::InlineAttr;
4+
use rustc_const_eval::transform::validate::equal_up_to_regions;
45
use rustc_index::bit_set::BitSet;
56
use rustc_index::vec::Idx;
67
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
78
use rustc_middle::mir::visit::*;
89
use rustc_middle::mir::*;
9-
use rustc_middle::traits::ObligationCause;
1010
use rustc_middle::ty::subst::Subst;
1111
use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
12+
use rustc_session::config::OptLevel;
1213
use rustc_span::{hygiene::ExpnKind, ExpnData, LocalExpnId, Span};
1314
use rustc_target::spec::abi::Abi;
1415

@@ -43,7 +44,15 @@ impl<'tcx> MirPass<'tcx> for Inline {
4344
return enabled;
4445
}
4546

46-
sess.opts.mir_opt_level() >= 3
47+
match sess.mir_opt_level() {
48+
0 | 1 => false,
49+
2 => {
50+
(sess.opts.optimize == OptLevel::Default
51+
|| sess.opts.optimize == OptLevel::Aggressive)
52+
&& sess.opts.incremental == None
53+
}
54+
_ => true,
55+
}
4756
}
4857

4958
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
@@ -76,13 +85,6 @@ fn inline<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
7685
}
7786

7887
let param_env = tcx.param_env_reveal_all_normalized(def_id);
79-
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id);
80-
let param_env = rustc_trait_selection::traits::normalize_param_env_or_error(
81-
tcx,
82-
def_id.to_def_id(),
83-
param_env,
84-
ObligationCause::misc(body.span, hir_id),
85-
);
8688

8789
let mut this = Inliner {
8890
tcx,
@@ -166,6 +168,45 @@ impl<'tcx> Inliner<'tcx> {
166168
return Err("failed to normalize callee body");
167169
};
168170

171+
// Check call signature compatibility.
172+
// Normally, this shouldn't be required, but trait normalization failure can create a
173+
// validation ICE.
174+
let terminator = caller_body[callsite.block].terminator.as_ref().unwrap();
175+
let TerminatorKind::Call { args, destination, .. } = &terminator.kind else { bug!() };
176+
let destination_ty = destination.ty(&caller_body.local_decls, self.tcx).ty;
177+
let output_type = callee_body.return_ty();
178+
if !equal_up_to_regions(self.tcx, self.param_env, output_type, destination_ty) {
179+
trace!(?output_type, ?destination_ty);
180+
return Err("failed to normalize return type");
181+
}
182+
if callsite.fn_sig.abi() == Abi::RustCall {
183+
let mut args = args.into_iter();
184+
let _ = args.next(); // Skip `self` argument.
185+
let arg_tuple_ty = args.next().unwrap().ty(&caller_body.local_decls, self.tcx);
186+
assert!(args.next().is_none());
187+
188+
let ty::Tuple(arg_tuple_tys) = arg_tuple_ty.kind() else {
189+
bug!("Closure arguments are not passed as a tuple");
190+
};
191+
192+
for (arg_ty, input) in arg_tuple_tys.iter().zip(callee_body.args_iter().skip(1)) {
193+
let input_type = callee_body.local_decls[input].ty;
194+
if !equal_up_to_regions(self.tcx, self.param_env, arg_ty, input_type) {
195+
trace!(?arg_ty, ?input_type);
196+
return Err("failed to normalize tuple argument type");
197+
}
198+
}
199+
} else {
200+
for (arg, input) in args.iter().zip(callee_body.args_iter()) {
201+
let input_type = callee_body.local_decls[input].ty;
202+
let arg_ty = arg.ty(&caller_body.local_decls, self.tcx);
203+
if !equal_up_to_regions(self.tcx, self.param_env, arg_ty, input_type) {
204+
trace!(?arg_ty, ?input_type);
205+
return Err("failed to normalize argument type");
206+
}
207+
}
208+
}
209+
169210
let old_blocks = caller_body.basic_blocks().next_index();
170211
self.inline_call(caller_body, &callsite, callee_body);
171212
let new_blocks = old_blocks..caller_body.basic_blocks().next_index();
@@ -263,6 +304,10 @@ impl<'tcx> Inliner<'tcx> {
263304
return None;
264305
}
265306

307+
if self.history.contains(&callee) {
308+
return None;
309+
}
310+
266311
let fn_sig = self.tcx.bound_fn_sig(def_id).subst(self.tcx, substs);
267312

268313
return Some(CallSite {
@@ -285,8 +330,14 @@ impl<'tcx> Inliner<'tcx> {
285330
callsite: &CallSite<'tcx>,
286331
callee_attrs: &CodegenFnAttrs,
287332
) -> Result<(), &'static str> {
288-
if let InlineAttr::Never = callee_attrs.inline {
289-
return Err("never inline hint");
333+
match callee_attrs.inline {
334+
InlineAttr::Never => return Err("never inline hint"),
335+
InlineAttr::Always | InlineAttr::Hint => {}
336+
InlineAttr::None => {
337+
if self.tcx.sess.mir_opt_level() <= 2 {
338+
return Err("at mir-opt-level=2, only #[inline] is inlined");
339+
}
340+
}
290341
}
291342

292343
// Only inline local functions if they would be eligible for cross-crate
@@ -407,22 +458,9 @@ impl<'tcx> Inliner<'tcx> {
407458
}
408459

409460
TerminatorKind::Call { func: Operand::Constant(ref f), cleanup, .. } => {
410-
if let ty::FnDef(def_id, substs) =
461+
if let ty::FnDef(def_id, _) =
411462
*callsite.callee.subst_mir(self.tcx, &f.literal.ty()).kind()
412463
{
413-
if let Ok(substs) =
414-
self.tcx.try_normalize_erasing_regions(self.param_env, substs)
415-
{
416-
if let Ok(Some(instance)) =
417-
Instance::resolve(self.tcx, self.param_env, def_id, substs)
418-
{
419-
if callsite.callee.def_id() == instance.def_id() {
420-
return Err("self-recursion");
421-
} else if self.history.contains(&instance) {
422-
return Err("already inlined");
423-
}
424-
}
425-
}
426464
// Don't give intrinsics the extra penalty for calls
427465
if tcx.is_intrinsic(def_id) {
428466
cost += INSTR_COST;
@@ -482,14 +520,12 @@ impl<'tcx> Inliner<'tcx> {
482520
if let InlineAttr::Always = callee_attrs.inline {
483521
debug!("INLINING {:?} because inline(always) [cost={}]", callsite, cost);
484522
Ok(())
523+
} else if cost <= threshold {
524+
debug!("INLINING {:?} [cost={} <= threshold={}]", callsite, cost, threshold);
525+
Ok(())
485526
} else {
486-
if cost <= threshold {
487-
debug!("INLINING {:?} [cost={} <= threshold={}]", callsite, cost, threshold);
488-
Ok(())
489-
} else {
490-
debug!("NOT inlining {:?} [cost={} > threshold={}]", callsite, cost, threshold);
491-
Err("cost above threshold")
492-
}
527+
debug!("NOT inlining {:?} [cost={} > threshold={}]", callsite, cost, threshold);
528+
Err("cost above threshold")
493529
}
494530
}
495531

‎compiler/rustc_mir_transform/src/inline/cycle.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ pub(crate) fn mir_callgraph_reachable<'tcx>(
4848
trace!(?caller, ?param_env, ?substs, "cannot normalize, skipping");
4949
continue;
5050
};
51-
let Some(callee) = ty::Instance::resolve(tcx, param_env, callee, substs).unwrap() else {
51+
let Ok(Some(callee)) = ty::Instance::resolve(tcx, param_env, callee, substs) else {
5252
trace!(?callee, "cannot resolve, skipping");
5353
continue;
5454
};

‎compiler/rustc_monomorphize/src/collector.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,8 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
730730
def_id,
731731
substs,
732732
ty::ClosureKind::FnOnce,
733-
);
733+
)
734+
.expect("failed to normalize and resolve closure during codegen");
734735
if should_codegen_locally(self.tcx, &instance) {
735736
self.output.push(create_fn_mono_item(self.tcx, instance, span));
736737
}

‎compiler/rustc_ty_utils/src/instance.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,12 +332,12 @@ fn resolve_associated_item<'tcx>(
332332
}),
333333
traits::ImplSource::Closure(closure_data) => {
334334
let trait_closure_kind = tcx.fn_trait_kind_from_lang_item(trait_id).unwrap();
335-
Some(Instance::resolve_closure(
335+
Instance::resolve_closure(
336336
tcx,
337337
closure_data.closure_def_id,
338338
closure_data.substs,
339339
trait_closure_kind,
340-
))
340+
)
341341
}
342342
traits::ImplSource::FnPointer(ref data) => match data.fn_ty.kind() {
343343
ty::FnDef(..) | ty::FnPtr(..) => Some(Instance {

‎src/test/codegen/issue-37945.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ pub fn is_empty_1(xs: Iter<f32>) -> bool {
1717
// CHECK-NEXT: start:
1818
// CHECK-NEXT: [[A:%.*]] = icmp ne {{i32\*|ptr}} %xs.1, null
1919
// CHECK-NEXT: tail call void @llvm.assume(i1 [[A]])
20-
// CHECK-NEXT: [[B:%.*]] = icmp eq {{i32\*|ptr}} %xs.0, %xs.1
20+
// CHECK-NEXT: [[B:%.*]] = icmp eq {{i32\*|ptr}} %xs.1, %xs.0
2121
// CHECK-NEXT: ret i1 [[B:%.*]]
2222
{xs}.next().is_none()
2323
}
@@ -28,7 +28,7 @@ pub fn is_empty_2(xs: Iter<f32>) -> bool {
2828
// CHECK-NEXT: start:
2929
// CHECK-NEXT: [[C:%.*]] = icmp ne {{i32\*|ptr}} %xs.1, null
3030
// CHECK-NEXT: tail call void @llvm.assume(i1 [[C]])
31-
// CHECK-NEXT: [[D:%.*]] = icmp eq {{i32\*|ptr}} %xs.0, %xs.1
31+
// CHECK-NEXT: [[D:%.*]] = icmp eq {{i32\*|ptr}} %xs.1, %xs.0
3232
// CHECK-NEXT: ret i1 [[D:%.*]]
3333
xs.map(|&x| x).next().is_none()
3434
}

‎src/test/codegen/issue-75659.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// This test checks that the call to memchr/slice_contains is optimized away
22
// when searching in small slices.
33

4-
// compile-flags: -O
4+
// compile-flags: -O -Zinline-mir=no
55
// only-x86_64
66

77
#![crate_type = "lib"]

‎src/test/codegen/mem-replace-direct-memcpy.rs

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// may e.g. multiply `size_of::<T>()` with a variable "count" (which is only
44
// known to be `1` after inlining).
55

6-
// compile-flags: -C no-prepopulate-passes
6+
// compile-flags: -C no-prepopulate-passes -Zinline-mir=no
77

88
#![crate_type = "lib"]
99

@@ -12,14 +12,12 @@ pub fn replace_byte(dst: &mut u8, src: u8) -> u8 {
1212
}
1313

1414
// NOTE(eddyb) the `CHECK-NOT`s ensure that the only calls of `@llvm.memcpy` in
15-
// the entire output, are the two direct calls we want, from `ptr::{read,write}`.
15+
// the entire output, are the two direct calls we want, from `ptr::replace`.
1616

1717
// CHECK-NOT: call void @llvm.memcpy
18-
// CHECK: ; core::ptr::read
18+
// CHECK: ; core::mem::replace
1919
// CHECK-NOT: call void @llvm.memcpy
20-
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %src, i{{.*}} 1, i1 false)
20+
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %{{.*}}, {{i8\*|ptr}} align 1 %dest, i{{.*}} 1, i1 false)
2121
// CHECK-NOT: call void @llvm.memcpy
22-
// CHECK: ; core::ptr::write
23-
// CHECK-NOT: call void @llvm.memcpy
24-
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %dst, {{i8\*|ptr}} align 1 %src, i{{.*}} 1, i1 false)
22+
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 1 %dest, {{i8\*|ptr}} align 1 %src{{.*}}, i{{.*}} 1, i1 false)
2523
// CHECK-NOT: call void @llvm.memcpy

‎src/test/codegen/remap_path_prefix/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// ignore-windows
22
//
33

4-
// compile-flags: -g -C no-prepopulate-passes --remap-path-prefix={{cwd}}=/the/cwd --remap-path-prefix={{src-base}}=/the/src
4+
// compile-flags: -g -C no-prepopulate-passes --remap-path-prefix={{cwd}}=/the/cwd --remap-path-prefix={{src-base}}=/the/src -Zinline-mir=no
55
// aux-build:remap_path_prefix_aux.rs
66

77
extern crate remap_path_prefix_aux;

‎src/test/codegen/simd-wide-sum.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,9 @@ pub fn wider_reduce_iter(x: Simd<u8, N>) -> u16 {
4747
#[no_mangle]
4848
// CHECK-LABEL: @wider_reduce_into_iter
4949
pub fn wider_reduce_into_iter(x: Simd<u8, N>) -> u16 {
50-
// CHECK: zext <8 x i8>
51-
// CHECK-SAME: to <8 x i16>
52-
// CHECK: call i16 @llvm.vector.reduce.add.v8i16(<8 x i16>
50+
// FIXME MIR inlining messes up LLVM optimizations.
51+
// WOULD-CHECK: zext <8 x i8>
52+
// WOULD-CHECK-SAME: to <8 x i16>
53+
// WOULD-CHECK: call i16 @llvm.vector.reduce.add.v8i16(<8 x i16>
5354
x.to_array().into_iter().map(u16::from).sum()
5455
}

0 commit comments

Comments
 (0)