[libc] Add __FE_DENORM to the fenv macros #91353
Conversation
|
@llvm/pr-subscribers-libc Author: Joseph Huber (jhuber6) ChangesSummary: This provides it in our headers, however I'm unsure if we should make it Full diff: https://github.com/llvm/llvm-project/pull/91353.diff 1 Files Affected:
diff --git a/libc/include/llvm-libc-macros/fenv-macros.h b/libc/include/llvm-libc-macros/fenv-macros.h
index 72ac660cd98cba..1826723f93490d 100644
--- a/libc/include/llvm-libc-macros/fenv-macros.h
+++ b/libc/include/llvm-libc-macros/fenv-macros.h
@@ -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
#define FE_ALL_EXCEPT \
(FE_DIVBYZERO | FE_INEXACT | FE_INVALID | FE_OVERFLOW | FE_UNDERFLOW)
|
| #define FE_INVALID 0x4 | ||
| #define FE_OVERFLOW 0x8 | ||
| #define FE_UNDERFLOW 0x10 | ||
| #define __FE_DENORM 0x20 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
maybe something like (MAX(FE_DIVBYZERO, FE_INEXACT, FE_INVALID, FE_OVERFLOW, FE_UNDERFLOW) << 1) if it's not too much work :D.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
That's probably occupied by something else, internally we map from the "real" values to the "libc" ones.
There was a problem hiding this comment.
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
Summary: Some targets support denormals as floating point exceptions. This is provided as an extension in the GNU headers as __FE_DENORM. This provides it in our headers, however I'm unsure if we should make it internal or external. I do not think it should be in all exception as it doesn't represent an exceptional behavior as far as the standard is concerned, but I'm not an expert.
Summary:
Some targets support denormals as floating point exceptions. This is
provided as an extension in the GNU headers as __FE_DENORM.
This provides it in our headers, however I'm unsure if we should make it
internal or external. I do not think it should be in all exception as it
doesn't represent an exceptional behavior as far as the standard is
concerned, but I'm not an expert.