-
Notifications
You must be signed in to change notification settings - Fork 10.8k
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
Add explicit conversion to fix arm64 builds. #82429
Conversation
@llvm/pr-subscribers-libc Author: Moshe (MosheBerman) ChangesFixes issue preventing builds on ARM-based Macs. Full diff: https://github.com/llvm/llvm-project/pull/82429.diff 1 Files Affected:
diff --git a/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h b/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h
index ea1fd68a5fcdfc..fd915373020ec9 100644
--- a/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h
+++ b/libc/src/__support/FPUtil/aarch64/fenv_darwin_impl.h
@@ -161,8 +161,8 @@ LIBC_INLINE int set_except(int excepts) {
LIBC_INLINE int raise_except(int excepts) {
float zero = 0.0f;
float one = 1.0f;
- float large_value = FPBits<float>::max_normal();
- float small_value = FPBits<float>::min_normal();
+ float large_value = FPBits<float>::max_normal().get_val();
+ float small_value = FPBits<float>::min_normal().get_val();
auto divfunc = [](float a, float b) {
__asm__ __volatile__("ldr s0, %0\n\t"
"ldr s1, %1\n\t"
@@ -277,8 +277,8 @@ LIBC_INLINE int set_env(const fenv_t *envp) {
return 0;
}
const FEnv::FPState *state = reinterpret_cast<const FEnv::FPState *>(envp);
- FEnv::set_control_word(state->ControlWord);
- FEnv::set_status_word(state->StatusWord);
+ FEnv::set_control_word(static_cast<uint32_t>(state->ControlWord));
+ FEnv::set_status_word(static_cast<uint32_t>(state->StatusWord));
return 0;
}
|
|
Turned off the "keep emails private." Do I need to submit this again? |
No, it should be fine. The private email rule is mainly because if the email is private then the author won't get emails if a bot breaks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated, but this and others should probably use LIBC_INLINE_ASM
@jhuber6 I'm not familiar enough to know what should be using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we're requiring LIBC_INLINE_ASM
right now, but it's a macro for marking asm calls, defined here: https://github.com/llvm/llvm-project/blob/main/libc/src/__support/macros/attributes.h#L24
I remember there being some discussion around deprecating it, since marking all of our asm as volatile pessimizes any optimization. In theory it would allow a user to redefine that macro and set their own asm attributes.
Regardless, it's out of scope for this patch. This patch is fine to land as is. If you don't have commit access then just ping me and I'll land it for you.
Yeah I don't have access. Thanks for merging! |
Fixes issue preventing builds on ARM-based Macs.
#82205.