Skip to content

Codegen units break #[inline(always)] at -O0 #45201

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

Closed
alexcrichton opened this issue Oct 11, 2017 · 6 comments
Closed

Codegen units break #[inline(always)] at -O0 #45201

alexcrichton opened this issue Oct 11, 2017 · 6 comments
Labels
A-codegen Area: Code generation regression-from-stable-to-beta Performance or correctness regression from stable to beta.

Comments

@alexcrichton
Copy link
Member

On the current nightly using multiple codegen units breaks #[inline(always)] in debug mode (as they could show up in different codegen units, preventing inlining). We should still treat #[inline(always)] as inline everywhere!

@alexcrichton alexcrichton added A-codegen Area: Code generation regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Oct 11, 2017
@alexcrichton
Copy link
Member Author

cc rust-lang/stdarch#108

@alexcrichton
Copy link
Member Author

cc @michaelwoerister

(I can work on this when I get a chance, just wanted to make sure you were aware)

@hanna-kruppe
Copy link
Contributor

Why do we honor inline(always) at -O0 at all? It should be purely an optimization, and I would have hoped we ignore it at -O0 to generate less code faster. While it's useful to have something to override inlining heuristics if optimizations are enabled, I am more than a bit puzzled by any functional change due to it not being honored. Can you explain why stdsimd CI breaks if inline(always) is ignored?

@michaelwoerister
Copy link
Member

Yeah, I was thinking that #[inline(always)] functions should be duplicated to all CGU, regardless of other settings. On the other hand, as far as I know, #[inline(always)] is still just a hint to LLVM and does not guarantee inlining either.

@alexcrichton
Copy link
Member Author

@rkruppe sure yeah I can go into some more detail!

So the problem with SIMD is that we actually need these functions to always get inlined to deal with ABI issues. Otherwise we have a function like:

#[target_feature = "+avx"]
unsafe fn foo() {
    let a = i16x16::new(...);
}

So in this case foo is using 256 bit AVX types like i16x16 which means that if i16x16 were actually a function call the return value would be in an AVX register. The i16x16 function, however, is portable and therefore doesn't have a #[target_feature] annotation. Instead we rely on #[inline(always)] to ensure that i16x16::new gets the correct ABI by just using the surrounding function's ABI instead of any one particular ABI.

In essence we rely on #[inline(always)] to erase ABI differences. I've seen this in some other situations where you rely on particular diassembly and whatnot where #[inline(always)] is critical in all modes to work correctly. Other than SIMD though a specific situation isn't coming to mind.


As for how we treat this at the LLVM layer, it's actually quite intentional that we always respect #[inline(always)] (otherwise it's not always!). Even with no optimizations we add an always inline pass which mirrors the behavior of clang.

We have said many times before that #[inline] is harmful to compile time and #[inline(always)] is even more harmful. In that sense I wouldn't personally consider it too surprising that #[inline(always)] even has ramifications in debug mode. In general you never need #[inline(always)] unless you're in a SIMD-like situation where the ABI/codegen requires it.


@michaelwoerister AFAIK #[inline(always)] truly does always inline it at the LLVM layer whenever it's "reasonable", and the only "unreasonable" situation I've ever encountered is dynamic dispatch where it's impossible to do inlining.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Oct 12, 2017
This commit updates the handling of `#[inline(always)]` functions at -O0 to
ensure that it's always inlined regardless of the number of codegen units used.

Closes rust-lang#45201
@hanna-kruppe
Copy link
Contributor

@alexcrichton Thanks for elaborating and, ouch, what a nasty problem!

bors added a commit that referenced this issue Oct 16, 2017
…ster

rustc: Handle #[inline(always)] at -O0

This commit updates the handling of `#[inline(always)]` functions at -O0 to
ensure that it's always inlined regardless of the number of codegen units used.

Closes #45201
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation regression-from-stable-to-beta Performance or correctness regression from stable to beta.
Projects
None yet
Development

No branches or pull requests

3 participants