Skip to content

Commit

Permalink
inliner: Use substs_for_mir_body
Browse files Browse the repository at this point in the history
Changes from 68965 extended the kind of instances that are being
inlined. For some of those, the `instance_mir` returns a MIR body that
is already expressed in terms of the types found in substitution array,
and doesn't need further substitution.

Use `substs_for_mir_body` to take that into account.
  • Loading branch information
tmiasko committed Nov 6, 2020
1 parent 9d78d1d commit 8a8ee1a
Show file tree
Hide file tree
Showing 9 changed files with 142 additions and 42 deletions.
12 changes: 5 additions & 7 deletions compiler/rustc_codegen_cranelift/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,11 @@ impl<'tcx, M: Module> FunctionCx<'_, 'tcx, M> {
where
T: TypeFoldable<'tcx> + Copy,
{
if let Some(substs) = self.instance.substs_for_mir_body() {
self.tcx
.subst_and_normalize_erasing_regions(substs, ty::ParamEnv::reveal_all(), value)
} else {
self.tcx
.normalize_erasing_regions(ty::ParamEnv::reveal_all(), *value)
}
self.instance.subst_mir_and_normalize_erasing_regions(
self.tcx,
ty::ParamEnv::reveal_all(),
value
)
}

pub(crate) fn clif_type(&self, ty: Ty<'tcx>) -> Option<Type> {
Expand Down
14 changes: 5 additions & 9 deletions compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,11 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
T: Copy + TypeFoldable<'tcx>,
{
debug!("monomorphize: self.instance={:?}", self.instance);
if let Some(substs) = self.instance.substs_for_mir_body() {
self.cx.tcx().subst_and_normalize_erasing_regions(
substs,
ty::ParamEnv::reveal_all(),
&value,
)
} else {
self.cx.tcx().normalize_erasing_regions(ty::ParamEnv::reveal_all(), *value)
}
self.instance.subst_mir_and_normalize_erasing_regions(
self.cx.tcx(),
ty::ParamEnv::reveal_all(),
value,
)
}
}

Expand Down
27 changes: 25 additions & 2 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::middle::codegen_fn_attrs::CodegenFnAttrFlags;
use crate::ty::print::{FmtPrinter, Printer};
use crate::ty::subst::InternalSubsts;
use crate::ty::subst::{InternalSubsts, Subst};
use crate::ty::{self, SubstsRef, Ty, TyCtxt, TypeFoldable};
use rustc_errors::ErrorReported;
use rustc_hir::def::Namespace;
Expand Down Expand Up @@ -470,10 +470,33 @@ impl<'tcx> Instance<'tcx> {
/// This function returns `Some(substs)` in the former case and `None` otherwise -- i.e., if
/// this function returns `None`, then the MIR body does not require substitution during
/// codegen.
pub fn substs_for_mir_body(&self) -> Option<SubstsRef<'tcx>> {
fn substs_for_mir_body(&self) -> Option<SubstsRef<'tcx>> {
if self.def.has_polymorphic_mir_body() { Some(self.substs) } else { None }
}

pub fn subst_mir<T>(&self, tcx: TyCtxt<'tcx>, v: &T) -> T
where
T: TypeFoldable<'tcx> + Copy,
{
if let Some(substs) = self.substs_for_mir_body() { v.subst(tcx, substs) } else { *v }
}

pub fn subst_mir_and_normalize_erasing_regions<T>(
&self,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
v: &T,
) -> T
where
T: TypeFoldable<'tcx> + Clone,
{
if let Some(substs) = self.substs_for_mir_body() {
tcx.subst_and_normalize_erasing_regions(substs, param_env, v)
} else {
tcx.normalize_erasing_regions(param_env, v.clone())
}
}

/// Returns a new `Instance` where generic parameters in `instance.substs` are replaced by
/// identify parameters if they are determined to be unused in `instance.def`.
pub fn polymorphize(self, tcx: TyCtxt<'tcx>) -> Self {
Expand Down
6 changes: 1 addition & 5 deletions compiler/rustc_mir/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -505,11 +505,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
frame: &Frame<'mir, 'tcx, M::PointerTag, M::FrameExtra>,
value: T,
) -> T {
if let Some(substs) = frame.instance.substs_for_mir_body() {
self.tcx.subst_and_normalize_erasing_regions(substs, self.param_env, &value)
} else {
self.tcx.normalize_erasing_regions(self.param_env, value)
}
frame.instance.subst_mir_and_normalize_erasing_regions(*self.tcx, self.param_env, &value)
}

/// The `substs` are assumed to already be in our interpreter "universe" (param_env).
Expand Down
10 changes: 5 additions & 5 deletions compiler/rustc_mir/src/monomorphize/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,11 +543,11 @@ impl<'a, 'tcx> MirNeighborCollector<'a, 'tcx> {
T: TypeFoldable<'tcx>,
{
debug!("monomorphize: self.instance={:?}", self.instance);
if let Some(substs) = self.instance.substs_for_mir_body() {
self.tcx.subst_and_normalize_erasing_regions(substs, ty::ParamEnv::reveal_all(), &value)
} else {
self.tcx.normalize_erasing_regions(ty::ParamEnv::reveal_all(), value)
}
self.instance.subst_mir_and_normalize_erasing_regions(
self.tcx,
ty::ParamEnv::reveal_all(),
&value,
)
}
}

Expand Down
24 changes: 10 additions & 14 deletions compiler/rustc_mir/src/transform/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ use rustc_index::vec::Idx;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_span::{hygiene::ExpnKind, ExpnData, Span};
use rustc_target::spec::abi::Abi;
Expand Down Expand Up @@ -128,17 +127,15 @@ impl Inliner<'tcx> {
self.tcx.instance_mir(callsite.callee.def)
};

