Skip to content

Commit 433b1bb

Browse files
saethlinsaethlin
committed
Allow inlining of non-exportable MIR into non-exported functions
1 parent b9beeae commit 433b1bb

5 files changed

+145
-50
lines changed

‎compiler/rustc_mir_transform/src/inline.rs

+33-20
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use rustc_hir::def_id::DefId;
55
use rustc_index::bit_set::BitSet;
66
use rustc_index::Idx;
77
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
8+
use rustc_middle::mir::interpret::{ConstValue, GlobalAlloc, Scalar};
89
use rustc_middle::mir::visit::*;
910
use rustc_middle::mir::*;
1011
use rustc_middle::ty::TypeVisitableExt;
@@ -87,12 +88,19 @@ fn inline<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
8788

8889
let param_env = tcx.param_env_reveal_all_normalized(def_id);
8990

91+
let codegen_fn_attrs = tcx.codegen_fn_attrs(def_id);
92+
93+
let is_exported = tcx.is_constructor(def_id.into())
94+
|| tcx.generics_of(def_id).requires_monomorphization(tcx)
95+
|| codegen_fn_attrs.requests_inline();
96+
9097
let mut this = Inliner {
9198
tcx,
9299
param_env,
93-
codegen_fn_attrs: tcx.codegen_fn_attrs(def_id),
100+
codegen_fn_attrs,
94101
history: Vec::new(),
95102
changed: false,
103+
is_exported,
96104
};
97105
let blocks = START_BLOCK..body.basic_blocks.next_index();
98106
this.process_blocks(body, blocks);
@@ -112,6 +120,7 @@ struct Inliner<'tcx> {
112120
history: Vec<DefId>,
113121
/// Indicates that the caller body has been modified.
114122
changed: bool,
123+
is_exported: bool,
115124
}
116125

