trans: Only instantiate #[inline] functions in codegen units referencing them #36524
Conversation
|
(rust_highfive has picked a reviewer for you, use r? to override) |
|
I just tested this PR locally, and it appears to work. According to Here is the full LLVM IR for winapi: ; ModuleID = 'winapi.cgu-0.rs'
source_filename = "winapi.cgu-0.rs"
target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-pc-windows-msvc"
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!3, !4}
!0 = distinct !DICompileUnit(language: DW_LANG_Rust, file: !1, producer: "rustc version 1.13.0-dev (cf976fe2c 2016-09-15)", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2)
!1 = !DIFile(filename: ".\5Csrc\5Clib.rs", directory: "C:\5Cmsys64\5Chome\5CPeter\5Cwinapi-rs")
!2 = !{}
!3 = !{i32 2, !"CodeView", i32 1}
!4 = !{i32 2, !"Debug Info Version", i32 3}EDIT: Removed claim about inherent methods not working. I just forgot to recompile apparently after fixing a bug with a macro. |
|
@retep998 Wow! 😲 |
|
@retep998 Phew! And I was already wondering why I couldn't reproduce your results It's to be expected that translation still takes some time, since that also includes the time for exporting metadata. |
brson
added
the
relnotes
| // This is coincidental: We could also instantiate something only if it | ||
| // is referenced (e.g. a regular, private function) but place it in its | ||
| // own codegen unit with "external" linkage. | ||
| self.is_instantiated_only_on_demand(tcx) |
There was a problem hiding this comment.
Nit: it might be nice to put a few notes here as to why things need_local_copy
|
@bors r+ |
|
📌 Commit cf976fe has been approved by |
|
⌛ Testing commit cf976fe with merge 4f363c9... |
|
💔 Test failed - auto-win-gnu-32-opt-rustbuild |
|
@bors retry Spurious? cc @alexcrichton |
…r=nikomatsakis trans: Only instantiate #[inline] functions in codegen units referencing them This PR changes how `#[inline]` functions are translated. Before, there was one "master instance" of the function with `external` linkage and a number of on-demand instances with `available_externally` linkage in each codegen unit that referenced the function. This had two downsides: * Public functions marked with `#[inline]` would be present in machine code of libraries unnecessarily (see #36280 for an example) * LLVM would crash on `i686-pc-windows-msvc` due to what I suspect to be a bug in LLVM's Win32 exception handling code, because it doesn't like `available_externally` there (#36309). This PR changes the behavior, so that there is no master instance and only on-demand instances with `internal` linkage. The downside of this is potential code-bloat if LLVM does not completely inline away the `internal` instances because then there'd be N instances of the function instead of 1. However, this can only become a problem when using more than one codegen unit per crate. cc @rust-lang/compiler
1 similar comment
This PR changes how
#[inline]functions are translated. Before, there was one "master instance" of the function withexternallinkage and a number of on-demand instances withavailable_externallylinkage in each codegen unit that referenced the function. This had two downsides:#[inline]would be present in machine code of libraries unnecessarily (see Don't generate code for unused #[inline] function #36280 for an example)i686-pc-windows-msvcdue to what I suspect to be a bug in LLVM's Win32 exception handling code, because it doesn't likeavailable_externallythere (codegen-units setting in Cargo.toml causes LLVM error with nightly-i686-pc-windows-msvc #36309).This PR changes the behavior, so that there is no master instance and only on-demand instances with
internallinkage. The downside of this is potential code-bloat if LLVM does not completely inline away theinternalinstances because then there'd be N instances of the function instead of 1. However, this can only become a problem when using more than one codegen unit per crate.cc @rust-lang/compiler