Apply `weak` attributes to all intrinsics #598

Amjad50 · 2024-04-29T14:07:59Z

Removed the weak-intrinsics feature, so that all functions will have the weak linkage attribute.

But this caused issues for memset (maybe other mem functions are related), so added a special feature for those mem-weak-intrinsics.

mem functions won't have the weak attribute unless this feature is enabled.

This also fixed the bug in rust-lang/rust#124042.

Before this commit, generated code will be

pub extern "C" fn <name>(...) -> ... {
        // code...
}
pub mod <name> {
    #[linkage = "weak"]
    #[no_mangle]
    pub extern "C" fn <name>(...) -> ... {
        super::<name>(...)
    }
}

The issue is that there is 2 weak linkage, the first one is not required. Along refactoring weak attributes, this was fixed.

src/macros.rs

Amanieu

This may also be a good opportunity to cleanup the mod wrappers around the no_mangle version of each intrinsic. Specifically:

There is no need for these modules to be pub. Same with the function inside them. #[no_mangle] will ensure the symbol is exported.
The whole module should be #[cfg(not(feature = "mangled-names")]. The functions themselves can then be #[no_mangle] unconditionally.

src/macros.rs

Removed the `weak-intrinsics` feature, so that all functions will have the `weak` linkage attribute. Also this fixed the bug in rust-lang/rust#124042. Before this commit, generated code will be ```rust pub extern "C" fn <name>(...) -> ... { // code... } pub mod <name> { #[linkage = "weak"] #[no_mangle] pub extern "C" fn <name>(...) -> ... { super::<name>(...) } } ``` The issue is that there is 2 `weak` linkage, the first one is not required. Along refactoring `weak` attributes, this was fixed.

`weak` is only used with `no_mangle`

Don't generate the whole function if we are not going to use `no_mangle`, there is no point

Amjad50 · 2024-05-03T12:11:18Z

Thanks @Amanieu , can a new version be released?

Amanieu · 2024-05-04T19:20:33Z

Done

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These cause a compilation error (reproduce with `ENABLE_GCC_CODEGEN=1 src/ci/docker/run.sh x86_64-gnu-llvm-17`), so enable `no-f16-f128` for now.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

- rust-lang/compiler-builtins#598 - rust-lang/compiler-builtins#601

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

Update compiler_builtins to 0.1.112 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

…iler-errors Update compiler_builtins to 0.1.112 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc. Also disable it for LLVM targets that don't support it.

Update compiler_builtins to 0.1.112 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc. Also disable it for LLVM targets that don't support it.

Update compiler_builtins to 0.1.112 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

…anieu Update compiler_builtins to 0.1.113 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc. Also disable it for LLVM targets that don't support it.

After <rust-lang#598>, arm-android was failing to complete in CI because it was hanging on some tests. This issue appears to have been caused by symbols related to atomics, e.g. `__sync_val_compare_and_swap_4`, to have become weak. It turns out that these symbols were always strong before, even though Android was always setting the `weak-intrinsics` feature. So, making them weak presumably caused the system implementation to get linked, which appears buggy. Resolve this by making `__sync_*` symbols weak on Android. (this includes a recursion limit increase, our macros are getting big). Link: rust-lang#641

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc. Also disable it for LLVM targets that don't support it.

…ss35 Update compiler_builtins to 0.1.114 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc. Also disable it for LLVM targets that don't support it.

Update compiler_builtins to 0.1.114 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc. Also disable it for LLVM targets that don't support it.

Update compiler_builtins to 0.1.114 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc. Also disable it for LLVM targets that don't support it.

Update compiler_builtins to 0.1.114 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc. Also disable it for LLVM targets that don't support it.

Update compiler_builtins to 0.1.114 The `weak-intrinsics` feature was removed from compiler_builtins in rust-lang/compiler-builtins#598, so dropped the `compiler-builtins-weak-intrinsics` feature from alloc/std/sysroot. In rust-lang/compiler-builtins#593, some builtins for f16/f128 were added. These don't work for all compiler backends, so add a `compiler-builtins-no-f16-f128` feature and disable it for cranelift and gcc.

