fold-const: Treat fp conversion to a type with same mode as copy

  Hi,

In function fold_convert_const_real_from_real, when the modes of
two types involved in fp conversion are the same, we can simply
take it as copy, rebuild with the exactly same TREE_REAL_CST and
the target type.  It is more efficient and helps to avoid possible
unexpected signalling bit clearing in [1].

Bootstrapped and regtested on x86_64-redhat-linux, aarch64-linux-gnu
and powerpc64{,le}-linux-gnu.

Is it ok for trunk?

[1] https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608533.html

gcc/ChangeLog:

	* fold-const.cc (fold_convert_const_real_from_real): Treat floating
	point conversion to a type with same mode as copy instead of normal
	convertFormat.
---
 gcc/fold-const.cc | 9 +++++++++
 1 file changed, 9 insertions(+)

--
2.27.0
  

On Mon, Dec 19, 2022 at 9:12 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>
> Hi,
>
> In function fold_convert_const_real_from_real, when the modes of
> two types involved in fp conversion are the same, we can simply
> take it as copy, rebuild with the exactly same TREE_REAL_CST and
> the target type.  It is more efficient and helps to avoid possible
> unexpected signalling bit clearing in [1].
>
> Bootstrapped and regtested on x86_64-redhat-linux, aarch64-linux-gnu
> and powerpc64{,le}-linux-gnu.
>
> Is it ok for trunk?

But shouldn't

double x = (double) __builtin_nans("sNAN");

result in a quiet NaN?

> [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608533.html
>
> gcc/ChangeLog:
>
>         * fold-const.cc (fold_convert_const_real_from_real): Treat floating
>         point conversion to a type with same mode as copy instead of normal
>         convertFormat.
> ---
>  gcc/fold-const.cc | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
> index 114258fa182..eb4b6ca8820 100644
> --- a/gcc/fold-const.cc
> +++ b/gcc/fold-const.cc
> @@ -2178,6 +2178,15 @@ fold_convert_const_real_from_real (tree type, const_tree arg1)
>    REAL_VALUE_TYPE value;
>    tree t;
>
> +  /* If the underlying modes are the same, simply treat it as
> +     copy and rebuild with TREE_REAL_CST information and the
> +     given type.  */
> +  if (TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (arg1)))
> +    {
> +      t = build_real (type, TREE_REAL_CST (arg1));
> +      return t;
> +    }
> +
>    /* Don't perform the operation if flag_signaling_nans is on
>       and the operand is a signaling NaN.  */
>    if (HONOR_SNANS (arg1)
> --
> 2.27.0
  

Hi Richi,

Thanks for the comments!

on 2022/12/19 16:49, Richard Biener wrote:
> On Mon, Dec 19, 2022 at 9:12 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
>>
>> Hi,
>>
>> In function fold_convert_const_real_from_real, when the modes of
>> two types involved in fp conversion are the same, we can simply
>> take it as copy, rebuild with the exactly same TREE_REAL_CST and
>> the target type.  It is more efficient and helps to avoid possible
>> unexpected signalling bit clearing in [1].
>>
>> Bootstrapped and regtested on x86_64-redhat-linux, aarch64-linux-gnu
>> and powerpc64{,le}-linux-gnu.
>>
>> Is it ok for trunk?
> 
> But shouldn't
> 
> double x = (double) __builtin_nans("sNAN");
> 
> result in a quiet NaN?

IMHO, this treatment is still an tiny enhancement even without the
consideration on signalling nan.

I had the same doubt on if the bif should return qNaN without the
explicit option -fsignaling-nans, by checking the manual it says

"Compile code assuming that IEEE signaling NaNs may generate user-visible
traps during floating-point operations."

for the default -fno-signaling-nans, it doesn't exclude sNaN existence
clearly, and the generation of sNaN seems not to be like a normal floating
point operation which can cause trap.  So I was inclined to believe that
the current behavior is expected.  Besides the glibc uses this built-in
for SNAN macro, it seems not a good idea to return a qNaN for it at some
cases.

BR,
Kewen

> 
>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608533.html
>>
>> gcc/ChangeLog:
>>
>>         * fold-const.cc (fold_convert_const_real_from_real): Treat floating
>>         point conversion to a type with same mode as copy instead of normal
>>         convertFormat.
>> ---
>>  gcc/fold-const.cc | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc
>> index 114258fa182..eb4b6ca8820 100644
>> --- a/gcc/fold-const.cc
>> +++ b/gcc/fold-const.cc
>> @@ -2178,6 +2178,15 @@ fold_convert_const_real_from_real (tree type, const_tree arg1)
>>    REAL_VALUE_TYPE value;
>>    tree t;
>>
>> +  /* If the underlying modes are the same, simply treat it as
>> +     copy and rebuild with TREE_REAL_CST information and the
>> +     given type.  */
>> +  if (TYPE_MODE (type) == TYPE_MODE (TREE_TYPE (arg1)))
>> +    {
>> +      t = build_real (type, TREE_REAL_CST (arg1));
>> +      return t;
>> +    }
>> +
>>    /* Don't perform the operation if flag_signaling_nans is on
>>       and the operand is a signaling NaN.  */
>>    if (HONOR_SNANS (arg1)
>> --
>> 2.27.0
  

On Mon, Dec 19, 2022 at 09:49:36AM +0100, Richard Biener wrote:
> On Mon, Dec 19, 2022 at 9:12 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
> > In function fold_convert_const_real_from_real, when the modes of
> > two types involved in fp conversion are the same, we can simply
> > take it as copy, rebuild with the exactly same TREE_REAL_CST and
> > the target type.  It is more efficient and helps to avoid possible
> > unexpected signalling bit clearing in [1].
> >
> > Bootstrapped and regtested on x86_64-redhat-linux, aarch64-linux-gnu
> > and powerpc64{,le}-linux-gnu.
> >
> > Is it ok for trunk?
> 
> But shouldn't
> 
> double x = (double) __builtin_nans("sNAN");
> 
> result in a quiet NaN?

I think it doesn't already, fold_convert_const starts with
  tree arg_type = TREE_TYPE (arg1);
  if (arg_type == type)
    return arg1;
and so if it is a conversion to the same type, fold_convert_const_real_from_real
won't be called at all.
It is just cast to a different type with same mode (say typedef double doublet)
that triggers it right now.
Try:

double
foo (void)
{
  return __builtin_nans ("");
}

double
bar (void)
{
  return (double) __builtin_nans ("");
}

typedef double doublet;

doublet
baz (void)
{
  return __builtin_nans ("");
}

doublet
qux (void)
{
  return (doublet) __builtin_nans ("");
}

float
corge (void)
{
  return (float) __builtin_nans ("");
}

GCC right now returns a sNaN in foo and bar and qNaN in baz, qux and corge,
clang and ICC (tried 19.0.1 on godbolt) return sNaN in foo, bar, baz, qux
and qNaN in corge.

The last case is required by C:
convertFormat - different formats	cast and implicit conversions	6.3.1.5, 6.5.4
convertFormat - same format		canonicalize			7.12.11.7, F.10.8.7
As for the rest, C n3047.pdf has:
Whether C assignment (6.5.16) (and conversion as if by assignment) to the same format is an
IEC 60559 convertFormat or copy operation439) is implementation-defined, even if <fenv.h> defines
the macro FE_SNANS_ALWAYS_SIGNAL (F.2.1). If the return expression of a return statement is
evaluated to the floating-point format of the return type, it is implementation-defined whether a
convertFormat operation is applied to the result of the return expression.

439) Where the source and destination formats are the same, convertFormat operations differ from copy operations in
that convertFormat operations raise the "invalid" floating-point exception on signaling NaN inputs and do not propagate
non-canonical encodings.

I think the posted patch is good for consistency, treating conversion to the
same format sometimes as convertFormat and sometimes as copy is maybe valid
but confusing, especially when on:

double
foo (double x)
{
  return x;
}

double
bar (double x)
{
  return (double) x;
}

typedef double doublet;

doublet
baz (double x)
{
  return x;
}

doublet
qux (double x)
{
  return (doublet) x;
}

float
corge (double x)
{
  return (float) x;
}
we actually use copy operations in all of foo, bar, baz and qux.

	Jakub
  

Hi!

