Skip to content

Commit 4bb96f6

Browse files
committed
Auto merge of #45575 - michaelwoerister:tweak-inline-trans-items, r=nikomatsakis
Only instantiate inline- and const-fns if they are referenced (again). It seems that we have regressed on not translating `#[inline]` functions unless they are actually used. This should bring back this optimization. I also added a regression test this time so it doesn't happen again accidentally. Fixes #40392. r? @alexcrichton UPDATE & PSA --------------------- This patch **makes translation very lazy** -- in general this is a good thing (we don't want the compiler to do unnecessary work) but it has two consequences: 1. Some error messages are only generated when an item is actually translated. Consequently, this patch will lead to more cases where the compiler will only start emitting errors when the erroneous function is actually used. This has always been true to some extend (e.g. when passing generic values to an intrinsic) but since this is something user-facing it's worth mentioning. 2. When writing tests, one has to make sure that the functions in question are actually generated. In other words, it must not be dead code. This can usually be achieved by either 1. making sure the function is exported from the resulting binary or 2. by making sure the function is called from something that is exported (or `main()`). Note that it depends on the crate type what functions are exported: 1. For rlibs and dylibs everything that is reachable from the outside is exported. 2. For executables, cdylibs, and staticlibs, items are only exported if they are additionally `#[no_mangle]` or have an `#[export_name]`. The commits in this PR contain many examples of how tests can be updated to comply to the new requirements.
2 parents f733f48 + 081ef8e commit 4bb96f6

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

54 files changed

+520
-407
lines changed

src/librustc/dep_graph/graph.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,12 @@ impl DepGraph {
328328
}
329329

330330
pub fn fingerprint_of(&self, dep_node: &DepNode) -> Fingerprint {
331-
self.fingerprints.borrow()[dep_node]
331+
match self.fingerprints.borrow().get(dep_node) {
332+
Some(&fingerprint) => fingerprint,
333+
None => {
334+
bug!("Could not find current fingerprint for {:?}", dep_node)
335+
}
336+
}
332337
}
333338

