Repository navigation
transpile: Implement __builtin_nans* and honor NaN payloads - #2049
shashikanth-a wants to merge 3 commits into
Conversation
|
|
||
| /// The IEEE format of the result of a `__builtin_nan*` builtin. | ||
| #[derive(Copy, Clone)] | ||
| enum NanFloat { |
There was a problem hiding this comment.
This seems superfluous to the FloatingKind enum?
There was a problem hiding this comment.
Agreed. Removed NanFloat and moved the helpers onto CFloatingKind in 932c3bb.
| // Both are translated to `f128::f128` (binary128). | ||
| LongDouble | Float128 => Some((112, 15)), | ||
|
|
||
| Half | BFloat16 => None, |
There was a problem hiding this comment.
Not really that important, but since the values needed for 16-bit floats are known too, they might as well be added? Then it doesn't have to return Option and neither does nan_bits.
There was a problem hiding this comment.
Done in 8226c99. layout() and nan_bits() no longer return Option, and 16-bit floats are rejected once at the start of convert_builtin_nan. The new test values match clang (Half) and LLVM's APFloat (BFloat16), since clang has no bf16 NaN builtins
|
Not a maintainer myself, but the maintainers of c2rust seem to generally prefer rebasing and editing existing commits when changes are requested in a review, rather than adding new commits. |
8226c99 to
a54d797
Compare
|
Thanks! I squashed the fix into the CFloatingKind commit and force-pushed. |
`__builtin_nans`, `__builtin_nansf` and `__builtin_nansl` were unimplemented, and `__builtin_nan*` ignored their payload argument. Translate both to the bits clang's constant folding produces: the payload string is parsed like LLVM's `StringRef::getAsInteger` with an auto-detected radix, and the NaN is built like `APFloat::makeNaN`. `long double` uses binary128, matching its translation to `f128::f128`. Where clang doesn't fold (a non-literal or malformed payload), it emits a libm call instead. For the quiet variants, emit the default NaN with a warning, keeping the argument's side effects. libm has no `nans`, so an unfoldable signaling NaN is an error. Edition 2024 uses `f32::from_bits`/`f64::from_bits`. Older editions use `transmute`, as `from_bits` isn't `const` on their pinned toolchain and these can appear in `static` initializers. Fixes immunant#1800.
`__builtin_nanf128`, `__builtin_nansf128`, `__builtin_inff128` and `__builtin_huge_valf128` were unimplemented. `__float128` is translated to `f128::f128` like `long double`, so handle them like the `l` variants.
a54d797 to
8f1aff4
Compare
|
Rebased onto latest master to resolve conflicts with the convert_builtin changes. |
Fixes #1800.
__builtin_nans{,f,l}were unimplemented,__builtin_nan*ignored their payload argument, and the__float128variants (__builtin_{nan,nans,inf,huge_val}f128) were unimplemented too.All NaN builtins now translate to the bits clang's constant folding produces:
StringRef::getAsIntegerwith an auto-detected radix ("", decimal, leading-0octal,0x,0b,0o), and the NaN is built likeAPFloat::makeNaN. A signaling NaN clears the quiet bit, and sets the next bit down if the payload is then empty.nans, so an unfoldable signaling NaN is an error.f64::from_bits(..). Older editions emittransmute, sincefrom_bitsisn'tconston their pinned toolchain (1.70) and these can appear instaticinitializers. The default quiet NaN still becomesf32::NAN/f64::NAN.long doubleand__float128usetransmute::<u128, f128::f128>, as thef128crate has no public bit-pattern constructor.One behaviour change:
__builtin_nanl("")used to bef128::f128::NAN, whose payload (all ones) differs from clang's. It now gets clang's bits.The second commit adds the
__float128builtins separately, in case you'd rather review or take it on its own.Testing
nan.candf128_builtins.c(translated forx86_64-unknown-linux-gnu), plus unit tests for payload parsing and NaN bits.nan,nans,nanfandnansf, I compiled and ran both the C and the translated Rust (editions 2021 and 2024) and compared the bits. Thef128variants were compared against clang's x86_64 constants, including payloads wider than 128 bits.f128output on x86_64 Linux against the realf128crate, and compared it with GCC.Limitations
long doubleis translated as binary128, so on x86 the bits differ from x87's 80-bit format. This is the existing mapping._Float16isn't supported by the transpiler at all, so__builtin_nanf16isn't handled here.