let callee_body: &Body<'tcx> = &*callee_body;

let callee_body = if self.consider_optimizing(callsite, callee_body) {
self.tcx.subst_and_normalize_erasing_regions(
&callsite.callee.substs,
self.param_env,
callee_body,
)
} else {
if !self.consider_optimizing(callsite, &callee_body) {
continue;
};
}

let callee_body = callsite.callee.subst_mir_and_normalize_erasing_regions(
self.tcx,
self.param_env,
callee_body,
);

// Copy only unevaluated constants from the callee_body into the caller_body.
// Although we are only pushing `ConstKind::Unevaluated` consts to
Expand Down Expand Up @@ -317,7 +314,7 @@ impl Inliner<'tcx> {
work_list.push(target);
// If the place doesn't actually need dropping, treat it like
// a regular goto.
let ty = place.ty(callee_body, tcx).subst(tcx, callsite.callee.substs).ty;
let ty = callsite.callee.subst_mir(self.tcx, &place.ty(callee_body, tcx).ty);
if ty.needs_drop(tcx, self.param_env) {
cost += CALL_PENALTY;
if let Some(unwind) = unwind {
Expand Down Expand Up @@ -379,8 +376,7 @@ impl Inliner<'tcx> {
let ptr_size = tcx.data_layout.pointer_size.bytes();

for v in callee_body.vars_and_temps_iter() {
let v = &callee_body.local_decls[v];
let ty = v.ty.subst(tcx, callsite.callee.substs);
let ty = callsite.callee.subst_mir(self.tcx, &callee_body.local_decls[v].ty);
// Cost of the var is the size in machine-words, if we know
// it.
if let Some(size) = type_size_of(tcx, self.param_env, ty) {
Expand Down
13 changes: 13 additions & 0 deletions src/test/mir-opt/inline/inline-shims.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// ignore-wasm32-bare compiled with panic=abort by default
#![crate_type = "lib"]

// EMIT_MIR inline_shims.clone.Inline.diff
pub fn clone<A, B>(f: fn(A, B)) -> fn(A, B) {
f.clone()
}

// EMIT_MIR inline_shims.drop.Inline.diff
pub fn drop<A, B>(a: *mut Vec<A>, b: *mut Option<B>) {
unsafe { std::ptr::drop_in_place(a) }
unsafe { std::ptr::drop_in_place(b) }
}
26 changes: 26 additions & 0 deletions src/test/mir-opt/inline/inline_shims.clone.Inline.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
- // MIR for `clone` before Inline
+ // MIR for `clone` after Inline

fn clone(_1: fn(A, B)) -> fn(A, B) {
debug f => _1; // in scope 0 at $DIR/inline-shims.rs:5:20: 5:21
let mut _0: fn(A, B); // return place in scope 0 at $DIR/inline-shims.rs:5:36: 5:44
let mut _2: &fn(A, B); // in scope 0 at $DIR/inline-shims.rs:6:5: 6:6
+ scope 1 (inlined <fn(A, B) as Clone>::clone - shim(fn(A, B))) { // at $DIR/inline-shims.rs:6:5: 6:14
+ }

bb0: {
StorageLive(_2); // scope 0 at $DIR/inline-shims.rs:6:5: 6:6
_2 = &_1; // scope 0 at $DIR/inline-shims.rs:6:5: 6:6
- _0 = <fn(A, B) as Clone>::clone(move _2) -> bb1; // scope 0 at $DIR/inline-shims.rs:6:5: 6:14
- // mir::Constant
- // + span: $DIR/inline-shims.rs:6:7: 6:12
- // + literal: Const { ty: for<'r> fn(&'r fn(A, B)) -> fn(A, B) {<fn(A, B) as std::clone::Clone>::clone}, val: Value(Scalar(<ZST>)) }
- }
-
- bb1: {
+ _0 = (*_2); // scope 1 at $DIR/inline-shims.rs:6:5: 6:14
StorageDead(_2); // scope 0 at $DIR/inline-shims.rs:6:13: 6:14
return; // scope 0 at $DIR/inline-shims.rs:7:2: 7:2
}
}

52 changes: 52 additions & 0 deletions src/test/mir-opt/inline/inline_shims.drop.Inline.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
- // MIR for `drop` before Inline
+ // MIR for `drop` after Inline

fn drop(_1: *mut Vec<A>, _2: *mut Option<B>) -> () {
debug a => _1; // in scope 0 at $DIR/inline-shims.rs:10:19: 10:20
debug b => _2; // in scope 0 at $DIR/inline-shims.rs:10:35: 10:36
let mut _0: (); // return place in scope 0 at $DIR/inline-shims.rs:10:54: 10:54
let _3: (); // in scope 0 at $DIR/inline-shims.rs:11:14: 11:40
let mut _4: *mut std::vec::Vec<A>; // in scope 0 at $DIR/inline-shims.rs:11:38: 11:39
let mut _5: *mut std::option::Option<B>; // in scope 0 at $DIR/inline-shims.rs:12:38: 12:39
scope 1 {
}
scope 2 {
+ scope 3 (inlined drop_in_place::<Option<B>> - shim(Some(Option<B>))) { // at $DIR/inline-shims.rs:12:14: 12:40
+ let mut _6: isize; // in scope 3 at $DIR/inline-shims.rs:12:14: 12:40
+ let mut _7: isize; // in scope 3 at $DIR/inline-shims.rs:12:14: 12:40
+ }
}

bb0: {
StorageLive(_3); // scope 0 at $DIR/inline-shims.rs:11:5: 11:42
StorageLive(_4); // scope 1 at $DIR/inline-shims.rs:11:38: 11:39
_4 = _1; // scope 1 at $DIR/inline-shims.rs:11:38: 11:39
_3 = drop_in_place::<Vec<A>>(move _4) -> bb1; // scope 1 at $DIR/inline-shims.rs:11:14: 11:40
// mir::Constant
// + span: $DIR/inline-shims.rs:11:14: 11:37
// + literal: Const { ty: unsafe fn(*mut std::vec::Vec<A>) {std::intrinsics::drop_in_place::<std::vec::Vec<A>>}, val: Value(Scalar(<ZST>)) }
}

bb1: {
StorageDead(_4); // scope 1 at $DIR/inline-shims.rs:11:39: 11:40
StorageDead(_3); // scope 0 at $DIR/inline-shims.rs:11:41: 11:42
StorageLive(_5); // scope 2 at $DIR/inline-shims.rs:12:38: 12:39
_5 = _2; // scope 2 at $DIR/inline-shims.rs:12:38: 12:39
- _0 = drop_in_place::<Option<B>>(move _5) -> bb2; // scope 2 at $DIR/inline-shims.rs:12:14: 12:40
- // mir::Constant
- // + span: $DIR/inline-shims.rs:12:14: 12:37
- // + literal: Const { ty: unsafe fn(*mut std::option::Option<B>) {std::intrinsics::drop_in_place::<std::option::Option<B>>}, val: Value(Scalar(<ZST>)) }
+ _6 = discriminant((*_5)); // scope 3 at $DIR/inline-shims.rs:12:14: 12:40
+ switchInt(move _6) -> [0_isize: bb2, otherwise: bb3]; // scope 3 at $DIR/inline-shims.rs:12:14: 12:40
}

bb2: {
StorageDead(_5); // scope 2 at $DIR/inline-shims.rs:12:39: 12:40
return; // scope 0 at $DIR/inline-shims.rs:13:2: 13:2
+ }
+
+ bb3: {
+ drop((((*_5) as Some).0: B)) -> bb2; // scope 3 at $DIR/inline-shims.rs:12:14: 12:40
}
}

0 comments on commit 8a8ee1a

Please sign in to comment.