Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Functions are missing from "--emit=llvm-ir" and "--emit=asm" #119850

Open
RalfJung opened this issue Jan 11, 2024 · 4 comments
Open

Functions are missing from "--emit=llvm-ir" and "--emit=asm" #119850

RalfJung opened this issue Jan 11, 2024 · 4 comments
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

Ever since #116505 landed, functions can be missing from "--emit=llvm-ir" and "--emit=asm" unless they are declared with #[inline(never)] or #[no_mangle]. This is quite surprising to most people that want to explore the LLVM IR / asm of a Rust function.

It's unclear how to best fix this, since not producing LLVM IR or asm (but only MIR) when compiling these functions was part of the goal of #116505. Maybe these --emit flags should themselves cause the functions to be monomorphized so they become visible again?

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 11, 2024
@saethlin saethlin added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 11, 2024
@compiler-errors
Copy link
Member

Maybe these --emit flags should themselves cause the functions to be monomorphized so they become visible again?

I think this is reasonable and probably the best way to fix it.

@briansmith
Copy link
Contributor

There are some use cases where it would be better for a crate to be able to switch back to opt-in inlining. For example, if an application is compiling crates to dylibs they may be doing so so that they can replace a single dynamic library with a bug fix, without having to ship an update to the entire app. In this case they'd like very few functions inlined. It is error-prone to add #[inline(never)] to each public item and also impossible with #[derive].

With this in mind, instead of special casing these --emit flags, I think it would be better to have a separate mechanism to switch to "opt in to inlining" behavior, perhaps which the --emit flags automatically trigger.

Note that I realize that the compiler never made any guarantees about inlining, so what I'm asking for is actually an improvement over even the previous state of things, before automatic inlining was implemented. I do think automatic inlining by default was a good change.

@RalfJung
Copy link
Member Author

To replace a function you'll have to annotate it with #[no_mangle] anyway I think; functions without any such attribute are assumed to be fully under rustc control. #[no_mangle] already disables automatic inlining.

@briansmith
Copy link
Contributor

To replace a function you'll have to annotate it with #[no_mangle] anyway I think; functions without any such attribute are assumed to be fully under rustc control. #[no_mangle] already disables automatic inlining.

Ideally we'd be able to recompile a dylib crate with the same toolchain and have the resulting shared library be a drop-in replacement (ABI-compatible) for the previous one, as long as we don't change the public API of the crate in any way, at least if we use -C symbol-mangling-version=v0.

narpfel added a commit to narpfel/compiler-explorer that referenced this issue Jan 16, 2024
narpfel added a commit to narpfel/compiler-explorer that referenced this issue Jan 16, 2024
jeremy-rifkin pushed a commit to compiler-explorer/compiler-explorer that referenced this issue Jan 17, 2024
…output (#6013)

Resolves #5939.

Rust issue about this problem:
rust-lang/rust#119850

Hopefully this can be reverted again when there is a fix on the Rust
side.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-discussion Category: Discussion or questions that doesn't represent real issues. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants