_Float16-related failures on x86_64-apple-darwin

_Float16-related failures on x86_64-apple-darwin

[FX]

Hi,

Some recently introduced tests have been failing for several weeks on x86_64-apple-darwin.

FAIL: gcc.target/i386/cond_op_maxmin__Float16-1.c
FAIL: gcc.target/i386/pr102464-copysign-1.c
FAIL: gcc.target/i386/pr102464-fma.c
FAIL: gcc.target/i386/pr102464-maxmin.c
FAIL: gcc.target/i386/pr102464-sqrtph.c
FAIL: gcc.target/i386/pr102464-sqrtsh.c
FAIL: gcc.target/i386/pr102464-vrndscaleph.c

In all cases the symptom is the same: the include of <math.h> errors out with “Unsupported value of __FLT_EVAL_METHOD__”. It appears that the compile option -mavx512fp16 defines __FLT_EVAL_METHOD__ to have value 16, which is not understood by darwin:

> /*  Define float_t and double_t per C standard, ISO/IEC 9899:2011 7.12 2,
>     taking advantage of GCC's __FLT_EVAL_METHOD__ (which a compiler may
>     define anytime and GCC does) that shadows FLT_EVAL_METHOD (which a
>     compiler must define only in float.h).                                    */
> #if __FLT_EVAL_METHOD__ == 0
>     typedef float float_t;
>     typedef double double_t;
> #elif __FLT_EVAL_METHOD__ == 1
>     typedef double float_t;
>     typedef double double_t;
> #elif __FLT_EVAL_METHOD__ == 2 || __FLT_EVAL_METHOD__ == -1
>     typedef long double float_t;
>     typedef long double double_t;
> #else /* __FLT_EVAL_METHOD__ */
> #   error "Unsupported value of __FLT_EVAL_METHOD__."
> #endif /* __FLT_EVAL_METHOD__ */


Is the use of __FLT_EVAL_METHOD__ set to 16 supposed to be portable across all targets? Or is it linux-only, and should marked as such?

Thanks for any help you can give.

FX

Re: _Float16-related failures on x86_64-apple-darwin

[Andrew Pinski]

On Thu, Dec 23, 2021, 14:24 FX via Gcc <gcc@gcc.gnu.org> wrote:

> Hi,
>
> Some recently introduced tests have been failing for several weeks on
> x86_64-apple-darwin.
>
> FAIL: gcc.target/i386/cond_op_maxmin__Float16-1.c
> FAIL: gcc.target/i386/pr102464-copysign-1.c
> FAIL: gcc.target/i386/pr102464-fma.c
> FAIL: gcc.target/i386/pr102464-maxmin.c
> FAIL: gcc.target/i386/pr102464-sqrtph.c
> FAIL: gcc.target/i386/pr102464-sqrtsh.c
> FAIL: gcc.target/i386/pr102464-vrndscaleph.c
>
> In all cases the symptom is the same: the include of <math.h> errors out
> with “Unsupported value of __FLT_EVAL_METHOD__”. It appears that the
> compile option -mavx512fp16 defines __FLT_EVAL_METHOD__ to have value 16,
> which is not understood by darwin:
>
> > /*  Define float_t and double_t per C standard, ISO/IEC 9899:2011 7.12 2,
> >     taking advantage of GCC's __FLT_EVAL_METHOD__ (which a compiler may
> >     define anytime and GCC does) that shadows FLT_EVAL_METHOD (which a
> >     compiler must define only in float.h).
>       */
> > #if __FLT_EVAL_METHOD__ == 0
> >     typedef float float_t;
> >     typedef double double_t;
> > #elif __FLT_EVAL_METHOD__ == 1
> >     typedef double float_t;
> >     typedef double double_t;
> > #elif __FLT_EVAL_METHOD__ == 2 || __FLT_EVAL_METHOD__ == -1
> >     typedef long double float_t;
> >     typedef long double double_t;
> > #else /* __FLT_EVAL_METHOD__ */
> > #   error "Unsupported value of __FLT_EVAL_METHOD__."
> > #endif /* __FLT_EVAL_METHOD__ */
>
>
> Is the use of __FLT_EVAL_METHOD__ set to 16 supposed to be portable across
> all targets? Or is it linux-only, and should marked as such?
>

See  https://gcc.gnu.org/bugzilla//show_bug.cgi?id=100854 .


> Thanks for any help you can give.
>
> FX

Re: _Float16-related failures on x86_64-apple-darwin

[FX]

Hi,

> See  https://gcc.gnu.org/bugzilla//show_bug.cgi?id=100854 .

I found that, indeed, but what I struggle to see is: this behaviour of __FLT_EVAL_METHOD__ has been around for several years now, so why aren’t there more tests failing?

I’m not sure what the fix should be, either. We could use fixinclude to make the darwin headers happy, but we don’t really have a macro to provide the right value. Like a __FLT_EVAL_METHOD_OLDSTYLE__ macro.

What should be the float_t and double_t types for FLT_EVAL_METHOD == 16? float and double, if I understand right?

FX

Re: _Float16-related failures on x86_64-apple-darwin

[FX]

> I’m not sure what the fix should be, either. We could use fixinclude to make the darwin headers happy, but we don’t really have a macro to provide the right value. Like a __FLT_EVAL_METHOD_OLDSTYLE__ macro.
> 
> What should be the float_t and double_t types for FLT_EVAL_METHOD == 16? float and double, if I understand right?

This is one possibility, assuming I am right about the types:

diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
index 46e3b8c993a..bea85ef7367 100644
--- a/fixincludes/inclhack.def
+++ b/fixincludes/inclhack.def
@@ -1767,6 +1767,18 @@ fix = {
     test_text = ""; /* Don't provide this for wrap fixes.  */
 };
 
