Skip to content

release/19.x: [compiler-rt] Stop using x86 builtin on AArch64 with GCC (#93890) #115006

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

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Nov 5, 2024

Backport 8aa9d62

Requested by: @XrXr

@llvmbot llvmbot added this to the LLVM 19.X Release milestone Nov 5, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Nov 5, 2024

@statham-arm What do you think about merging this PR to the release branch?

Copy link

github-actions bot commented Nov 5, 2024

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

Copy link
Collaborator

@statham-arm statham-arm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a good idea to me!

@tru
Copy link
Collaborator

tru commented Nov 12, 2024

Looking at the patch it looks sane to me. Is there any risk that this breaks something if we merge it as is now?

@statham-arm
Copy link
Collaborator

Despite the force-push, I think the current patch looks the same as the one I clicked approve on (maybe it was just rebased?). I can't see a problem – it's the same patch that hasn't been causing trouble on main.

@tru
Copy link
Collaborator

tru commented Nov 12, 2024

The force push was just a rebase to get it on the latest code.

@tru tru force-pushed the issue114720 branch 2 times, most recently from 04e2bbe to 19026a4 Compare November 15, 2024 08:21
@tru tru merged commit 19026a4 into llvm:release/19.x Nov 15, 2024
Previously, building `multc3.c` on A64 with GCC 7 or up but 9 and lower
will attempt to reference `__builtin_copysignq`, an [x86-specific
intrinsic][1]:

```
$ gcc -c multc3.c
In file included from fp_lib.h:24,
                 from multc3.c:14:
multc3.c: In function '__multc3':
int_math.h:71:32: warning: implicit declaration of function '__builtin_copysignq'; did you mean '__builtin_copysign'? [-Wimplicit-function-declaration]
 #define crt_copysignf128(x, y) __builtin_copysignq((x), (y))
                                ^~~~~~~~~~~~~~~~~~~
```

This is because `__has_builtin` is from GCC 10, and defined to 0 at the
top of int_math.h for affected GCC versions, so the fallback definition
is used. But `__builtin_copysignq` is unavailable on A64.

Use version detection to find `__builtin_copysignf128` instead. It's
available since GCC 7 and [available][2] on both x86 and A64, given this
macro is only used when `CRT_HAS_IEEE_TF`.

---

I realize this is fixing a problem for an out-of-tree build
configuration, but help would be greatly appreciated. Rust
[builds](https://github.com/rust-lang/compiler-builtins) `multc3.c` with
GCC 8 and this mis-selection is causing [build
issues](rust-lang/rust#125619) way downstream.

ref: d2ce3e9

[1]: https://gcc.gnu.org/onlinedocs/gcc/x86-Built-in-Functions.html
[2]: https://gcc.gnu.org/gcc-7/changes.html

(cherry picked from commit 8aa9d62)
Copy link

@XrXr (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants