[builtins] Avoid using long double in FreeBSD standalone environment

[DimitryAndric]

After 05a4212 a number of long double related declarations are enabled in int_types.h, whenever the CPU architecture and platform support it. However, this does not work with FreeBSD's standalone environment, which disallows any use of floating point.

In add98b2 this was made conditional with CRT_HAS_FLOATING_POINT, so use that macro to also determine whether HAS_80_BIT_LONG_DOUBLE can be turned on.

[arichardson]

I think it might be cleaner to just hoist the #if CRT_HAS_FLOATING_POINT from line 225 above this check and omit all floating point typedefs etc.

[compnerd]

Yeah, I agree, move the HAS_80_BIT_LONG_DOUBLE into the else clause around L129.

[DimitryAndric]

I'm sorry, but you both seem to suggest different things. :)

So @arichardson seems to suggest extending the #if CRT_HAS_FLOATING_POINT block from line 132 (just after the #if defined(__FreeBSD__) && defined(_STANDALONE) line) to the next-to-last #endif (just before #endif // INT_TYPES_H). I.e., remove the #endif on line 142 (just after the float_bits and double_bits unions), and also remove #if CRT_HAS_FLOATING_POINT on line 223 (just before the #if __STDC_VERSION__ >= 199901L line. Effectively, this makes everything from line 132 onward disappear when floating point isn't supported.

@compnerd seems to to suggest only moving only the block that defines HAS_80BIT_LONG_DOUBLE (lines 154 through 166) to just after line 129. I.e. in the #else case of the #if defined(__FreeBSD__) && defined(_STANDALONE) block.

Have I got that right? I think @arichardson's suggestion is somewhat more clear, and I would also like to add comments in the #endif directives to indicated what block they belong to. I.e. #endif // CRT_HAS_FLOATING_POINT etc.

[compnerd]

There are some interleaving definitions which would be removed if it were extended. If there's no references to them, extending the else clause should be okay.

[DimitryAndric]

As far as I can see, all the types that are defined (in the original int_types.h) from line 124 onward, so after the #endif // CRT_HAS_128BIT block, are specifically meant for floating point support in some form or other. I wouldn't want to break any other uses though, there are a whole bunch of powerpc64 and 128-bit defines in between, and not all of them fall under CRT_HAS_FLOATING_POINT. I will just move only the really required stuff to the top.

[DimitryAndric]

Hmm I need to do a bit more work on this, since HAS_80_BIT_LONG_DOUBLE isn't defined for some architectures, giving me errors when building all of those architectures for FreeBSD:

$ grep -m1 error: /home/dim/log/universe-2023-12-22a/_.*
/home/dim/log/universe-2023-12-22a/_.arm.armv7.buildworld:/home/dim/src/freebsd/llvm-18-update/contrib/llvm-project/compiler-rt/lib/builtins/divxc3.c:20:35: error: unknown type name 'xf_float'
/home/dim/log/universe-2023-12-22a/_.arm64.aarch64.buildworld:/home/dim/src/freebsd/llvm-18-update/contrib/llvm-project/compiler-rt/lib/builtins/divxc3.c:20:35: error: unknown type name 'xf_float'; did you mean 'tf_float'?
/home/dim/log/universe-2023-12-22a/_.powerpc.powerpc64.buildworld:/home/dim/src/freebsd/llvm-18-update/contrib/llvm-project/compiler-rt/lib/builtins/divtc3.c:23:28: error: call to undeclared function 'crt_fabstf'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
/home/dim/log/universe-2023-12-22a/_.powerpc.powerpc64le.buildworld:/home/dim/src/freebsd/llvm-18-update/contrib/llvm-project/compiler-rt/lib/builtins/divtc3.c:23:28: error: call to undeclared function 'crt_fabstf'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
/home/dim/log/universe-2023-12-22a/_.riscv.riscv64.buildworld:/home/dim/src/freebsd/llvm-18-update/contrib/llvm-project/compiler-rt/lib/builtins/divxc3.c:20:35: error: unknown type name 'xf_float'; did you mean 'tf_float'?

@arichardson do you remember the details about this? I think HAS_80_BIT_LONG_DOUBLE might also have to be defined to 1 for armv7, or we have to stop building divxc3.c ? And similar for aarch64 and riscv?

(The powerpc64 failures seem to be something entirely different, these are calls to undeclared crt_fabstf and crt_copysigntf functions. Not sure what is going on there.)

[DimitryAndric]

@arichardson the problem, at least on FreeBSD, is that we've been building xf_float based primitives for platforms that strictly do not support 80 bit long double, such as aarch64, arm, powerpc64, and riscv. (See https://github.com/freebsd/freebsd-src/blob/main/lib/libcompiler_rt/Makefile.inc, specifically the initial list for all arches, which includes divtc3.c and divxc3.c.) I could remove these from the list for any architectures that do not support 80 bit long doubles, but then I would be breaking backwards compat.

I.e. in older versions of compiler-rt, functions like divtc3 were just defined with long double in their parameter lists. I think I need to have an alternative where xf_float is a plain long double...

[DimitryAndric]

Ok, in the end it was much easier to just extend the CRT_HAS_FLOATING_POINT block to cover all floating point declarations. That fixes the FreeBSD libstand case, and should not influence any other cases.

[compnerd]

Sure, this seems reasonable. I still wonder if anyone is going to be impacted by the missing type definitions, but if they are, we can deal with them then.

[arichardson]

I think the uqwords struct should be hoisted before the #ifdef block for the floating point types but otherwise this looks good to me.

[arichardson]

Is this struct not needed outside of the no-fp case?

[DimitryAndric]

No, uqwords is only used in the subsequent declarations of the xf_bits union (for 80 bit long doubles) and the tf_bits union (for 128 bit quads). (This is not the case for the earlier udwords, btw)

[arichardson]

@arichardson the problem, at least on FreeBSD, is that we've been building xf_float based primitives for platforms that strictly do not support 80 bit long double, such as aarch64, arm, powerpc64, and riscv. (See https://github.com/freebsd/freebsd-src/blob/main/lib/libcompiler_rt/Makefile.inc, specifically the initial list for all arches, which includes divtc3.c and divxc3.c.) I could remove these from the list for any architectures that do not support 80 bit long doubles, but then I would be breaking backwards compat.

I.e. in older versions of compiler-rt, functions like divtc3 were just defined with long double in their parameter lists. I think I need to have an alternative where xf_float is a plain long double...

The tf functions are not long double, they are tetra-float, i.e. 128-bit floating point. Using long double was incorrect for x86 which should only be using it for the xf functions. So you should remove the xf files for !x86.

The other option would be to use the same pattern as in the tf files where they compile to nothing if 80-bit floats are not available (this would require updating the ifdef guards and is probably a bit more work than just changing the makefile).

[DimitryAndric]

The tf functions are not long double, they are tetra-float, i.e. 128-bit floating point. Using long double was incorrect for x86 which should only be using it for the xf functions. So you should remove the xf files for !x86.

The other option would be to use the same pattern as in the tf files where they compile to nothing if 80-bit floats are not available (this would require updating the ifdef guards and is probably a bit more work than just changing the makefile).

Yes, I've done this in DimitryAndric/freebsd-src@2d20348 , effectively moving divxc3 and friends to 80-bit x86-only, and divtc3 and friends to the 128-bit quad section.