+/*  The darwin headers don't accept __FLT_EVAL_METHOD__ == 16.
+*/
+fix = {
+    hackname  = darwin_flt_eval_method;
+    mach      = "*-*-darwin*";
+    files     = math.h;
+    select    = "^#if __FLT_EVAL_METHOD__ == 0$";
+    c_fix     = format;
+    c_fix_arg = "#if __FLT_EVAL_METHOD__ == 0 || __FLT_EVAL_METHOD__ == 16";
+    test_text = "#if __FLT_EVAL_METHOD__ == 0";
+};
+
 /*
  *  Fix <c_asm.h> on Digital UNIX V4.0:
  *  It contains a prototype for a DEC C internal asm() function,


Sucks to have to fix headers… and we certainly can’t fix people’s code that may depend on __FLT_EVAL_METHOD__ have well-defined values. So not convinced this is the right approach.

FX

Re: _Float16-related failures on x86_64-apple-darwin

[Hongtao Liu]

gcc define __FLT_EVAL_METHOD__ according to

  builtin_define_with_int_value ("__FLT_EVAL_METHOD__",
c_flt_eval_method (true));

and guess we need to handle things like:

   /* GCC only supports one interchange type right now, _Float16.  If
      we're evaluating _Float16 in 16-bit precision, then flt_eval_method
      will be FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16.  */
+  if (x == FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16
+      && x == y)
+    return FLT_EVAL_METHOD_PROMOTE_TO_FLOAT;
   if (x == FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16)
     return y;

I'm testing the patch but still need approval from related MAINTAINERs.

On Fri, Dec 24, 2021 at 7:15 AM FX via Gcc <gcc@gcc.gnu.org> wrote:
>
> > I’m not sure what the fix should be, either. We could use fixinclude to make the darwin headers happy, but we don’t really have a macro to provide the right value. Like a __FLT_EVAL_METHOD_OLDSTYLE__ macro.
> >
> > What should be the float_t and double_t types for FLT_EVAL_METHOD == 16? float and double, if I understand right?
>
> This is one possibility, assuming I am right about the types:
>
> diff --git a/fixincludes/inclhack.def b/fixincludes/inclhack.def
> index 46e3b8c993a..bea85ef7367 100644
> --- a/fixincludes/inclhack.def
> +++ b/fixincludes/inclhack.def
> @@ -1767,6 +1767,18 @@ fix = {
>      test_text = ""; /* Don't provide this for wrap fixes.  */
>  };
>
> +/*  The darwin headers don't accept __FLT_EVAL_METHOD__ == 16.
> +*/
> +fix = {
> +    hackname  = darwin_flt_eval_method;
> +    mach      = "*-*-darwin*";
> +    files     = math.h;
> +    select    = "^#if __FLT_EVAL_METHOD__ == 0$";
> +    c_fix     = format;
> +    c_fix_arg = "#if __FLT_EVAL_METHOD__ == 0 || __FLT_EVAL_METHOD__ == 16";
> +    test_text = "#if __FLT_EVAL_METHOD__ == 0";
> +};
> +
>  /*
>   *  Fix <c_asm.h> on Digital UNIX V4.0:
>   *  It contains a prototype for a DEC C internal asm() function,
>
>
> Sucks to have to fix headers… and we certainly can’t fix people’s code that may depend on __FLT_EVAL_METHOD__ have well-defined values. So not convinced this is the right approach.
>
> FX



-- 
BR,
Hongtao

Re: _Float16-related failures on x86_64-apple-darwin

[Joseph Myers]

On Fri, 24 Dec 2021, Hongtao Liu via Gcc wrote:

> gcc define __FLT_EVAL_METHOD__ according to
> 
>   builtin_define_with_int_value ("__FLT_EVAL_METHOD__",
> c_flt_eval_method (true));
> 
> and guess we need to handle things like:
> 
>    /* GCC only supports one interchange type right now, _Float16.  If
>       we're evaluating _Float16 in 16-bit precision, then flt_eval_method
>       will be FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16.  */
> +  if (x == FLT_EVAL_METHOD_PROMOTE_TO_FLOAT16
> +      && x == y)
> +    return FLT_EVAL_METHOD_PROMOTE_TO_FLOAT;

That's an incorrect change.  excess_precision_mode_join has 
straightforward, well-defined semantics documented in the comment above 
the function and correcly implemented by it before this change; modifying 
those semantics is not the appropriate way to address this issue.

fixincludes is the right place for a fix for this issue.  There is a 
plausible case for having an architecture-independent 
__FLT_EVAL_METHOD_<something>__ macro that takes only values defined by 
C99 (regardless of -fpermitted-flt-eval-methods), rather than using the 
new C23 values such as 16, but if you did have such a macro you'd still 
need to fixinclude the system headers - it would just affect exactly what 
change fixincludes makes to those headers (if there were such a macro, 
fixincludes could change the headers to use it).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: _Float16-related failures on x86_64-apple-darwin

[FX]

Hi Joseph,

> fixincludes is the right place for a fix for this issue.  There is a 
> plausible case for having an architecture-independent 
> __FLT_EVAL_METHOD_<something>__ macro that takes only values defined by 
> C99 (regardless of -fpermitted-flt-eval-methods), rather than using the 
> new C23 values such as 16, but if you did have such a macro you'd still 
> need to fixinclude the system headers - it would just affect exactly what 
> change fixincludes makes to those headers (if there were such a macro, 
> fixincludes could change the headers to use it).

There is still a difference. If we define a new macro __FLT_EVAL_METHOD_OLDSTYLE__ (however it is named), it means we can make a more robust fixinclude, using that macro. If we fixinclude right now to handle the value of 16, then we might have to update the fixinclude for any new value that comes along in the future.

FX