Skip to content

[libc] Add __FE_DENORM to the fenv macros #91353

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.

Sign up for GitHub

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
May 7, 2024
Merged

[libc] Add __FE_DENORM to the fenv macros #91353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions libc/hdr/fenv_macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@

#include <fenv.h>

// If this is not provided by the system, define it for use internally.
#ifndef __FE_DENORM
#define __FE_DENORM (1 << 6)
#endif

#endif // LLVM_LIBC_FULL_BUILD

#endif // LLVM_LIBC_HDR_FENV_MACROS_H
11 changes: 6 additions & 5 deletions libc/include/llvm-libc-macros/fenv-macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@
#ifndef LLVM_LIBC_MACROS_FENV_MACROS_H
#define LLVM_LIBC_MACROS_FENV_MACROS_H

#define FE_DIVBYZERO 1
#define FE_INEXACT 2
#define FE_INVALID 4
#define FE_OVERFLOW 8
#define FE_UNDERFLOW 16
#define FE_DIVBYZERO 0x1
#define FE_INEXACT 0x2
#define FE_INVALID 0x4
#define FE_OVERFLOW 0x8
#define FE_UNDERFLOW 0x10
#define __FE_DENORM 0x20
Copy link
Contributor

Choose a reason for hiding this comment

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

Shall we all so define it in hdr/fenv_macros.h for overlay mode if the system header does not define it? For completeness and hermeticity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, unsure because if it comes from the system we don't necessarily know what value to assign to it. I think GNU uses 0x2 or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

"I am the system" -libc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean in overlay mode we just rely on the GNU headers in practice, it seems non-trivial to select a value that's not going to conflict, unless 1 << 31 or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

For overlay mode, anything we fill in hdr/fenv_macros.h should only be used internally. So it is kind of the best place to filling in any macros that are missing from the system/compiler headers. And if it's not presented in the system header, it shouldn't matter much which value you assign to it, as long as it is different from the rest of the FE_* right?

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe something like (MAX(FE_DIVBYZERO, FE_INEXACT, FE_INVALID, FE_OVERFLOW, FE_UNDERFLOW) << 1) if it's not too much work :D.

Copy link
Contributor Author

@jhuber6 jhuber6 May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, for now 64 is probably safe enough. Unsure if it's worth the effort since we'd need to make some variadic max function. That'd probably be doable with constexpr functions and C++ magic, but with macros it seems like a big pain unless we want to invoke an actual function call. But this is supposed to be a compile time constant right.

Copy link
Contributor

Choose a reason for hiding this comment

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

For AMDGPU the best values to use would just be to directly take the bit positions from TRAP_STS. i.e. INPUT_DENORMAL is bit 1, FE_INVALID is 0, and so on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's probably occupied by something else, internally we map from the "real" values to the "libc" ones.

Copy link
Contributor

@arsenm arsenm May 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

The standard is implementation defined power of 2 for all of these. We can just define all of them to what we want. We don't need to stick to the current set of arbitrary numbers

#define FE_ALL_EXCEPT \
(FE_DIVBYZERO | FE_INEXACT | FE_INVALID | FE_OVERFLOW | FE_UNDERFLOW)

Expand Down