On Mon, Dec 19, 2022 at 11:02:30AM +0100, Jakub Jelinek wrote:
> On Mon, Dec 19, 2022 at 09:49:36AM +0100, Richard Biener wrote:
> > On Mon, Dec 19, 2022 at 9:12 AM Kewen.Lin <linkw@linux.ibm.com> wrote:
> > > In function fold_convert_const_real_from_real, when the modes of
> > > two types involved in fp conversion are the same, we can simply
> > > take it as copy, rebuild with the exactly same TREE_REAL_CST and
> > > the target type.  It is more efficient and helps to avoid possible
> > > unexpected signalling bit clearing in [1].
> > >
> > > Bootstrapped and regtested on x86_64-redhat-linux, aarch64-linux-gnu
> > > and powerpc64{,le}-linux-gnu.
> > >
> > > Is it ok for trunk?
> > 
> > But shouldn't
> > 
> > double x = (double) __builtin_nans("sNAN");
> > 
> > result in a quiet NaN?

No, it is no conversion, it is a no-op?

With -fno-signaling-nans, anything goes.

With -fsignaling-nans, only corge should return a qNaN *and trap*, and
it does that for me, too (it does an frsp insn on the datum, and that
has that effect).

With signalling NaNs your program should always trap when it converts a
SNaN to a QNaN.  Not all operations on SNaNs can be optimised away; any
SNaN that is operated on should remain trapping.  With -fsignaling-nans
very many floating point operations can not be optimised very much.

> GCC right now returns a sNaN in foo and bar and qNaN in baz, qux and corge,
> clang and ICC (tried 19.0.1 on godbolt) return sNaN in foo, bar, baz, qux
> and qNaN in corge.

I just get an lfd everywhere (so returns SNaN, doesn't trap) except in
corge(), which also does an frsp (round to SP float, so it returns a
QNaN, but traps first).

> As for the rest, C n3047.pdf has:
> Whether C assignment (6.5.16) (and conversion as if by assignment) to the same format is an
> IEC 60559 convertFormat or copy operation439) is implementation-defined, even if <fenv.h> defines
> the macro FE_SNANS_ALWAYS_SIGNAL (F.2.1).

And GCC just does a copy, already?  At -O2 anyway.

> I think the posted patch is good for consistency, treating conversion to the
> same format sometimes as convertFormat and sometimes as copy is maybe valid
> but confusing, especially when on:

Agreed.


Segher
  

On Mon, Dec 19, 2022 at 04:11:59PM +0800, Kewen.Lin wrote:
> In function fold_convert_const_real_from_real, when the modes of
> two types involved in fp conversion are the same, we can simply
> take it as copy, rebuild with the exactly same TREE_REAL_CST and
> the target type.  It is more efficient and helps to avoid possible
> unexpected signalling bit clearing in [1].
> 
> Bootstrapped and regtested on x86_64-redhat-linux, aarch64-linux-gnu
> and powerpc64{,le}-linux-gnu.
> 
> Is it ok for trunk?
> 
> [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608533.html
> 
> gcc/ChangeLog:
> 
> 	* fold-const.cc (fold_convert_const_real_from_real): Treat floating
> 	point conversion to a type with same mode as copy instead of normal
> 	convertFormat.

The patch is ok for trunk.  Thanks.

	Jakub
  

on 2022/12/20 20:14, Jakub Jelinek wrote:
> On Mon, Dec 19, 2022 at 04:11:59PM +0800, Kewen.Lin wrote:
>> In function fold_convert_const_real_from_real, when the modes of
>> two types involved in fp conversion are the same, we can simply
>> take it as copy, rebuild with the exactly same TREE_REAL_CST and
>> the target type.  It is more efficient and helps to avoid possible
>> unexpected signalling bit clearing in [1].
>>
>> Bootstrapped and regtested on x86_64-redhat-linux, aarch64-linux-gnu
>> and powerpc64{,le}-linux-gnu.
>>
>> Is it ok for trunk?
>>
>> [1] https://gcc.gnu.org/pipermail/gcc-patches/2022-December/608533.html
>>
>> gcc/ChangeLog:
>>
>> 	* fold-const.cc (fold_convert_const_real_from_real): Treat floating
>> 	point conversion to a type with same mode as copy instead of normal
>> 	convertFormat.
> 
> The patch is ok for trunk.  Thanks.
> 

Thanks!  Committed in r13-4815-g94cf7a2d95bf6d.

BR,
Kewen