Re: Patch ping (Re: [PATCH] Implement __builtin_issignaling)

[Joseph Myers <joseph@codesourcery.com>]

On Tue, 23 Aug 2022, Jakub Jelinek via Gcc-patches wrote:

> On Tue, Aug 16, 2022 at 11:41:06AM +0000, Richard Biener wrote:
> > I'm OK with the rest of the patch if Joseph doesn't have comments
> > on the actual issignaling lowerings (which I didn't review for
> > correctness due to lack of knowledge).
> 
> I'd like to ping this patch.
> Patch: https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599697.html
> with
> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599794.html
> https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599800.html
> incremental additions.

One testsuite comment:

> +#ifdef __SIZEOF_FLOAT128__
> +  if (f7 (w) || !f7 (__builtin_nansf128 ("0x123")) || f7 (42.0Q) || f7 (__builtin_nanf128 ("0x234"))
> +      || f7 (__builtin_inff128 ()) || f7 (-__builtin_inff128 ()) || f7 (-42.0Q) || f7 (-0.0Q) || f7 (0.0Q))
> +    __builtin_abort ();

__SIZEOF_FLOAT128__ is a target-specific macro for two targets.  I think 
it's better to model things on the existing _FloatN and _FloatNx tests, so 
have such a test for each such type, preferably with most of the test 
implementation type-generic (see e.g. floatn-builtin.h) and then with the 
individual tests including e.g.

/* { dg-add-options float128 } */
/* { dg-require-effective-target float128_runtime } */

to enable and test for the relevant support.  That would mean _Float128 
gets tested wherever supported - and also that the support is properly 
tested for _Float16, which doesn't look like it's covered by the tests in 
this patch at all at present.

-- 
Joseph S. Myers
joseph@codesourcery.com