117126
impl<'tcx> Inliner<'tcx> {
@@ -190,7 +199,9 @@ impl<'tcx> Inliner<'tcx> {
190199

191200
self.check_mir_is_available(caller_body, &callsite.callee)?;
192201
let callee_body = try_instance_mir(self.tcx, callsite.callee.def)?;
193-
self.check_mir_is_exportable(&callsite.callee, callee_attrs, callee_body)?;
202+
if self.is_exported {
203+
self.check_mir_is_exportable(&callsite.callee, callee_attrs, callee_body)?;
204+
}
194205
self.check_mir_body(callsite, callee_body, callee_attrs)?;
195206

196207
if !self.tcx.consider_optimizing(|| {
@@ -382,15 +393,19 @@ impl<'tcx> Inliner<'tcx> {
382393
return Ok(());
383394
}
384395

385-
// So now we are trying to inline a function from another crate which is not inline and is
386-
// not a generic. This might work. But if this pulls in a symbol from the other crater, we
387-
// will fail to link.
388-
// So this is our heuritic for handling this:
396+
// The general situation we need to avoid is this:
397+
// * We inline a non-exported function into an exported function
398+
// * The non-exported function pulls a symbol previously determined to be internal into an
399+
// exported function
400+
// * Later, we inline the exported function (which is now not exportable) into another crate
401+
// * Linker error (or ICE)
402+
//
403+
// So this is our heuristic for preventing that:
389404
// If this function has any calls in it, that might reference an internal symbol.
390405
// If this function contains a pointer constant, that might be a reference to an internal
391406
// static.
392407
// So if either of those conditions are met, we cannot inline this.
393-
if !contains_pointer_constant(callee_body) {
408+
if !contains_pointer_constant(self.tcx, callee_body) {
394409
debug!("Has no pointer constants, must be exportable");
395410
return Ok(());
396411
}
@@ -1184,17 +1199,18 @@ fn try_instance_mir<'tcx>(
11841199
}
11851200
}
11861201

1187-
fn contains_pointer_constant(body: &Body<'_>) -> bool {
1188-
let mut finder = ConstFinder { found: false };
1202+
fn contains_pointer_constant<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>) -> bool {
1203+
let mut finder = ConstFinder { tcx, found: false };
11891204
finder.visit_body(body);
11901205
finder.found
11911206
}
11921207

1193-
struct ConstFinder {
1208+
struct ConstFinder<'tcx> {
1209+
tcx: TyCtxt<'tcx>,
11941210
found: bool,
11951211
}
11961212

1197-
impl Visitor<'_> for ConstFinder {
1213+
impl<'tcx> Visitor<'_> for ConstFinder<'tcx> {
11981214
fn visit_constant(&mut self, constant: &Constant<'_>, location: Location) {
11991215
debug!("Visiting constant: {:?}", constant.literal);
12001216
if let ConstantKind::Unevaluated(..) = constant.literal {
@@ -1203,16 +1219,13 @@ impl Visitor<'_> for ConstFinder {
12031219
}
12041220

12051221
if let ConstantKind::Val(val, ty) = constant.literal {
1206-
ty::tls::with(|tcx| {
1207-
let val = tcx.lift(val).unwrap();
1208-
if let ConstValue::Scalar(Scalar::Ptr(ptr, _size)) = val {
1209-
if let Some(GlobalAlloc::Static(_)) =
1210-
tcx.try_get_global_alloc(ptr.into_parts().0)
1211-
{
1212-
self.found = true;
1213-
}
1222+
if let ConstValue::Scalar(Scalar::Ptr(ptr, _size)) = val {
1223+
if let Some(GlobalAlloc::Static(_)) =
1224+
self.tcx.try_get_global_alloc(ptr.into_parts().0)
1225+
{
1226+
self.found = true;
12141227
}
1215-
});
1228+
}
12161229

12171230
if ty.is_fn() {
12181231
self.found = true;

‎tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-abort.diff

+30-5
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,42 @@
33

44
fn main() -> () {
55
let mut _0: ();
6-
let _1: ();
7-
let mut _2: ((), u8, u8);
6+
let mut _5: ();
7+
let mut _6: u8;
8+
let mut _7: u8;
9+
scope 1 (inlined encode) {
10+
debug this => ((), u8, u8){ .0 => _5, .1 => _6, .2 => _7, };
11+
let mut _1: bool;
12+
let mut _2: bool;
13+
let mut _3: u8;
14+
let mut _4: !;
15+
}
816

917
bb0: {
18+
StorageLive(_5);
19+
StorageLive(_6);
20+
_5 = const ();
21+
_6 = const 0_u8;
22+
_7 = const 0_u8;
23+
StorageLive(_1);
1024
StorageLive(_2);
11-
_2 = (const (), const 0_u8, const 0_u8);
12-
_1 = encode(move _2) -> [return: bb1, unwind unreachable];
25+
- _2 = Eq(_7, const 0_u8);
26+
- _1 = Not(move _2);
27+
+ _2 = const true;
28+
+ _1 = const false;
29+
StorageDead(_2);
30+
- switchInt(move _1) -> [0: bb2, otherwise: bb1];
31+
+ switchInt(const false) -> [0: bb2, otherwise: bb1];
1332
}
1433

1534
bb1: {
16-
StorageDead(_2);
35+
_4 = core::panicking::panic(const "assertion failed: this.2 == 0") -> unwind unreachable;
36+
}
37+
38+
bb2: {
39+
StorageDead(_1);
40+
StorageDead(_5);
41+
StorageDead(_6);
1742
return;
1843
}
1944
}

‎tests/mir-opt/const_prop/issue_66971.main.ConstProp.panic-unwind.diff

+30-5
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,42 @@
33

44
fn main() -> () {
55
let mut _0: ();
6-
let _1: ();
7-
let mut _2: ((), u8, u8);
6+
let mut _5: ();
7+
let mut _6: u8;
8+
let mut _7: u8;
9+
scope 1 (inlined encode) {
10+
debug this => ((), u8, u8){ .0 => _5, .1 => _6, .2 => _7, };
11+
let mut _1: bool;
12+
let mut _2: bool;
13+
let mut _3: u8;
14+
let mut _4: !;
15+
}
816

917
bb0: {
18+
StorageLive(_5);
19+
StorageLive(_6);
20+
_5 = const ();
21+
_6 = const 0_u8;
22+
_7 = const 0_u8;
23+
StorageLive(_1);
1024
StorageLive(_2);
11-
_2 = (const (), const 0_u8, const 0_u8);
12-
_1 = encode(move _2) -> bb1;
25+
- _2 = Eq(_7, const 0_u8);
26+
- _1 = Not(move _2);
27+
+ _2 = const true;
28+
+ _1 = const false;
29+
StorageDead(_2);
30+
- switchInt(move _1) -> [0: bb2, otherwise: bb1];
31+
+ switchInt(const false) -> [0: bb2, otherwise: bb1];
1332
}
1433

1534
bb1: {
16-
StorageDead(_2);
35+
_4 = core::panicking::panic(const "assertion failed: this.2 == 0");
36+
}
37+
38+
bb2: {
39+
StorageDead(_1);
40+
StorageDead(_5);
41+
StorageDead(_6);
1742
return;
1843
}
1944
}

‎tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-abort.diff

+26-10
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,38 @@
33

44
fn main() -> () {
55
let mut _0: ();
6-
let _1: ();
7-
let mut _2: ((u8, u8),);
8-
let mut _3: (u8, u8);
6+
let mut _5: u8;
7+
let mut _6: u8;
8+
let mut _7: u8;
9+
let mut _8: u8;
10+
scope 1 (inlined test) {
11+
debug this => ((u8, u8),){ .0.0 => _7, .0.1 => _8, };
12+
let mut _1: bool;
13+
let mut _2: bool;
14+
let mut _3: u8;
15+
let mut _4: !;
16+
}
917

1018
bb0: {
19+
_7 = const 1_u8;
20+
_8 = const 2_u8;
21+
StorageLive(_1);
1122
StorageLive(_2);
12-
StorageLive(_3);
13-
- _3 = (const 1_u8, const 2_u8);
14-
+ _3 = const (1_u8, 2_u8);
15-
_2 = (move _3,);
16-
StorageDead(_3);
17-
_1 = test(move _2) -> [return: bb1, unwind unreachable];
23+
- _2 = Eq(_7, const 1_u8);
24+
- _1 = Not(move _2);
25+
+ _2 = const true;
26+
+ _1 = const false;
27+
StorageDead(_2);
28+
- switchInt(move _1) -> [0: bb2, otherwise: bb1];
29+
+ switchInt(const false) -> [0: bb2, otherwise: bb1];
1830
}
1931

2032
bb1: {
21-
StorageDead(_2);
33+
_4 = core::panicking::panic(const "assertion failed: (this.0).0 == 1") -> unwind unreachable;
34+
}
35+
36+
bb2: {
37+
StorageDead(_1);
2238
return;
2339
}
2440
}

‎tests/mir-opt/const_prop/issue_67019.main.ConstProp.panic-unwind.diff

+26-10
Original file line numberDiff line numberDiff line change
@@ -3,22 +3,38 @@
33

44
fn main() -> () {
55
let mut _0: ();
6-
let _1: ();
7-
let mut _2: ((u8, u8),);
8-
let mut _3: (u8, u8);
6+
let mut _5: u8;
7+
let mut _6: u8;
8+
let mut _7: u8;
9+
let mut _8: u8;
10+
scope 1 (inlined test) {
11+
debug this => ((u8, u8),){ .0.0 => _7, .0.1 => _8, };
12+
let mut _1: bool;
13+
let mut _2: bool;
14+
let mut _3: u8;
15+
let mut _4: !;
16+
}
917

1018
bb0: {
19+
_7 = const 1_u8;
20+
_8 = const 2_u8;
21+
StorageLive(_1);
1122
StorageLive(_2);
12-
StorageLive(_3);
13-
- _3 = (const 1_u8, const 2_u8);
14-
+ _3 = const (1_u8, 2_u8);
15-
_2 = (move _3,);
16-
StorageDead(_3);
17-
_1 = test(move _2) -> bb1;
23+
- _2 = Eq(_7, const 1_u8);
24+
- _1 = Not(move _2);
25+
+ _2 = const true;
26+
+ _1 = const false;
27+
StorageDead(_2);
28+
- switchInt(move _1) -> [0: bb2, otherwise: bb1];
29+
+ switchInt(const false) -> [0: bb2, otherwise: bb1];
1830
}
1931

2032
bb1: {
21-
StorageDead(_2);
33+
_4 = core::panicking::panic(const "assertion failed: (this.0).0 == 1");
34+
}
35+
36+
bb2: {
37+
StorageDead(_1);
2238
return;
2339
}
2440
}

0 commit comments

Comments
 (0)