Amjad50 changed the title ~~Apply weak attributes to all* intrinsics~~ Apply weak attributes to most intrinsics Apr 29, 2024

Amjad50 changed the title ~~Apply weak attributes to most intrinsics~~ Apply weak attributes to most intrinsics by default Apr 29, 2024

Amanieu reviewed Apr 29, 2024

View reviewed changes

src/macros.rs Outdated Show resolved Hide resolved

Amjad50 force-pushed the remove_weak-intrinsics branch from 9040acc to 7a0f662 Compare April 30, 2024 03:33

Amjad50 changed the title ~~Apply weak attributes to most intrinsics by default~~ Apply weak attributes to all intrinsics Apr 30, 2024

Amjad50 force-pushed the remove_weak-intrinsics branch from 7a0f662 to 7b56c74 Compare April 30, 2024 03:34

Amjad50 marked this pull request as draft May 1, 2024 04:18

Amjad50 force-pushed the remove_weak-intrinsics branch from 7b56c74 to 93b722d Compare May 1, 2024 04:19

Amjad50 marked this pull request as ready for review May 1, 2024 11:27

Amjad50 requested a review from Amanieu May 1, 2024 11:27

Amanieu reviewed May 1, 2024

View reviewed changes

src/macros.rs Outdated Show resolved Hide resolved

Amjad50 requested a review from Amanieu May 2, 2024 14:00

Amjad50 force-pushed the remove_weak-intrinsics branch from 657629d to e8e6722 Compare May 2, 2024 14:30

Amjad50 added 3 commits May 3, 2024 13:27

Remove unneeded weak for optimized-c function

16c9ca9

`weak` is only used with `no_mangle`

Cleanup manged-names macro

8a7ba9a

Don't generate the whole function if we are not going to use `no_mangle`, there is no point

Amanieu force-pushed the remove_weak-intrinsics branch from e8e6722 to 8a7ba9a Compare May 3, 2024 11:27

Amanieu enabled auto-merge May 3, 2024 11:27

Amanieu merged commit ab63499 into rust-lang:master May 3, 2024
23 checks passed

Patryk27 mentioned this pull request May 8, 2024

chore: Bump compiler_builtins rust-lang/rust#124886

Closed

nicholasbishop mentioned this pull request May 11, 2024

Update compiler_builtins to 0.1.114 rust-lang/rust#125016

Merged

Patryk27 added a commit to Patryk27/rust that referenced this pull request May 12, 2024

chore: Bump compiler_builtins

2535624

- rust-lang/compiler-builtins#598 - rust-lang/compiler-builtins#601

tgross35 mentioned this pull request Jul 11, 2024

Errors building latest version on arm-android #641

Closed

tgross35 mentioned this pull request Jul 18, 2024

Fix missing extern "C" on unsafe intrinsics #642

Merged

aeubanks mentioned this pull request Aug 2, 2024

Various symbols on win/apple are no longer weak after #598 #653

Open

This was referenced Aug 22, 2024

Fix weak linkage on windows and apple platforms #672

Merged

Mark symbols as weak on Windows and Apple as well #660

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply `weak` attributes to all intrinsics #598

Apply `weak` attributes to all intrinsics #598

Amjad50 commented Apr 29, 2024 •

edited

Loading

Amanieu left a comment

Amjad50 commented May 3, 2024

Amanieu commented May 4, 2024

Apply weak attributes to all intrinsics #598

Apply weak attributes to all intrinsics #598

Conversation

Amjad50 commented Apr 29, 2024 • edited Loading

Amanieu left a comment

Choose a reason for hiding this comment

Amjad50 commented May 3, 2024

Amanieu commented May 4, 2024

Apply `weak` attributes to all intrinsics #598

Apply `weak` attributes to all intrinsics #598

Amjad50 commented Apr 29, 2024 •

edited

Loading