Conversation
| static_assert(sizeof(long double) == 16, | ||
| "When the long double format is x86 80-bit extended floating point, CCCL requires the size of long " | ||
| "double to be 16 bytes."); |
There was a problem hiding this comment.
Q: Could this be a source breaking change on any of our supported platforms?
There was a problem hiding this comment.
When implementing the long double format detection, I thought it would be better to make the compilation fail once the user calls an API that works with long double. However, what I missed is that the l variants of cmath functions (for example fabsl) are not templated, thus all of the machinery is instantiated even just when the headers is included.
Currently, the long double format detection results in an invalid format if the size of long double is not 16. But that also means that if your long double has 96-bits and you include <cuda/std/cmath> the compilation would fail. So, this doesn't change with this PR.
Another thing is that we don't allow long double with CUDA compilation yet.
So, the only combination for which it could be a source breaking change is C++ compilation with gcc with -m96bit-long-double flag (16 byte size is default on 64-bit systems) of source file that doesn't include <cuda/std/__cmath/abs.h>, but includes <cuda/std/__floating_point/format.h>.
I think this is very unlikely to happen.
Also, I'd like to point out that we don't currently test any of our long double functionality.
There was a problem hiding this comment.
I think that sounds rare enough. And IIUC, if a user would be broken by this PR, they could define CCCL_DISABLE_LONG_DOUBLE to make their code compile again. Unless they would rely on long double functionality, which is broken for their platform anyway. I think we are fine. Thx for the analysis!
There was a problem hiding this comment.
I love that we have 80 bit long double with 128 bit size requirement ^^
This comment has been minimized.
This comment has been minimized.
| static_assert(sizeof(long double) == 16, | ||
| "When the long double format is x86 80-bit extended floating point, CCCL requires the size of long " | ||
| "double to be 16 bytes."); |
There was a problem hiding this comment.
I love that we have 80 bit long double with 128 bit size requirement ^^
Co-authored-by: Michael Schellenberger Costa <miscco@nvidia.com>
This comment has been minimized.
This comment has been minimized.
|
I've renamed the macro to |
🥳 CI Workflow Results🟩 Finished in 2h 05m: Pass: 100%/84 | Total: 1d 11h | Max: 1h 37m | Hits: 99%/199232See results here. |
| { | ||
| return __fp_format::__invalid; | ||
| } | ||
| # if LDBL_MIN_EXP == -1021 && LDBL_MAX_EXP == 1024 && LDBL_MANT_DIG == 53 |
There was a problem hiding this comment.
would not be better to use if constexpr for values and check for #if defined(LDBL_MIN_EXP) outside?
There was a problem hiding this comment.
I had it like that before, but there is no simple way to make the static_assert(sizeof(long double) == 16) work only for the fp80 branch, so I ended up using the preprocessor directives
* Enforce supported `long double` format * Update dialect.h Co-authored-by: Michael Schellenberger Costa <miscco@nvidia.com> * Be consistent with other macros --------- Co-authored-by: Michael Schellenberger Costa <miscco@nvidia.com>
4 participants
After thinking a bit more about #7332, I think we should make unknown/unsupported
long doubleformat a hard error. TheCCCL_DISABLE_LONG_DOUBLE_SUPPORTmacro can be used to disablelong doublesupport in CCCL.The advantage is that now the user gets a message about what went wrong, not just getting some errors deep inside the libcu++'s internals.