match.pd: Fix x * 0.0 -> 0.0 folding [PR104389]

  Hi!

The recent PR95115 change to punt in const_binop on folding operation
with non-NaN operands into NaN if flag_trapping_math broke the following
testcase, because the x * 0.0 simplification punts just if
x maybe a NaN (because NaN * 0.0 is NaN not 0.0) or if one of the operands
could be negative zero.  But Inf * 0.0 or -Inf * 0.0 is also NaN, not
0.0, so when NaNs are honored we need to punt for possible infinities too.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
and 11/10 where the PR95115 change has been unfortunately backported to
as well?

2022-02-04  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/104389
	(x * 0 -> 0): Punt if x maybe infinite and NaNs are honored.

	* gcc.dg/pr104389.c: New test.


	Jakub
  

> Am 05.02.2022 um 00:08 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
> 
> Hi!
> 
> The recent PR95115 change to punt in const_binop on folding operation
> with non-NaN operands into NaN if flag_trapping_math broke the following
> testcase, because the x * 0.0 simplification punts just if
> x maybe a NaN (because NaN * 0.0 is NaN not 0.0) or if one of the operands
> could be negative zero.  But Inf * 0.0 or -Inf * 0.0 is also NaN, not
> 0.0, so when NaNs are honored we need to punt for possible infinities too.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> and 11/10 where the PR95115 change has been unfortunately backported to
> as well?

Ok


