Skip to content

Commit 885bf62

Browse files
borsbors
committed
Auto merge of #105582 - saethlin:instcombine-assert-inhabited, r=cjgillot
InstCombine away intrinsic validity assertions This optimization (currently) fires 246 times on the standard library. It seems to fire hardly at all on the big crates in the benchmark suite. Interesting.
2 parents 2a17174 + 5bfad5c commit 885bf62

File tree

12 files changed

+276
-16
lines changed

12 files changed

+276
-16
lines changed

‎compiler/rustc_codegen_cranelift/src/intrinsics/mod.rs‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ mod simd;
2121
pub(crate) use cpuid::codegen_cpuid_call;
2222
pub(crate) use llvm::codegen_llvm_intrinsic_call;
2323

24+
use rustc_middle::ty::layout::HasParamEnv;
2425
use rustc_middle::ty::print::with_no_trimmed_paths;
2526
use rustc_middle::ty::subst::SubstsRef;
2627
use rustc_span::symbol::{kw, sym, Symbol};
@@ -659,7 +660,9 @@ fn codegen_regular_intrinsic_call<'tcx>(
659660
return;
660661
}
661662

662-
if intrinsic == sym::assert_zero_valid && !fx.tcx.permits_zero_init(layout) {
663+
if intrinsic == sym::assert_zero_valid
664+
&& !fx.tcx.permits_zero_init(fx.param_env().and(layout))
665+
{
663666
with_no_trimmed_paths!({
664667
crate::base::codegen_panic(
665668
fx,
@@ -674,7 +677,7 @@ fn codegen_regular_intrinsic_call<'tcx>(
674677
}
675678

676679
if intrinsic == sym::assert_mem_uninitialized_valid
677-
&& !fx.tcx.permits_uninit_init(layout)
680+
&& !fx.tcx.permits_uninit_init(fx.param_env().and(layout))
678681
{
679682
with_no_trimmed_paths!({
680683
crate::base::codegen_panic(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -678,8 +678,8 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
678678
let layout = bx.layout_of(ty);
679679
let do_panic = match intrinsic {
680680
Inhabited => layout.abi.is_uninhabited(),
681-
ZeroValid => !bx.tcx().permits_zero_init(layout),
682-
MemUninitializedValid => !bx.tcx().permits_uninit_init(layout),
681+
ZeroValid => !bx.tcx().permits_zero_init(bx.param_env().and(layout)),
682+
MemUninitializedValid => !bx.tcx().permits_uninit_init(bx.param_env().and(layout)),
683683
};
684684
Some(if do_panic {
685685
let msg_str = with_no_visible_paths!({

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -447,7 +447,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
447447
}
448448

449449
if intrinsic_name == sym::assert_zero_valid {
450-
let should_panic = !self.tcx.permits_zero_init(layout);
450+
let should_panic = !self.tcx.permits_zero_init(self.param_env.and(layout));
451451

452452
if should_panic {
453453
M::abort(
@@ -461,7 +461,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
461461
}
462462

463463
if intrinsic_name == sym::assert_mem_uninitialized_valid {
464-
let should_panic = !self.tcx.permits_uninit_init(layout);
464+
let should_panic = !self.tcx.permits_uninit_init(self.param_env.and(layout));
465465

466466
if should_panic {
467467
M::abort(

‎compiler/rustc_const_eval/src/lib.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,12 @@ pub fn provide(providers: &mut Providers) {
5858
let (param_env, value) = param_env_and_value.into_parts();
5959
const_eval::deref_mir_constant(tcx, param_env, value)
6060
};
61-
providers.permits_uninit_init =
62-
|tcx, ty| util::might_permit_raw_init(tcx, ty, InitKind::UninitMitigated0x01Fill);
63-
providers.permits_zero_init = |tcx, ty| util::might_permit_raw_init(tcx, ty, InitKind::Zero);
61+
providers.permits_uninit_init = |tcx, param_env_and_ty| {
62+
let (param_env, ty) = param_env_and_ty.into_parts();
63+
util::might_permit_raw_init(tcx, param_env, ty, InitKind::UninitMitigated0x01Fill)
64+
};
65+
providers.permits_zero_init = |tcx, param_env_and_ty| {
66+
let (param_env, ty) = param_env_and_ty.into_parts();
67+
util::might_permit_raw_init(tcx, param_env, ty, InitKind::Zero)
68+
};
6469
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,14 @@ use crate::interpret::{InterpCx, MemoryKind, OpTy};
2020
/// to the full uninit check).
2121
pub fn might_permit_raw_init<'tcx>(
2222
tcx: TyCtxt<'tcx>,
23+
param_env: ParamEnv<'tcx>,
2324
ty: TyAndLayout<'tcx>,
2425
kind: InitKind,
2526
) -> bool {
2627
if tcx.sess.opts.unstable_opts.strict_init_checks {
2728
might_permit_raw_init_strict(ty, tcx, kind)
2829
} else {
29-
let layout_cx = LayoutCx { tcx, param_env: ParamEnv::reveal_all() };
30+
let layout_cx = LayoutCx { tcx, param_env };
3031
might_permit_raw_init_lax(ty, &layout_cx, kind)
3132
}
3233
}

‎compiler/rustc_middle/src/query/mod.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2116,12 +2116,12 @@ rustc_queries! {
21162116
separate_provide_extern
21172117
}
21182118

2119-
query permits_uninit_init(key: TyAndLayout<'tcx>) -> bool {
2120-
desc { "checking to see if `{}` permits being left uninit", key.ty }
2119+
query permits_uninit_init(key: ty::ParamEnvAnd<'tcx, TyAndLayout<'tcx>>) -> bool {
2120+
desc { "checking to see if `{}` permits being left uninit", key.value.ty }
21212121
}
21222122

2123-
query permits_zero_init(key: TyAndLayout<'tcx>) -> bool {
2124-
desc { "checking to see if `{}` permits being left zeroed", key.ty }
2123+
query permits_zero_init(key: ty::ParamEnvAnd<'tcx, TyAndLayout<'tcx>>) -> bool {
2124+
desc { "checking to see if `{}` permits being left zeroed", key.value.ty }
21252125
}
21262126

21272127
query compare_impl_const(

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use crate::ty::{self, AliasTy, InferConst, Lift, Term, TermKind, Ty, TyCtxt};
1111
use rustc_data_structures::functor::IdFunctor;
1212
use rustc_hir::def::Namespace;
1313
use rustc_index::vec::{Idx, IndexVec};
14+
use rustc_target::abi::TyAndLayout;
1415

1516
use std::fmt;
1617
use std::mem::ManuallyDrop;
@@ -853,3 +854,9 @@ impl<'tcx> TypeSuperVisitable<'tcx> for ty::UnevaluatedConst<'tcx> {
853854
self.substs.visit_with(visitor)
854855
}
855856
}
857+
858+
impl<'tcx> TypeVisitable<'tcx> for TyAndLayout<'tcx, Ty<'tcx>> {
859+
fn visit_with<V: TypeVisitor<'tcx>>(&self, visitor: &mut V) -> ControlFlow<V::BreakTy> {
860+
visitor.visit_ty(self.ty)
861+
}
862+
}

‎compiler/rustc_mir_transform/src/instcombine.rs‎

Lines changed: 84 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@ use rustc_middle::mir::{
66
BinOp, Body, Constant, ConstantKind, LocalDecls, Operand, Place, ProjectionElem, Rvalue,
77
SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp,
88
};
9-
use rustc_middle::ty::{self, TyCtxt};
9+
use rustc_middle::ty::{self, layout::TyAndLayout, ParamEnv, ParamEnvAnd, SubstsRef, Ty, TyCtxt};
10+
use rustc_span::symbol::{sym, Symbol};
1011

1112
pub struct InstCombine;
1213

@@ -16,7 +17,11 @@ impl<'tcx> MirPass<'tcx> for InstCombine {
1617
}
1718

1819
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
19-
let ctx = InstCombineContext { tcx, local_decls: &body.local_decls };
20+
let ctx = InstCombineContext {
21+
tcx,
22+
local_decls: &body.local_decls,
23+
param_env: tcx.param_env_reveal_all_normalized(body.source.def_id()),
24+
};
2025
for block in body.basic_blocks.as_mut() {
2126
for statement in block.statements.iter_mut() {
2227
match statement.kind {
@@ -33,13 +38,18 @@ impl<'tcx> MirPass<'tcx> for InstCombine {
3338
&mut block.terminator.as_mut().unwrap(),
3439
&mut block.statements,
3540
);
41+
ctx.combine_intrinsic_assert(
42+
&mut block.terminator.as_mut().unwrap(),
43+
&mut block.statements,
44+
);
3645
}
3746
}
3847
}
3948

4049
struct InstCombineContext<'tcx, 'a> {
4150
tcx: TyCtxt<'tcx>,
4251
local_decls: &'a LocalDecls<'tcx>,
52+
param_env: ParamEnv<'tcx>,
4353
}
4454

4555
impl<'tcx> InstCombineContext<'tcx, '_> {
@@ -200,4 +210,76 @@ impl<'tcx> InstCombineContext<'tcx, '_> {
200210
});
201211
terminator.kind = TerminatorKind::Goto { target: destination_block };
202212
}
213+
214+
fn combine_intrinsic_assert(
215+
&self,
216+
terminator: &mut Terminator<'tcx>,
217+
_statements: &mut Vec<Statement<'tcx>>,
218+
) {
219+
let TerminatorKind::Call { func, target, .. } = &mut terminator.kind else { return; };
220+
let Some(target_block) = target else { return; };
221+
let func_ty = func.ty(self.local_decls, self.tcx);
222+
let Some((intrinsic_name, substs)) = resolve_rust_intrinsic(self.tcx, func_ty) else {
223+
return;
224+
};
225+
// The intrinsics we are interested in have one generic parameter
226+
if substs.is_empty() {
227+
return;
228+
}
229+
let ty = substs.type_at(0);
230+
231+
// Check this is a foldable intrinsic before we query the layout of our generic parameter
232+
let Some(assert_panics) = intrinsic_assert_panics(intrinsic_name) else { return; };
233+
let Ok(layout) = self.tcx.layout_of(self.param_env.and(ty)) else { return; };
234+
if assert_panics(self.tcx, self.param_env.and(layout)) {
235+
// If we know the assert panics, indicate to later opts that the call diverges
236+
*target = None;
237+
} else {
238+
// If we know the assert does not panic, turn the call into a Goto
239+
terminator.kind = TerminatorKind::Goto { target: *target_block };
240+
}
241+
}
242+
}
243+
244+
fn intrinsic_assert_panics<'tcx>(
245+
intrinsic_name: Symbol,
246+
) -> Option<fn(TyCtxt<'tcx>, ParamEnvAnd<'tcx, TyAndLayout<'tcx>>) -> bool> {
247+
fn inhabited_predicate<'tcx>(
248+
_tcx: TyCtxt<'tcx>,
249+
param_env_and_layout: ParamEnvAnd<'tcx, TyAndLayout<'tcx>>,
250+
) -> bool {
251+
let (_param_env, layout) = param_env_and_layout.into_parts();
252+
layout.abi.is_uninhabited()
253+
}
254+
fn zero_valid_predicate<'tcx>(
255+
tcx: TyCtxt<'tcx>,
256+
param_env_and_layout: ParamEnvAnd<'tcx, TyAndLayout<'tcx>>,
257+
) -> bool {
258+
!tcx.permits_zero_init(param_env_and_layout)
259+
}
260+
fn mem_uninitialized_valid_predicate<'tcx>(
261+
tcx: TyCtxt<'tcx>,
262+
param_env_and_layout: ParamEnvAnd<'tcx, TyAndLayout<'tcx>>,
263+
) -> bool {
264+
!tcx.permits_uninit_init(param_env_and_layout)
265+
}
266+
267+
match intrinsic_name {
268+
sym::assert_inhabited => Some(inhabited_predicate),
269+
sym::assert_zero_valid => Some(zero_valid_predicate),
270+
sym::assert_mem_uninitialized_valid => Some(mem_uninitialized_valid_predicate),
271+
_ => None,
272+
}
273+
}
274+
275+
fn resolve_rust_intrinsic<'tcx>(
276+
tcx: TyCtxt<'tcx>,
277+
func_ty: Ty<'tcx>,
278+
) -> Option<(Symbol, SubstsRef<'tcx>)> {
279+
if let ty::FnDef(def_id, substs) = *func_ty.kind() {
280+
if tcx.is_intrinsic(def_id) {
281+
return Some((tcx.item_name(def_id), substs));
282+
}
283+
}
284+
None
203285
}

‎tests/mir-opt/intrinsic_asserts.generic.InstCombine.diff‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
- // MIR for `generic` before InstCombine
2+
+ // MIR for `generic` after InstCombine
3+
4+
fn generic() -> () {
5+
let mut _0: (); // return place in scope 0 at $DIR/intrinsic_asserts.rs:+0:21: +0:21
6+
let _1: (); // in scope 0 at $DIR/intrinsic_asserts.rs:+1:5: +1:46
7+
let _2: (); // in scope 0 at $DIR/intrinsic_asserts.rs:+2:5: +2:47
8+
let _3: (); // in scope 0 at $DIR/intrinsic_asserts.rs:+3:5: +3:60
9+
10+
bb0: {
11+
StorageLive(_1); // scope 0 at $DIR/intrinsic_asserts.rs:+1:5: +1:46
12+
_1 = assert_inhabited::<T>() -> bb1; // scope 0 at $DIR/intrinsic_asserts.rs:+1:5: +1:46
13+
// mir::Constant
14+
// + span: $DIR/intrinsic_asserts.rs:25:5: 25:44
15+
// + literal: Const { ty: extern "rust-intrinsic" fn() {assert_inhabited::<T>}, val: Value(<ZST>) }
16+
}
17+
18+
bb1: {
19+
StorageDead(_1); // scope 0 at $DIR/intrinsic_asserts.rs:+1:46: +1:47
20+
StorageLive(_2); // scope 0 at $DIR/intrinsic_asserts.rs:+2:5: +2:47
21+
_2 = assert_zero_valid::<T>() -> bb2; // scope 0 at $DIR/intrinsic_asserts.rs:+2:5: +2:47
22+
// mir::Constant
23+
// + span: $DIR/intrinsic_asserts.rs:26:5: 26:45
24+
// + literal: Const { ty: extern "rust-intrinsic" fn() {assert_zero_valid::<T>}, val: Value(<ZST>) }
25+
}
26+
27+
bb2: {
28+
StorageDead(_2); // scope 0 at $DIR/intrinsic_asserts.rs:+2:47: +2:48
29+
StorageLive(_3); // scope 0 at $DIR/intrinsic_asserts.rs:+3:5: +3:60
30+
_3 = assert_mem_uninitialized_valid::<T>() -> bb3; // scope 0 at $DIR/intrinsic_asserts.rs:+3:5: +3:60
31+
// mir::Constant
32+
// + span: $DIR/intrinsic_asserts.rs:27:5: 27:58
33+
// + literal: Const { ty: extern "rust-intrinsic" fn() {assert_mem_uninitialized_valid::<T>}, val: Value(<ZST>) }
34+
}
35+
36+
bb3: {
37+
StorageDead(_3); // scope 0 at $DIR/intrinsic_asserts.rs:+3:60: +3:61
38+
nop; // scope 0 at $DIR/intrinsic_asserts.rs:+0:21: +4:2
39+
return; // scope 0 at $DIR/intrinsic_asserts.rs:+4:2: +4:2
40+
}
41+
}
42+

‎tests/mir-opt/intrinsic_asserts.panics.InstCombine.diff‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
- // MIR for `panics` before InstCombine
2+
+ // MIR for `panics` after InstCombine
3+
4+
fn panics() -> () {
5+
let mut _0: (); // return place in scope 0 at $DIR/intrinsic_asserts.rs:+0:17: +0:17
6+
let _1: (); // in scope 0 at $DIR/intrinsic_asserts.rs:+1:5: +1:50
7+
let _2: (); // in scope 0 at $DIR/intrinsic_asserts.rs:+2:5: +2:49
8+
let _3: (); // in scope 0 at $DIR/intrinsic_asserts.rs:+3:5: +3:62
9+
10+
bb0: {
11+
StorageLive(_1); // scope 0 at $DIR/intrinsic_asserts.rs:+1:5: +1:50
12+
- _1 = assert_inhabited::<Never>() -> bb1; // scope 0 at $DIR/intrinsic_asserts.rs:+1:5: +1:50
13+
+ _1 = assert_inhabited::<Never>(); // scope 0 at $DIR/intrinsic_asserts.rs:+1:5: +1:50
14+
// mir::Constant
15+
// + span: $DIR/intrinsic_asserts.rs:17:5: 17:48
16+
// + literal: Const { ty: extern "rust-intrinsic" fn() {assert_inhabited::<Never>}, val: Value(<ZST>) }
17+
}
18+
19+
bb1: {
20+
StorageDead(_1); // scope 0 at $DIR/intrinsic_asserts.rs:+1:50: +1:51
21+
StorageLive(_2); // scope 0 at $DIR/intrinsic_asserts.rs:+2:5: +2:49
22+
- _2 = assert_zero_valid::<&u8>() -> bb2; // scope 0 at $DIR/intrinsic_asserts.rs:+2:5: +2:49
23+
+ _2 = assert_zero_valid::<&u8>(); // scope 0 at $DIR/intrinsic_asserts.rs:+2:5: +2:49
24+
// mir::Constant
25+
// + span: $DIR/intrinsic_asserts.rs:18:5: 18:47
26+
// + user_ty: UserType(0)
27+
// + literal: Const { ty: extern "rust-intrinsic" fn() {assert_zero_valid::<&u8>}, val: Value(<ZST>) }
28+
}
29+
30+
bb2: {
31+
StorageDead(_2); // scope 0 at $DIR/intrinsic_asserts.rs:+2:49: +2:50
32+
StorageLive(_3); // scope 0 at $DIR/intrinsic_asserts.rs:+3:5: +3:62
33+
- _3 = assert_mem_uninitialized_valid::<&u8>() -> bb3; // scope 0 at $DIR/intrinsic_asserts.rs:+3:5: +3:62
34+
+ _3 = assert_mem_uninitialized_valid::<&u8>(); // scope 0 at $DIR/intrinsic_asserts.rs:+3:5: +3:62
35+
// mir::Constant
36+
// + span: $DIR/intrinsic_asserts.rs:19:5: 19:60
37+
// + user_ty: UserType(1)
38+
// + literal: Const { ty: extern "rust-intrinsic" fn() {assert_mem_uninitialized_valid::<&u8>}, val: Value(<ZST>) }
39+
}
40+
41+
bb3: {
42+
StorageDead(_3); // scope 0 at $DIR/intrinsic_asserts.rs:+3:62: +3:63
43+
nop; // scope 0 at $DIR/intrinsic_asserts.rs:+0:17: +4:2
44+
return; // scope 0 at $DIR/intrinsic_asserts.rs:+4:2: +4:2
45+
}
46+
}
47+

0 commit comments

Comments
 (0)