iains / gcc-darwin-arm64

GCC master branch for Darwin with experimental support for Arm64. Currently GCC-15.0.0 [September 2024]
GNU General Public License v2.0
268 stars 33 forks source link

Handle quad names of TFmode builtins in preprocessor #122

Closed fxcoudert closed 10 months ago

fxcoudert commented 10 months ago

Doh. Much, much easier. (The f128 builtins did not exist when I wrote the original code two or three years ago.)

iains commented 10 months ago

just to clarify; what the underlying reason for needing the __builtin_xxxxq () versions is. Is it because of the naming of the library functions from libquadmath? or some other expectation?

there are quite a lot of the f128 builtins that resolve to a libcall.

fxcoudert commented 10 months ago

just to clarify; what the underlying reason for needing the __builtin_xxxxq () versions is. Is it because of the naming of the library functions from libquadmath? or some other expectation?

On targets where __float128 exists and differs from long double:

They existed before the __builtin_*f128 names were added. They are defined on x86_64, IA-64 and HPPA. Both are trivial to implement (because they fold to a constant) and therefore I added them to my original implementation.

In addition, x86_64 also defines:

They're not required by anything inside GCC, but they are available on x86_64-darwin and so, having them on aarch64-darwin gives us some “feature parity” (although they can be trivially replaced by the f128-suffixed variants).

iains commented 10 months ago

OK, that makes sense, so when these are done we're 'done' which is good.

fxcoudert commented 10 months ago

The patch still needs a full regression-testing, which I cannot do right now, but I think it's ready after that.

iains commented 10 months ago

it looks like (at least in my current build) that all the entries in libquadmath have a "q" suffix, so that we need to do this for every library builtin(?)

edit: because __float128 d = __builtin_sinf128 (x); gets emitted as a call to _sinq edit2: and we do not seem to have declarations for either sinq or sinf128 in our math header.

fxcoudert commented 10 months ago

I don't understand your latest comment above. libquadmath is providing an implementation of the quad functions (not prefixed with __builtin_). The compiler can emit calls to those, it is the user's responsibility to link in libquadmath in this case.

iains commented 10 months ago

I don't understand your latest comment above. libquadmath is providing an implementation of the quad functions (not prefixed with __builtin_). The compiler can emit calls to those, it is the user's responsibility to link in libquadmath in this case.

I see that the library provides the functions, but math.h does not provide interfaces (so currently the only way to access sinq, for example is by writing __builtin_sinf128 - which is somewhat non-obvious). Do we need to tell users to include some additional header? (or do we need to make a fixincludes to add xxxq to math.h).

fxcoudert commented 10 months ago

libquadmath provides the quadmath.h header, containing the interfaces. This is as it should be: they are nonstandard so they shouldn't be in <math.h> or another system header.

fxcoudert commented 10 months ago

Regtesting in progress for this PR, will post the results in a couple of hours.

iains commented 10 months ago

actually, probably we should move the cppbuiltins assignments to aarch64/darwin.h: TARGET_OS_CPP_BUILTINS (but I can deal with that during merge/split/cleanup-for-posting.