> 2022-02-04  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR tree-optimization/104389
>    (x * 0 -> 0): Punt if x maybe infinite and NaNs are honored.
> 
>    * gcc.dg/pr104389.c: New test.
> 
> --- gcc/match.pd.jj    2022-02-04 14:36:55.393599880 +0100
> +++ gcc/match.pd    2022-02-04 20:30:48.548213594 +0100
> @@ -256,10 +256,12 @@ (define_operator_list SYNC_FETCH_AND_AND
> /* Maybe fold x * 0 to 0.  The expressions aren't the same
>    when x is NaN, since x * 0 is also NaN.  Nor are they the
>    same in modes with signed zeros, since multiplying a
> -   negative value by 0 gives -0, not +0.  */
> +   negative value by 0 gives -0, not +0.  Nor when x is +-Inf,
> +   since x * 0 is NaN.  */
> (simplify
>  (mult @0 real_zerop@1)
>  (if (!tree_expr_maybe_nan_p (@0)
> +      && (!HONOR_NANS (type) || !tree_expr_maybe_infinite_p (@0))
>       && !tree_expr_maybe_real_minus_zero_p (@0)
>       && !tree_expr_maybe_real_minus_zero_p (@1))
>   @1))
> --- gcc/testsuite/gcc.dg/pr104389.c.jj    2022-02-04 20:37:40.579537142 +0100
> +++ gcc/testsuite/gcc.dg/pr104389.c    2022-02-04 20:37:20.787809803 +0100
> @@ -0,0 +1,26 @@
> +/* PR tree-optimization/104389 */
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +/* { dg-add-options ieee } */
> +/* { dg-require-effective-target inf } */
> +
> +__attribute__((noipa)) double
> +foo (void)
> +{
> +  double a = __builtin_huge_val ();
> +  return a * 0.0;
> +}
> +
> +__attribute__((noipa)) long double
> +bar (void)
> +{
> +  return __builtin_huge_vall () * 0.0L;
> +}
> +
> +int
> +main ()
> +{
> +  if (!__builtin_isnan (foo ()) || !__builtin_isnanl (bar ()))
> +    __builtin_abort ();
> +  return 0;
> +}
> 
>    Jakub
>
  

On Sat, 2022-02-05 at 08:21 +0100, Richard Biener via Gcc-patches wrote:
> 
> 
> > Am 05.02.2022 um 00:08 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
> > 
> > Hi!
> > 
> > The recent PR95115 change to punt in const_binop on folding operation
> > with non-NaN operands into NaN if flag_trapping_math broke the following
> > testcase, because the x * 0.0 simplification punts just if
> > x maybe a NaN (because NaN * 0.0 is NaN not 0.0) or if one of the operands
> > could be negative zero.  But Inf * 0.0 or -Inf * 0.0 is also NaN, not
> > 0.0, so when NaNs are honored we need to punt for possible infinities too.

Sorry for the trouble, but some warning here: even with this patch
applied, Python would still need to replace inf * 0.0 with nan("") or
something.  Now with folding for inf * 0.0 disabled, the multiplication
will be evaluated at runtime and raise FE_INVALID, which is likely
unwanted by Python.  However, raising FE_INVALID is the correct behavior
no matter if we like or dislike it...

> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> > and 11/10 where the PR95115 change has been unfortunately backported to
> > as well?

As there may be many packages (unfortunately) using the nasty "inf *
0.0" pattern to get NaN, maybe we should just revert the PR95115 change
for 11/10, apply PR104389 patch for trunk, and announce the behavior
change in gcc-12/changes.html?
  

> Am 05.02.2022 um 09:21 schrieb Xi Ruoyao <xry111@mengyan1223.wang>:
> 
> On Sat, 2022-02-05 at 08:21 +0100, Richard Biener via Gcc-patches wrote:
>> 
>> 
>>>> Am 05.02.2022 um 00:08 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
>>> 
>>> Hi!
>>> 
>>> The recent PR95115 change to punt in const_binop on folding operation
>>> with non-NaN operands into NaN if flag_trapping_math broke the following
>>> testcase, because the x * 0.0 simplification punts just if
>>> x maybe a NaN (because NaN * 0.0 is NaN not 0.0) or if one of the operands
>>> could be negative zero.  But Inf * 0.0 or -Inf * 0.0 is also NaN, not
>>> 0.0, so when NaNs are honored we need to punt for possible infinities too.
> 
> Sorry for the trouble, but some warning here: even with this patch
> applied, Python would still need to replace inf * 0.0 with nan("") or
> something.  Now with folding for inf * 0.0 disabled, the multiplication
> will be evaluated at runtime and raise FE_INVALID, which is likely
> unwanted by Python.  However, raising FE_INVALID is the correct behavior
> no matter if we like or dislike it...
> 
>>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
>>> and 11/10 where the PR95115 change has been unfortunately backported to
>>> as well?
> 
> As there may be many packages (unfortunately) using the nasty "inf *
> 0.0" pattern to get NaN, maybe we should just revert the PR95115 change
> for 11/10, apply PR104389 patch for trunk, and announce the behavior
> change in gcc-12/changes.html?

That probably makes sense.  We could also revisit turning on -fno -trapping-math by default .


> -- 
> Xi Ruoyao <xry111@mengyan1223.wang>
> School of Aerospace Science and Technology, Xidian University
  

On Sat, Feb 05, 2022 at 08:21:26AM +0100, Richard Biener wrote:
> > Am 05.02.2022 um 00:08 schrieb Jakub Jelinek via Gcc-patches <gcc-patches@gcc.gnu.org>:
> > The recent PR95115 change to punt in const_binop on folding operation
> > with non-NaN operands into NaN if flag_trapping_math broke the following
> > testcase, because the x * 0.0 simplification punts just if
> > x maybe a NaN (because NaN * 0.0 is NaN not 0.0) or if one of the operands
> > could be negative zero.  But Inf * 0.0 or -Inf * 0.0 is also NaN, not
> > 0.0, so when NaNs are honored we need to punt for possible infinities too.
> > 
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
> > and 11/10 where the PR95115 change has been unfortunately backported to
> > as well?
> 
> Ok

Thanks, committed now.  Turns out gcc 10/11 don't suffer from this bug
(though reversion of the PR95115 change for 10/11 is probably a good idea
anyway), this bug has been introduced in
r12-1393-g5b02ed4b87685c0f7c5da9b46cde3ce56fcfd457
where we used there if (!HONOR_NANS (type) && !HONOR_SIGNED_ZEROS (type))
before.
Looking at that commit, there was also:

 (simplify                                                                                                                                                                            
  (minus @0 @0)                                                                                                                                                                       
- (if (!FLOAT_TYPE_P (type) || !HONOR_NANS (type))                                                                                                                                    
+ (if (!FLOAT_TYPE_P (type) || !tree_expr_maybe_nan_p (@0))                                                                                                                           
   { build_zero_cst (type); }))                                                                                                                                                       

Inf - Inf
or
(-Inf) - (-Inf)
is NaN, not 0.0,
but that got fixed in r12-5529-g6ea5fb3cc7f3cc9b731d72183c66c23543876f5a

	Jakub