334339
pub fn prev_fingerprint_of(&self, dep_node: &DepNode) -> Option<Fingerprint> {

src/librustc_trans_utils/collector.rs

+26-8
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,8 @@ use trans_item::{TransItemExt, DefPathBasedNames, InstantiationMode};
211211

212212
use rustc_data_structures::bitvec::BitVector;
213213

214+
use syntax::attr;
215+
214216
#[derive(PartialEq, Eq, Hash, Clone, Copy, Debug)]
215217
pub enum TransItemCollectionMode {
216218
Eager,
@@ -324,9 +326,14 @@ fn collect_roots<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
324326
let mut roots = Vec::new();
325327

326328
{
329+
let entry_fn = tcx.sess.entry_fn.borrow().map(|(node_id, _)| {
330+
tcx.hir.local_def_id(node_id)
331+
});
332+
327333
let mut visitor = RootCollector {
328334
tcx,
329335
mode,
336+
entry_fn,
330337
output: &mut roots,
331338
};
332339

@@ -875,6 +882,7 @@ struct RootCollector<'b, 'a: 'b, 'tcx: 'a + 'b> {
875882
tcx: TyCtxt<'a, 'tcx, 'tcx>,
876883
mode: TransItemCollectionMode,
877884
output: &'b mut Vec<TransItem<'tcx>>,
885+
entry_fn: Option<DefId>,
878886
}
879887

880888
impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> {
@@ -932,10 +940,7 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> {
932940
let tcx = self.tcx;
933941
let def_id = tcx.hir.local_def_id(item.id);
934942

935-
if (self.mode == TransItemCollectionMode::Eager ||
936-
!tcx.is_const_fn(def_id) || tcx.is_exported_symbol(def_id)) &&
937-
!item_has_type_parameters(tcx, def_id) {
938-
943+
if self.is_root(def_id) {
939944
debug!("RootCollector: ItemFn({})",
940945
def_id_to_string(tcx, def_id));
941946

@@ -957,10 +962,7 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> {
957962
let tcx = self.tcx;
958963
let def_id = tcx.hir.local_def_id(ii.id);
959964

960-
if (self.mode == TransItemCollectionMode::Eager ||
961-
!tcx.is_const_fn(def_id) ||
962-
tcx.is_exported_symbol(def_id)) &&
963-
!item_has_type_parameters(tcx, def_id) {
965+
if self.is_root(def_id) {
964966
debug!("RootCollector: MethodImplItem({})",
965967
def_id_to_string(tcx, def_id));
966968

@@ -973,6 +975,22 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> {
973975
}
974976
}
975977

978+
impl<'b, 'a, 'v> RootCollector<'b, 'a, 'v> {
979+
fn is_root(&self, def_id: DefId) -> bool {
980+
!item_has_type_parameters(self.tcx, def_id) && match self.mode {
981+
TransItemCollectionMode::Eager => {
982+
true
983+
}
984+
TransItemCollectionMode::Lazy => {
985+
self.entry_fn == Some(def_id) ||
986+
self.tcx.is_exported_symbol(def_id) ||
987+
attr::contains_name(&self.tcx.get_attrs(def_id),
988+
"rustc_std_internal_symbol")
989+
}
990+
}
991+
}
992+
}
993+
976994
fn item_has_type_parameters<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> bool {
977995
let generics = tcx.generics_of(def_id);
978996
generics.parent_types as usize + generics.types.len() > 0

src/librustc_trans_utils/common.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ pub fn requests_inline<'a, 'tcx>(
5151
// available to normal end-users.
5252
return true
5353
}
54-
attr::requests_inline(&instance.def.attrs(tcx)[..])
54+
attr::requests_inline(&instance.def.attrs(tcx)[..]) ||
55+
tcx.is_const_fn(instance.def.def_id())
5556
}
5657

5758
pub fn is_inline_instance<'a, 'tcx>(

src/librustc_trans_utils/lib.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,12 @@ extern crate rustc_data_structures;
4040
extern crate syntax;
4141
extern crate syntax_pos;
4242

43-
use rustc::ty::TyCtxt;
43+
use rustc::ty::{TyCtxt, Instance};
4444
use rustc::hir;
4545
use rustc::hir::def_id::LOCAL_CRATE;
4646
use rustc::hir::map as hir_map;
4747
use rustc::util::nodemap::NodeSet;
4848

49-
use syntax::attr;
50-
5149
pub mod common;
5250
pub mod link;
5351
pub mod collector;
@@ -77,7 +75,7 @@ pub fn check_for_rustc_errors_attr(tcx: TyCtxt) {
7775
///
7876
/// This list is later used by linkers to determine the set of symbols needed to
7977
/// be exposed from a dynamic library and it's also encoded into the metadata.
80-
pub fn find_exported_symbols(tcx: TyCtxt) -> NodeSet {
78+
pub fn find_exported_symbols<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> NodeSet {
8179
tcx.reachable_set(LOCAL_CRATE).0.iter().cloned().filter(|&id| {
8280
// Next, we want to ignore some FFI functions that are not exposed from
8381
// this crate. Reachable FFI functions can be lumped into two
@@ -107,11 +105,10 @@ pub fn find_exported_symbols(tcx: TyCtxt) -> NodeSet {
107105
node: hir::ImplItemKind::Method(..), .. }) => {
108106
let def_id = tcx.hir.local_def_id(id);
109107
let generics = tcx.generics_of(def_id);
110-
let attributes = tcx.get_attrs(def_id);
111108
(generics.parent_types == 0 && generics.types.is_empty()) &&
112109
// Functions marked with #[inline] are only ever translated
113110
// with "internal" linkage and are never exported.
114-
!attr::requests_inline(&attributes)
111+
!common::requests_inline(tcx, &Instance::mono(tcx, def_id))
115112
}
116113

117114
_ => false
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// ignore-tidy-linelength
12+
// compile-flags:-Zprint-trans-items=lazy
13+
14+
// NB: We do not expect *any* translation item to be generated here.
15+
16+
#![feature(const_fn)]
17+
#![deny(dead_code)]
18+
#![crate_type = "rlib"]
19+
20+
pub const fn foo(x: u32) -> u32 {
21+
x + 0xf00
22+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// ignore-tidy-linelength
12+
// compile-flags:-Zprint-trans-items=lazy
13+
14+
// NB: We do not expect *any* translation item to be generated here.
15+
16+
#![deny(dead_code)]
17+
#![crate_type = "rlib"]
18+
19+
#[inline]
20+
pub fn foo() -> bool {
21+
[1, 2] == [3, 4]
22+
}
23+

src/test/codegen-units/partitioning/extern-drop-glue.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
// compile-flags:-Zinline-in-all-cgus
1717

1818
#![allow(dead_code)]
19-
#![crate_type="lib"]
19+
#![crate_type="rlib"]
2020

2121
// aux-build:cgu_extern_drop_glue.rs
2222
extern crate cgu_extern_drop_glue;
@@ -25,20 +25,20 @@ extern crate cgu_extern_drop_glue;
2525

2626
struct LocalStruct(cgu_extern_drop_glue::Struct);
2727

28-
//~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[Internal]
29-
fn user()
28+
//~ TRANS_ITEM fn extern_drop_glue::user[0] @@ extern_drop_glue[External]
29+
pub fn user()
3030
{
3131
//~ TRANS_ITEM fn core::ptr[0]::drop_in_place[0]<extern_drop_glue::LocalStruct[0]> @@ extern_drop_glue[Internal]
3232
let _ = LocalStruct(cgu_extern_drop_glue::Struct(0));
3333
}
3434

35-
mod mod1 {
35+
pub mod mod1 {
3636
use cgu_extern_drop_glue;
3737

3838
struct LocalStruct(cgu_extern_drop_glue::Struct);
3939

40-
//~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[Internal]
41-
fn user()
40+
//~ TRANS_ITEM fn extern_drop_glue::mod1[0]::user[0] @@ extern_drop_glue-mod1[External]
41+
pub fn user()
4242
{
4343
//~ TRANS_ITEM fn core::ptr[0]::drop_in_place[0]<extern_drop_glue::mod1[0]::LocalStruct[0]> @@ extern_drop_glue-mod1[Internal]
4444
let _ = LocalStruct(cgu_extern_drop_glue::Struct(0));

src/test/codegen-units/partitioning/inlining-from-extern-crate.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,10 @@ pub fn user()
3535
cgu_explicit_inlining::never_inlined();
3636
}
3737

38-
mod mod1 {
38+
pub mod mod1 {
3939
use cgu_explicit_inlining;
4040

41-
//~ TRANS_ITEM fn inlining_from_extern_crate::mod1[0]::user[0] @@ inlining_from_extern_crate-mod1[Internal]
41+
//~ TRANS_ITEM fn inlining_from_extern_crate::mod1[0]::user[0] @@ inlining_from_extern_crate-mod1[External]
4242
pub fn user()
4343
{
4444
cgu_explicit_inlining::inlined();
@@ -48,10 +48,10 @@ mod mod1 {
4848
}
4949
}
5050

51-
mod mod2 {
51+
pub mod mod2 {
5252
use cgu_explicit_inlining;
5353

54-
//~ TRANS_ITEM fn inlining_from_extern_crate::mod2[0]::user[0] @@ inlining_from_extern_crate-mod2[Internal]
54+
//~ TRANS_ITEM fn inlining_from_extern_crate::mod2[0]::user[0] @@ inlining_from_extern_crate-mod2[External]
5555
pub fn user()
5656
{
5757
cgu_explicit_inlining::always_inlined();

src/test/codegen-units/partitioning/local-drop-glue.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// compile-flags:-Zinline-in-all-cgus
1616

1717
#![allow(dead_code)]
18-
#![crate_type="lib"]
18+
#![crate_type="rlib"]
1919

2020
//~ TRANS_ITEM fn core::ptr[0]::drop_in_place[0]<local_drop_glue::Struct[0]> @@ local_drop_glue[Internal] local_drop_glue-mod1[Internal]
2121
struct Struct {
@@ -32,8 +32,8 @@ struct Outer {
3232
_a: Struct
3333
}
3434

35-
//~ TRANS_ITEM fn local_drop_glue::user[0] @@ local_drop_glue[Internal]
36-
fn user()
35+
//~ TRANS_ITEM fn local_drop_glue::user[0] @@ local_drop_glue[External]
36+
pub fn user()
3737
{
3838
let _ = Outer {
3939
_a: Struct {
@@ -42,7 +42,7 @@ fn user()
4242
};
4343
}
4444

45-
mod mod1
45+
pub mod mod1
4646
{
4747
use super::Struct;
4848

@@ -53,8 +53,8 @@ mod mod1
5353
_b: (u32, Struct),
5454
}
5555

56-
//~ TRANS_ITEM fn local_drop_glue::mod1[0]::user[0] @@ local_drop_glue-mod1[Internal]
57-
fn user()
56+
//~ TRANS_ITEM fn local_drop_glue::mod1[0]::user[0] @@ local_drop_glue-mod1[External]
57+
pub fn user()
5858
{
5959
let _ = Struct2 {
6060
_a: Struct { _a: 0 },

src/test/codegen-units/partitioning/local-inlining-but-not-all.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -27,28 +27,28 @@ mod inline {
2727
}
2828
}
2929

30-
mod user1 {
30+
pub mod user1 {
3131
use super::inline;
3232

33-
//~ TRANS_ITEM fn local_inlining_but_not_all::user1[0]::foo[0] @@ local_inlining_but_not_all-user1[Internal]
34-
fn foo() {
33+
//~ TRANS_ITEM fn local_inlining_but_not_all::user1[0]::foo[0] @@ local_inlining_but_not_all-user1[External]
34+
pub fn foo() {
3535
inline::inlined_function();
3636
}
3737
}
3838

39-
mod user2 {
39+
pub mod user2 {
4040
use super::inline;
4141

42-
//~ TRANS_ITEM fn local_inlining_but_not_all::user2[0]::bar[0] @@ local_inlining_but_not_all-user2[Internal]
43-
fn bar() {
42+
//~ TRANS_ITEM fn local_inlining_but_not_all::user2[0]::bar[0] @@ local_inlining_but_not_all-user2[External]
43+
pub fn bar() {
4444
inline::inlined_function();
4545
}
4646
}
4747

48-
mod non_user {
48+
pub mod non_user {
4949

50-
//~ TRANS_ITEM fn local_inlining_but_not_all::non_user[0]::baz[0] @@ local_inlining_but_not_all-non_user[Internal]
51-
fn baz() {
50+
//~ TRANS_ITEM fn local_inlining_but_not_all::non_user[0]::baz[0] @@ local_inlining_but_not_all-non_user[External]
51+
pub fn baz() {
5252

5353
}
5454
}

src/test/codegen-units/partitioning/local-inlining.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -28,28 +28,28 @@ mod inline {
2828
}
2929
}
3030

31-
mod user1 {
31+
pub mod user1 {
3232
use super::inline;
3333

34-
//~ TRANS_ITEM fn local_inlining::user1[0]::foo[0] @@ local_inlining-user1[Internal]
35-
fn foo() {
34+
//~ TRANS_ITEM fn local_inlining::user1[0]::foo[0] @@ local_inlining-user1[External]
35+
pub fn foo() {
3636
inline::inlined_function();
3737
}
3838
}
3939

40-
mod user2 {
40+
pub mod user2 {
4141
use super::inline;
4242

43-
//~ TRANS_ITEM fn local_inlining::user2[0]::bar[0] @@ local_inlining-user2[Internal]
44-
fn bar() {
43+
//~ TRANS_ITEM fn local_inlining::user2[0]::bar[0] @@ local_inlining-user2[External]
44+
pub fn bar() {
4545
inline::inlined_function();
4646
}
4747
}
4848

49-
mod non_user {
49+
pub mod non_user {
5050

51-
//~ TRANS_ITEM fn local_inlining::non_user[0]::baz[0] @@ local_inlining-non_user[Internal]
52-
fn baz() {
51+
//~ TRANS_ITEM fn local_inlining::non_user[0]::baz[0] @@ local_inlining-non_user[External]
52+
pub fn baz() {
5353

5454
}
5555
}

src/test/codegen-units/partitioning/local-transitive-inlining.rs

+7-7
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
// compile-flags:-Zinline-in-all-cgus
1616

1717
#![allow(dead_code)]
18-
#![crate_type="lib"]
18+
#![crate_type="rlib"]
1919

2020
mod inline {
2121

@@ -37,19 +37,19 @@ mod direct_user {
3737
}
3838
}
3939

40-
mod indirect_user {
40+
pub mod indirect_user {
4141
use super::direct_user;
4242

43-
//~ TRANS_ITEM fn local_transitive_inlining::indirect_user[0]::bar[0] @@ local_transitive_inlining-indirect_user[Internal]
44-
fn bar() {
43+
//~ TRANS_ITEM fn local_transitive_inlining::indirect_user[0]::bar[0] @@ local_transitive_inlining-indirect_user[External]
44+
pub fn bar() {
4545
direct_user::foo();
4646
}
4747
}
4848

49-
mod non_user {
49+
pub mod non_user {
5050

51-
//~ TRANS_ITEM fn local_transitive_inlining::non_user[0]::baz[0] @@ local_transitive_inlining-non_user[Internal]
52-
fn baz() {
51+
//~ TRANS_ITEM fn local_transitive_inlining::non_user[0]::baz[0] @@ local_transitive_inlining-non_user[External]
52+
pub fn baz() {
5353

5454
}
5555
}

0 commit comments

Comments
 (0)