-
Notifications
You must be signed in to change notification settings - Fork 13k
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
#![no_builtins] should prevent a crate from participating in LTO #35540
Comments
OK, I looked at the code around LTO and I think I know how to implement this. Will implement it and report back. |
It works! I'll send a PR. |
japaric
pushed a commit
to japaric/rust
that referenced
this issue
Aug 13, 2016
this prevents intrinsics like `memcpy` from being mis-optimized to infinite recursive calls when LTO is used. fixes rust-lang#31544 closes rust-lang#35540
bors
added a commit
that referenced
this issue
Aug 16, 2016
exclude `#![no_builtins]` crates from LTO this prevents intrinsics like `memcpy` from being mis-optimized to infinite recursive calls when LTO is used. fixes #31544 closes #35540 --- r? @alexcrichton cc @Amanieu
This was referenced Jul 15, 2023
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jul 18, 2023
…, r=pnkfelix Add the `no-builtins` attribute to functions when `no_builtins` is applied at the crate level. **When `no_builtins` is applied at the crate level, we should add the `no-builtins` attribute to each function to ensure it takes effect in LTO.** This is also the reason why no_builtins does not take effect in LTO as mentioned in rust-lang#35540. Now, `#![no_builtins]` should be similar to `-fno-builtin` in clang/gcc, see https://clang.godbolt.org/z/z4j6Wsod5. Next, we should make `#![no_builtins]` participate in LTO again. That makes sense, as LTO also takes into consideration function-level instruction optimizations, such as the MachineOutliner. More importantly, when a user writes a large `#![no_builtins]` crate, they would like this crate to participate in LTO as well. We should also add a function-level no_builtins attribute to allow users to have more control over it. This is similar to Clang's `__attribute__((no_builtin))` feature, see https://clang.godbolt.org/z/Wod6KK6eq. Before implementing this feature, maybe we should discuss whether to support more fine-grained control, such as `__attribute__((no_builtin("memcpy")))`. Related discussions: - rust-lang#109821 - rust-lang#35540 Next (a separate pull request?): - [ ] Revert rust-lang#35637 - [ ] Add a function-level `no_builtin` attribute?
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jul 18, 2023
…, r=pnkfelix Add the `no-builtins` attribute to functions when `no_builtins` is applied at the crate level. **When `no_builtins` is applied at the crate level, we should add the `no-builtins` attribute to each function to ensure it takes effect in LTO.** This is also the reason why no_builtins does not take effect in LTO as mentioned in rust-lang#35540. Now, `#![no_builtins]` should be similar to `-fno-builtin` in clang/gcc, see https://clang.godbolt.org/z/z4j6Wsod5. Next, we should make `#![no_builtins]` participate in LTO again. That makes sense, as LTO also takes into consideration function-level instruction optimizations, such as the MachineOutliner. More importantly, when a user writes a large `#![no_builtins]` crate, they would like this crate to participate in LTO as well. We should also add a function-level no_builtins attribute to allow users to have more control over it. This is similar to Clang's `__attribute__((no_builtin))` feature, see https://clang.godbolt.org/z/Wod6KK6eq. Before implementing this feature, maybe we should discuss whether to support more fine-grained control, such as `__attribute__((no_builtin("memcpy")))`. Related discussions: - rust-lang#109821 - rust-lang#35540 Next (a separate pull request?): - [ ] Revert rust-lang#35637 - [ ] Add a function-level `no_builtin` attribute?
GuillaumeGomez
added a commit
to GuillaumeGomez/rust
that referenced
this issue
Jul 18, 2023
…, r=pnkfelix Add the `no-builtins` attribute to functions when `no_builtins` is applied at the crate level. **When `no_builtins` is applied at the crate level, we should add the `no-builtins` attribute to each function to ensure it takes effect in LTO.** This is also the reason why no_builtins does not take effect in LTO as mentioned in rust-lang#35540. Now, `#![no_builtins]` should be similar to `-fno-builtin` in clang/gcc, see https://clang.godbolt.org/z/z4j6Wsod5. Next, we should make `#![no_builtins]` participate in LTO again. That makes sense, as LTO also takes into consideration function-level instruction optimizations, such as the MachineOutliner. More importantly, when a user writes a large `#![no_builtins]` crate, they would like this crate to participate in LTO as well. We should also add a function-level no_builtins attribute to allow users to have more control over it. This is similar to Clang's `__attribute__((no_builtin))` feature, see https://clang.godbolt.org/z/Wod6KK6eq. Before implementing this feature, maybe we should discuss whether to support more fine-grained control, such as `__attribute__((no_builtin("memcpy")))`. Related discussions: - rust-lang#109821 - rust-lang#35540 Next (a separate pull request?): - [ ] Revert rust-lang#35637 - [ ] Add a function-level `no_builtin` attribute?
Dylan-DPC
added a commit
to Dylan-DPC/rust
that referenced
this issue
Jul 19, 2023
…, r=pnkfelix Add the `no-builtins` attribute to functions when `no_builtins` is applied at the crate level. **When `no_builtins` is applied at the crate level, we should add the `no-builtins` attribute to each function to ensure it takes effect in LTO.** This is also the reason why no_builtins does not take effect in LTO as mentioned in rust-lang#35540. Now, `#![no_builtins]` should be similar to `-fno-builtin` in clang/gcc, see https://clang.godbolt.org/z/z4j6Wsod5. Next, we should make `#![no_builtins]` participate in LTO again. That makes sense, as LTO also takes into consideration function-level instruction optimizations, such as the MachineOutliner. More importantly, when a user writes a large `#![no_builtins]` crate, they would like this crate to participate in LTO as well. We should also add a function-level no_builtins attribute to allow users to have more control over it. This is similar to Clang's `__attribute__((no_builtin))` feature, see https://clang.godbolt.org/z/Wod6KK6eq. Before implementing this feature, maybe we should discuss whether to support more fine-grained control, such as `__attribute__((no_builtin("memcpy")))`. Related discussions: - rust-lang#109821 - rust-lang#35540 Next (a separate pull request?): - [ ] Revert rust-lang#35637 - [ ] Add a function-level `no_builtin` attribute?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This attribute is needed to prevent LLVM from optimizing a
memcpy
implementation into a recursive call. However this attribute is not preserved in LTO builds: the attribute only works if it is set at the top-level crate.Since crates containing implementations of built-in functions aren't meant to be called directly, they don't benefit from LTO and should be linked as a separate object.
One workaround (using rlibc as an example) is to first compile a
rlibc_inner
crate as a staticlib with#![no_builtins]
, and then compile therlibc
crate as a rlib which links torlibc_inner.a
.cc #31544 @alexcrichton @japaric
The text was updated successfully, but these errors were encountered: