[PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Jakub Jelinek]

Hi!

The following patch does two things (both related to range extension
around the boundaries).

The first part (in the 2 real_isfinite blocks) is to make the ranges
narrower when the old boundaries are minimum and/or maximum representable
finite number.  In that case frange_nextafter gives -Inf or +Inf,
but then the resulting computed reverse range is very far from the actually
needed range, usually extends up to infinity or could even result in NaNs.
While infinities are really the next representable numbers in the
corresponding mode, REAL_VALUE_TYPE is actually a type with wider range
for exponent and 160 bit precision, so the patch instead uses
nextafter number in a hypothetical floating point format with the same
mantissa precision but wider range of exponents.  This significantly
improves the actual ranges of the reverse operations, while still making
them conservatively correct.

The second part is a fix for miscompilation of the new testcase below.
For -ffinite-math-only, without this patch we extend the minimum and/or
maximum representable finite number to -Inf or +Inf, with the patch to
some number outside of the normal exponent range of the mode, but then
we use set which canonicalizes it and turns the boundaries back to
the minimum and/or maximum representable finite numbers, but because
in say [__DBL_MAX__, __DBL_MAX__] = op1 + [__DBL_MAX__, __DBL_MAX__]
op1 can be larger than 0, up to the largest number which rounds to even
down back to __DBL_MAX__ and there are still no infinities involved,
it needs to work even with -ffinite-math-only.  So, we really need to
widen the lhs range a little bit even in that case.  The patch does
that through temporarily clearing -ffinite-math-only, such that the
value with infinities or the outside of bounds values passes the
setting and verification (the VR_VARYING case is needed because
we get ICEs otherwise, but when lhs is VR_VARYING in -ffast-math,
i.e. minimum to maximum representable finite and both signs of NaN,
then set does all we need, we don't need to or in a NaN range).
We don't really later use the range in a way that would become a problem
that it is wider than varying, we actually just perform maths on the
two boundaries.

As I said in the PR, this doesn't fix the !MODE_HAS_INFINITIES case,
I believe we actually need to treat the boundary values as infinities
in that case because they (probably) work like that, but it is unclear
if it is just the reverse operation lhs widening that is a problem there,
or whether it is a general problem.  I have zero experience with
floating points without infinities (PDP11, some ARM half type?,
what else?).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-03-10  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/109008
	* range-op-float.cc (float_widen_lhs_range): If lb is
	minimum representable finite number or ub is maximum
	representable finite number, instead of widening it to
	-inf or inf widen it to negative or positive 0x0.8p+(EMAX+1).
	Temporarily clear flag_finite_math_only when canonicalizing
	the widened range.

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

--- gcc/range-op-float.cc.jj	2023-03-09 09:54:53.880453046 +0100
+++ gcc/range-op-float.cc	2023-03-09 20:52:07.456284507 +0100
@@ -2217,12 +2217,42 @@ float_widen_lhs_range (tree type, const
   REAL_VALUE_TYPE lb = lhs.lower_bound ();
   REAL_VALUE_TYPE ub = lhs.upper_bound ();
   if (real_isfinite (&lb))
-    frange_nextafter (TYPE_MODE (type), lb, dconstninf);
+    {
+      frange_nextafter (TYPE_MODE (type), lb, dconstninf);
+      if (real_isinf (&lb))
+	{
+	  /* For -DBL_MAX, instead of -Inf use
+	     nexttoward (-DBL_MAX, -LDBL_MAX) in a hypothetical
+	     wider type with the same mantissa precision but larger
+	     exponent range; it is outside of range of double values,
+	     but makes it clear it is just one ulp larger rather than
+	     infinite amount larger.  */
+	  lb = dconstm1;
+	  SET_REAL_EXP (&lb, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1);
+	}
+    }
   if (real_isfinite (&ub))
-    frange_nextafter (TYPE_MODE (type), ub, dconstinf);
+    {
+      frange_nextafter (TYPE_MODE (type), ub, dconstinf);
+      if (real_isinf (&ub))
+	{
+	  /* For DBL_MAX similarly.  */
+	  ub = dconst1;
+	  SET_REAL_EXP (&ub, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1);
+	}
+    }
+  /* Temporarily disable -ffinite-math-only, so that frange::set doesn't
+     reduce the range back to real_min_representable (type) as lower bound
+     or real_max_representable (type) as upper bound.  */
+  bool save_flag_finite_math_only = flag_finite_math_only;
+  flag_finite_math_only = false;
   ret.set (type, lb, ub);
-  ret.clear_nan ();
-  ret.union_ (lhs);
+  if (lhs.kind () != VR_VARYING)
+    {
+      ret.clear_nan ();
+      ret.union_ (lhs);
+    }
+  flag_finite_math_only = save_flag_finite_math_only;
   return ret;
 }
 
--- gcc/testsuite/gcc.dg/pr109008.c.jj	2023-03-09 12:25:11.507955698 +0100
+++ gcc/testsuite/gcc.dg/pr109008.c	2023-03-09 12:33:35.795598344 +0100
@@ -0,0 +1,26 @@
+/* PR tree-optimization/109008 */
+/* { dg-do run } */
+/* { dg-options "-O2 -ffinite-math-only -fexcess-precision=standard" } */
+
+__attribute__((noipa)) double
+foo (double eps)
+{
+  double d = __DBL_MAX__ + eps;
+  if (d == __DBL_MAX__)
+    if (eps > 16.0)
+      return eps;
+  return 0.0;
+}
+
+int
+main ()
+{
+#if __DBL_MANT_DIG__ == 53 && __DBL_MAX_EXP__ == 1024 && __DBL_MIN_EXP__ == -1021 \
+    && __FLT_EVAL_METHOD__ == 0
+  if (foo (0x0.8p+970) == 0.0)
+    __builtin_abort ();
+  if (foo (32.0) == 0.0)
+    __builtin_abort ();
+#endif
+  return 0;
+}

	Jakub

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Richard Biener]

On Fri, 10 Mar 2023, Jakub Jelinek wrote:

> Hi!
> 
> The following patch does two things (both related to range extension
> around the boundaries).
> 
> The first part (in the 2 real_isfinite blocks) is to make the ranges
> narrower when the old boundaries are minimum and/or maximum representable
> finite number.  In that case frange_nextafter gives -Inf or +Inf,
> but then the resulting computed reverse range is very far from the actually
> needed range, usually extends up to infinity or could even result in NaNs.
> While infinities are really the next representable numbers in the
> corresponding mode, REAL_VALUE_TYPE is actually a type with wider range
> for exponent and 160 bit precision, so the patch instead uses
> nextafter number in a hypothetical floating point format with the same
> mantissa precision but wider range of exponents.  This significantly
> improves the actual ranges of the reverse operations, while still making
> them conservatively correct.
> 
> The second part is a fix for miscompilation of the new testcase below.
> For -ffinite-math-only, without this patch we extend the minimum and/or
> maximum representable finite number to -Inf or +Inf, with the patch to
> some number outside of the normal exponent range of the mode, but then
> we use set which canonicalizes it and turns the boundaries back to
> the minimum and/or maximum representable finite numbers, but because
> in say [__DBL_MAX__, __DBL_MAX__] = op1 + [__DBL_MAX__, __DBL_MAX__]
> op1 can be larger than 0, up to the largest number which rounds to even
> down back to __DBL_MAX__ and there are still no infinities involved,
> it needs to work even with -ffinite-math-only.  So, we really need to
> widen the lhs range a little bit even in that case.  The patch does
> that through temporarily clearing -ffinite-math-only, such that the
> value with infinities or the outside of bounds values passes the
> setting and verification (the VR_VARYING case is needed because
> we get ICEs otherwise, but when lhs is VR_VARYING in -ffast-math,
> i.e. minimum to maximum representable finite and both signs of NaN,
> then set does all we need, we don't need to or in a NaN range).
> We don't really later use the range in a way that would become a problem
> that it is wider than varying, we actually just perform maths on the
> two boundaries.
> 
> As I said in the PR, this doesn't fix the !MODE_HAS_INFINITIES case,
> I believe we actually need to treat the boundary values as infinities
> in that case because they (probably) work like that, but it is unclear
> if it is just the reverse operation lhs widening that is a problem there,
> or whether it is a general problem.  I have zero experience with
> floating points without infinities (PDP11, some ARM half type?,
> what else?).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> 2023-03-10  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/109008
> 	* range-op-float.cc (float_widen_lhs_range): If lb is
> 	minimum representable finite number or ub is maximum
> 	representable finite number, instead of widening it to
> 	-inf or inf widen it to negative or positive 0x0.8p+(EMAX+1).
> 	Temporarily clear flag_finite_math_only when canonicalizing
> 	the widened range.
> 
> 	* gcc.dg/pr109008.c: New test.
> 
> --- gcc/range-op-float.cc.jj	2023-03-09 09:54:53.880453046 +0100
> +++ gcc/range-op-float.cc	2023-03-09 20:52:07.456284507 +0100
> @@ -2217,12 +2217,42 @@ float_widen_lhs_range (tree type, const
>    REAL_VALUE_TYPE lb = lhs.lower_bound ();
>    REAL_VALUE_TYPE ub = lhs.upper_bound ();
>    if (real_isfinite (&lb))
> -    frange_nextafter (TYPE_MODE (type), lb, dconstninf);
> +    {
> +      frange_nextafter (TYPE_MODE (type), lb, dconstninf);
> +      if (real_isinf (&lb))
> +	{
> +	  /* For -DBL_MAX, instead of -Inf use
> +	     nexttoward (-DBL_MAX, -LDBL_MAX) in a hypothetical
> +	     wider type with the same mantissa precision but larger
> +	     exponent range; it is outside of range of double values,
> +	     but makes it clear it is just one ulp larger rather than
> +	     infinite amount larger.  */
> +	  lb = dconstm1;
> +	  SET_REAL_EXP (&lb, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1);
> +	}
> +    }
>    if (real_isfinite (&ub))
> -    frange_nextafter (TYPE_MODE (type), ub, dconstinf);
> +    {
> +      frange_nextafter (TYPE_MODE (type), ub, dconstinf);
> +      if (real_isinf (&ub))
> +	{
> +	  /* For DBL_MAX similarly.  */
> +	  ub = dconst1;
> +	  SET_REAL_EXP (&ub, FLOAT_MODE_FORMAT (TYPE_MODE (type))->emax + 1);
> +	}
> +    }
> +  /* Temporarily disable -ffinite-math-only, so that frange::set doesn't
> +     reduce the range back to real_min_representable (type) as lower bound
> +     or real_max_representable (type) as upper bound.  */
> +  bool save_flag_finite_math_only = flag_finite_math_only;
> +  flag_finite_math_only = false;
>    ret.set (type, lb, ub);
> -  ret.clear_nan ();
> -  ret.union_ (lhs);
> +  if (lhs.kind () != VR_VARYING)
> +    {
> +      ret.clear_nan ();
> +      ret.union_ (lhs);
> +    }
> +  flag_finite_math_only = save_flag_finite_math_only;

Meh - I wonder if we can avoid all this by making float_widen_lhs_range
friend of frange and simply access m_min/m_max directly and use the
copy-CTOR to copy bounds and nan state ... after all verify_range
will likely fail after you restore flag_finite_math_only ...

But OK for the moment.

Thanks,
Richard.

>    return ret;
>  }
>  
> --- gcc/testsuite/gcc.dg/pr109008.c.jj	2023-03-09 12:25:11.507955698 +0100
> +++ gcc/testsuite/gcc.dg/pr109008.c	2023-03-09 12:33:35.795598344 +0100
> @@ -0,0 +1,26 @@
> +/* PR tree-optimization/109008 */
> +/* { dg-do run } */
> +/* { dg-options "-O2 -ffinite-math-only -fexcess-precision=standard" } */
> +
> +__attribute__((noipa)) double
> +foo (double eps)
> +{
> +  double d = __DBL_MAX__ + eps;
> +  if (d == __DBL_MAX__)
> +    if (eps > 16.0)
> +      return eps;
> +  return 0.0;
> +}
> +
> +int
> +main ()
> +{
> +#if __DBL_MANT_DIG__ == 53 && __DBL_MAX_EXP__ == 1024 && __DBL_MIN_EXP__ == -1021 \
> +    && __FLT_EVAL_METHOD__ == 0
> +  if (foo (0x0.8p+970) == 0.0)
> +    __builtin_abort ();
> +  if (foo (32.0) == 0.0)
> +    __builtin_abort ();
> +#endif
> +  return 0;
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Jakub Jelinek]

On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote:
> Meh - I wonder if we can avoid all this by making float_widen_lhs_range
> friend of frange and simply access m_min/m_max directly and use the
> copy-CTOR to copy bounds and nan state ... after all verify_range
> will likely fail after you restore flag_finite_math_only ...

I'll defer such changes to Aldy.

As for verification, I think verify_range will not fail on it, it mainly
checks whether it is normalized (e.g. if minimum is frange_val_min and
maximum is frange_val_max and NaNs are possible with both signs (if NaNs
are supported) then it is VR_VARYING etc.).  It doesn't check if the actual
VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is
just equality check.
Of course, behavior of wider than varying ranges is still unexpected in
many ways, say the union_ of such a range and VR_VARYING will ICE etc.

Now, I guess another possibility for the reverse ops over these wider ranges
would be avoid calling fold_range in the reverse ops, but call rv_fold
directly or have fold_range variant which would instead of the op1, op2
argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it
would use those const REAL_VALUE_TYPE &op??b in preference to
op?.{lower,upper}_bound () or perhaps normal fold_range be implemented
in terms of this extended fold_range.  Then we wouldn't need to bother with
these non-standard franges...

> But OK for the moment.

Thanks, committed.

	Jakub

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Aldy Hernandez]

On 3/10/23 11:29, Jakub Jelinek wrote:
> On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote:
>> Meh - I wonder if we can avoid all this by making float_widen_lhs_range
>> friend of frange and simply access m_min/m_max directly and use the
>> copy-CTOR to copy bounds and nan state ... after all verify_range
>> will likely fail after you restore flag_finite_math_only ...
> 
> I'll defer such changes to Aldy.
> 
> As for verification, I think verify_range will not fail on it, it mainly
> checks whether it is normalized (e.g. if minimum is frange_val_min and
> maximum is frange_val_max and NaNs are possible with both signs (if NaNs
> are supported) then it is VR_VARYING etc.).  It doesn't check if the actual
> VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is
> just equality check.
> Of course, behavior of wider than varying ranges is still unexpected in
> many ways, say the union_ of such a range and VR_VARYING will ICE etc.
> 
> Now, I guess another possibility for the reverse ops over these wider ranges
> would be avoid calling fold_range in the reverse ops, but call rv_fold
> directly or have fold_range variant which would instead of the op1, op2
> argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it
> would use those const REAL_VALUE_TYPE &op??b in preference to
> op?.{lower,upper}_bound () or perhaps normal fold_range be implemented
> in terms of this extended fold_range.  Then we wouldn't need to bother with
> these non-standard franges...
> 
>> But OK for the moment.
> 
> Thanks, committed.

I'm not a big fan of constructing ranges that break all our internal 
consistency checks.  I'd also prefer to add another constructor (or add 
a flag to the current constructor) instead of making range-op-float 
routines friends.  We also have bits in the vrange (or frange) that we 
could use for other semantics, especially since frange_storage can be 
streamlined when stored in GC/etc.

I'm on PTO this week.  Could we revisit this next week?  And if worse 
comes to worse, leave it as is, and fix it properly next release?

Thanks for your work on this.
Aldy

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Richard Biener]

On Mon, 13 Mar 2023, Aldy Hernandez wrote:

> 
> 
> On 3/10/23 11:29, Jakub Jelinek wrote:
> > On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote:
> >> Meh - I wonder if we can avoid all this by making float_widen_lhs_range
> >> friend of frange and simply access m_min/m_max directly and use the
> >> copy-CTOR to copy bounds and nan state ... after all verify_range
> >> will likely fail after you restore flag_finite_math_only ...
> > 
> > I'll defer such changes to Aldy.
> > 
> > As for verification, I think verify_range will not fail on it, it mainly
> > checks whether it is normalized (e.g. if minimum is frange_val_min and
> > maximum is frange_val_max and NaNs are possible with both signs (if NaNs
> > are supported) then it is VR_VARYING etc.).  It doesn't check if the actual
> > VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is
> > just equality check.
> > Of course, behavior of wider than varying ranges is still unexpected in
> > many ways, say the union_ of such a range and VR_VARYING will ICE etc.
> > 
> > Now, I guess another possibility for the reverse ops over these wider ranges
> > would be avoid calling fold_range in the reverse ops, but call rv_fold
> > directly or have fold_range variant which would instead of the op1, op2
> > argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it
> > would use those const REAL_VALUE_TYPE &op??b in preference to
> > op?.{lower,upper}_bound () or perhaps normal fold_range be implemented
> > in terms of this extended fold_range.  Then we wouldn't need to bother with
> > these non-standard franges...
> > 
> >> But OK for the moment.
> > 
> > Thanks, committed.
> 
> I'm not a big fan of constructing ranges that break all our internal
> consistency checks.  I'd also prefer to add another constructor (or add a flag
> to the current constructor) instead of making range-op-float routines friends.
> We also have bits in the vrange (or frange) that we could use for other
> semantics, especially since frange_storage can be streamlined when stored in
> GC/etc.
> 
> I'm on PTO this week.  Could we revisit this next week?  And if worse comes to
> worse, leave it as is, and fix it properly next release?

Yes, sure - I just noticed that we're forced to use high-level API for
something that's quite low-level and should be internal (a range 
"breaking" internal consistency checks).

Richard.

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Aldy Hernandez]

On 3/13/23 08:50, Richard Biener wrote:
> On Mon, 13 Mar 2023, Aldy Hernandez wrote:
> 
>>
>>
>> On 3/10/23 11:29, Jakub Jelinek wrote:
>>> On Fri, Mar 10, 2023 at 08:53:37AM +0000, Richard Biener wrote:
>>>> Meh - I wonder if we can avoid all this by making float_widen_lhs_range
>>>> friend of frange and simply access m_min/m_max directly and use the
>>>> copy-CTOR to copy bounds and nan state ... after all verify_range
>>>> will likely fail after you restore flag_finite_math_only ...
>>>
>>> I'll defer such changes to Aldy.
>>>
>>> As for verification, I think verify_range will not fail on it, it mainly
>>> checks whether it is normalized (e.g. if minimum is frange_val_min and
>>> maximum is frange_val_max and NaNs are possible with both signs (if NaNs
>>> are supported) then it is VR_VARYING etc.).  It doesn't check if the actual
>>> VR_RANGE bounds are smaller or larger than the VR_VARYING bounds, there is
>>> just equality check.
>>> Of course, behavior of wider than varying ranges is still unexpected in
>>> many ways, say the union_ of such a range and VR_VARYING will ICE etc.
>>>
>>> Now, I guess another possibility for the reverse ops over these wider ranges
>>> would be avoid calling fold_range in the reverse ops, but call rv_fold
>>> directly or have fold_range variant which would instead of the op1, op2
>>> argument have 2 triplets, op1, op1lb, op1ub, op2, op2lb, op2ub, and it
>>> would use those const REAL_VALUE_TYPE &op??b in preference to
>>> op?.{lower,upper}_bound () or perhaps normal fold_range be implemented
>>> in terms of this extended fold_range.  Then we wouldn't need to bother with
>>> these non-standard franges...
>>>
>>>> But OK for the moment.
>>>
>>> Thanks, committed.
>>
>> I'm not a big fan of constructing ranges that break all our internal
>> consistency checks.  I'd also prefer to add another constructor (or add a flag
>> to the current constructor) instead of making range-op-float routines friends.
>> We also have bits in the vrange (or frange) that we could use for other
>> semantics, especially since frange_storage can be streamlined when stored in
>> GC/etc.
>>
>> I'm on PTO this week.  Could we revisit this next week?  And if worse comes to
>> worse, leave it as is, and fix it properly next release?
> 
> Yes, sure - I just noticed that we're forced to use high-level API for
> something that's quite low-level and should be internal (a range
> "breaking" internal consistency checks).

Yeah, let's fix the API.  No sense hacking around things if what we need 
is to tweak the design.

I don't like hacking around things.  It always comes back to bite me ;-).

Aldy

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Jakub Jelinek]

On Mon, Mar 13, 2023 at 08:59:15AM +0100, Aldy Hernandez wrote:
> > Yes, sure - I just noticed that we're forced to use high-level API for
> > something that's quite low-level and should be internal (a range
> > "breaking" internal consistency checks).
> 
> Yeah, let's fix the API.  No sense hacking around things if what we need is
> to tweak the design.
> 
> I don't like hacking around things.  It always comes back to bite me ;-).

Sure.  The current state is that I think the actual bugs are fixed except
for the !MODE_HAS_INFINITIES case which people rarely use, so fixing up the
API can wait even to next release.

For !MODE_HAS_INFINITIES, I wonder if the best fix wouldn't be to change
set and a few other spots, so that if the boundaries are 
real_min_representable/real_max_representable, we widen them to -inf and inf
and change frange_val_min/max to also be dconstninf/dconstinf for
!MODE_HAS_INFINITIES, because the min/max for that case (probably) really work as
infinities.  Whenever we actually round that value to mode, it will become
real_min_representable/real_max_representable again.
But that can also wait a week.

	Jakub

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Aldy Hernandez]

On 3/13/23 09:06, Jakub Jelinek wrote:
> On Mon, Mar 13, 2023 at 08:59:15AM +0100, Aldy Hernandez wrote:
>>> Yes, sure - I just noticed that we're forced to use high-level API for
>>> something that's quite low-level and should be internal (a range
>>> "breaking" internal consistency checks).
>>
>> Yeah, let's fix the API.  No sense hacking around things if what we need is
>> to tweak the design.
>>
>> I don't like hacking around things.  It always comes back to bite me ;-).
> 
> Sure.  The current state is that I think the actual bugs are fixed except
> for the !MODE_HAS_INFINITIES case which people rarely use, so fixing up the
> API can wait even to next release.
> 
> For !MODE_HAS_INFINITIES, I wonder if the best fix wouldn't be to change
> set and a few other spots, so that if the boundaries are
> real_min_representable/real_max_representable, we widen them to -inf and inf
> and change frange_val_min/max to also be dconstninf/dconstinf for
> !MODE_HAS_INFINITIES, because the min/max for that case (probably) really work as
> infinities.  Whenever we actually round that value to mode, it will become
> real_min_representable/real_max_representable again.
> But that can also wait a week.

That sounds very reasonable.  It would remove special casing and would 
make the code easier to read.  For that matter, that was what I had in 
the original implementation.

Aldy

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Jakub Jelinek]

On Mon, Mar 13, 2023 at 09:41:47AM +0100, Aldy Hernandez wrote:
> On 3/13/23 09:06, Jakub Jelinek wrote:
> > On Mon, Mar 13, 2023 at 08:59:15AM +0100, Aldy Hernandez wrote:
> > > > Yes, sure - I just noticed that we're forced to use high-level API for
> > > > something that's quite low-level and should be internal (a range
> > > > "breaking" internal consistency checks).
> > > 
> > > Yeah, let's fix the API.  No sense hacking around things if what we need is
> > > to tweak the design.
> > > 
> > > I don't like hacking around things.  It always comes back to bite me ;-).
> > 
> > Sure.  The current state is that I think the actual bugs are fixed except
> > for the !MODE_HAS_INFINITIES case which people rarely use, so fixing up the
> > API can wait even to next release.
> > 
> > For !MODE_HAS_INFINITIES, I wonder if the best fix wouldn't be to change
> > set and a few other spots, so that if the boundaries are
> > real_min_representable/real_max_representable, we widen them to -inf and inf
> > and change frange_val_min/max to also be dconstninf/dconstinf for
> > !MODE_HAS_INFINITIES, because the min/max for that case (probably) really work as
> > infinities.  Whenever we actually round that value to mode, it will become
> > real_min_representable/real_max_representable again.
> > But that can also wait a week.
> 
> That sounds very reasonable.  It would remove special casing and would make
> the code easier to read.  For that matter, that was what I had in the
> original implementation.

I think we don't want to remove the special casing for -ffinite-math-only
on types which do support infinities.
Thinking further on it, perhaps for !MODE_HAS_INFINITIES a better fix would
be to do something like the patch below.
Consider say having a range of VAX float type:
#define M0 -FLT_MAX
#define M1 nextafterf (F0, FLT_MAX)
#define M2 nextafterf (M1, FLT_MAX)
[M2, M2] - [M0, M1]
Or perhaps if one or both of the operands are in such a case a min and max,
perform real_arithmetic recurse on the argument replaced with
dconstninf/dconstinf and then depending on inf pick the mininum or maximum
of the two results (and carefully think about what to do if both operands
are min/max).

2023-03-20  Jakub Jelinek  <jakub@redhat.com>

	* range-op-float.cc (frange_arithmetic): For !MODE_HAS_INFINITIES
	types, pretend operands with minimum or maximum values are actually
	infinities.

--- gcc/range-op-float.cc.jj	2023-03-10 12:40:19.673108938 +0100
+++ gcc/range-op-float.cc	2023-03-20 16:58:36.604981486 +0100
@@ -313,8 +313,26 @@ frange_arithmetic (enum tree_code code,
   REAL_VALUE_TYPE value;
   enum machine_mode mode = TYPE_MODE (type);
   bool mode_composite = MODE_COMPOSITE_P (mode);
+  const REAL_VALUE_TYPE *pop1 = &op1;
+  const REAL_VALUE_TYPE *pop2 = &op2;
 
-  bool inexact = real_arithmetic (&value, code, &op1, &op2);
+  if (!MODE_HAS_INFINITIES (mode))
+    {
+      // If mode doesn't have infinities, the minimum and maximum
+      // values are saturating.  Pretend for real_arithmetic such
+      // values are actual infinities.  real_convert will then
+      // canonicalize the result not to be an infinity.
+      if (frange_val_is_min (op1, type))
+	pop1 = &dconstninf;
+      else if (frange_val_is_max (op1, type))
+	pop1 = &dconstinf;
+      if (frange_val_is_min (op2, type))
+	pop2 = &dconstninf;
+      else if (frange_val_is_max (op2, type))
+	pop2 = &dconstinf;
+    }
+
+  bool inexact = real_arithmetic (&value, code, pop1, pop2);
   real_convert (&result, mode, &value);
 
   // Be extra careful if there may be discrepancies between the


	Jakub

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Aldy Hernandez]

On Mon, Mar 20, 2023 at 5:14 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Mon, Mar 13, 2023 at 09:41:47AM +0100, Aldy Hernandez wrote:
> > On 3/13/23 09:06, Jakub Jelinek wrote:
> > > On Mon, Mar 13, 2023 at 08:59:15AM +0100, Aldy Hernandez wrote:
> > > > > Yes, sure - I just noticed that we're forced to use high-level API for
> > > > > something that's quite low-level and should be internal (a range
> > > > > "breaking" internal consistency checks).
> > > >
> > > > Yeah, let's fix the API.  No sense hacking around things if what we need is
> > > > to tweak the design.
> > > >
> > > > I don't like hacking around things.  It always comes back to bite me ;-).
> > >
> > > Sure.  The current state is that I think the actual bugs are fixed except
> > > for the !MODE_HAS_INFINITIES case which people rarely use, so fixing up the
> > > API can wait even to next release.
> > >
> > > For !MODE_HAS_INFINITIES, I wonder if the best fix wouldn't be to change
> > > set and a few other spots, so that if the boundaries are
> > > real_min_representable/real_max_representable, we widen them to -inf and inf
> > > and change frange_val_min/max to also be dconstninf/dconstinf for
> > > !MODE_HAS_INFINITIES, because the min/max for that case (probably) really work as
> > > infinities.  Whenever we actually round that value to mode, it will become
> > > real_min_representable/real_max_representable again.
> > > But that can also wait a week.
> >
> > That sounds very reasonable.  It would remove special casing and would make
> > the code easier to read.  For that matter, that was what I had in the
> > original implementation.
>
> I think we don't want to remove the special casing for -ffinite-math-only
> on types which do support infinities.
> Thinking further on it, perhaps for !MODE_HAS_INFINITIES a better fix would
> be to do something like the patch below.
> Consider say having a range of VAX float type:
> #define M0 -FLT_MAX
> #define M1 nextafterf (F0, FLT_MAX)
> #define M2 nextafterf (M1, FLT_MAX)
> [M2, M2] - [M0, M1]
> Or perhaps if one or both of the operands are in such a case a min and max,
> perform real_arithmetic recurse on the argument replaced with
> dconstninf/dconstinf and then depending on inf pick the mininum or maximum
> of the two results (and carefully think about what to do if both operands
> are min/max).

LGTM.
Aldy

>
> 2023-03-20  Jakub Jelinek  <jakub@redhat.com>
>
>         * range-op-float.cc (frange_arithmetic): For !MODE_HAS_INFINITIES
>         types, pretend operands with minimum or maximum values are actually
>         infinities.
>
> --- gcc/range-op-float.cc.jj    2023-03-10 12:40:19.673108938 +0100
> +++ gcc/range-op-float.cc       2023-03-20 16:58:36.604981486 +0100
> @@ -313,8 +313,26 @@ frange_arithmetic (enum tree_code code,
>    REAL_VALUE_TYPE value;
>    enum machine_mode mode = TYPE_MODE (type);
>    bool mode_composite = MODE_COMPOSITE_P (mode);
> +  const REAL_VALUE_TYPE *pop1 = &op1;
> +  const REAL_VALUE_TYPE *pop2 = &op2;
>
> -  bool inexact = real_arithmetic (&value, code, &op1, &op2);
> +  if (!MODE_HAS_INFINITIES (mode))
> +    {
> +      // If mode doesn't have infinities, the minimum and maximum
> +      // values are saturating.  Pretend for real_arithmetic such
> +      // values are actual infinities.  real_convert will then
> +      // canonicalize the result not to be an infinity.
> +      if (frange_val_is_min (op1, type))
> +       pop1 = &dconstninf;
> +      else if (frange_val_is_max (op1, type))
> +       pop1 = &dconstinf;
> +      if (frange_val_is_min (op2, type))
> +       pop2 = &dconstninf;
> +      else if (frange_val_is_max (op2, type))
> +       pop2 = &dconstinf;
> +    }
> +
> +  bool inexact = real_arithmetic (&value, code, pop1, pop2);
>    real_convert (&result, mode, &value);
>
>    // Be extra careful if there may be discrepancies between the
>
>
>         Jakub
>

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Aldy Hernandez]

On 3/10/23 09:53, Richard Biener wrote:
> On Fri, 10 Mar 2023, Jakub Jelinek wrote:

Coming back to this...

>   /* Temporarily disable -ffinite-math-only, so that frange::set doesn't
>      reduce the range back to real_min_representable (type) as lower bound
>      or real_max_representable (type) as upper bound.  */
>   bool save_flag_finite_math_only = flag_finite_math_only;
>   flag_finite_math_only = false;
>   ret.set (type, lb, ub);
>   if (lhs.kind () != VR_VARYING)
>     {
>       ret.clear_nan ();
>       ret.union_ (lhs);
>     }
>   flag_finite_math_only = save_flag_finite_math_only;

It looks like what you want to do is be able to create a range with a 
known NAN state, but without the setter reducing the range to 
min/max_representable.

How about we enhance the API to provide:

1. Constructor with a known NAN state.
2. Setter with a flag to keep it from canonicalizing into 
min/max_representable.

The flag in 2 could in the future be saved in the frange object to keep 
union and friends from further canonicalization.

So the above could be written as:

	// Construct [lb, ub] with a known NAN state.
	frange tmp (lb, ub, lhs.get_nan_state ());

	// Set RET without dropping/reducing the range to MIN/MAX.
	ret.set (tmp, FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION);

An alternative is to allow the setter to set everything:

	ret.set (type, lb, ub,
		lhs.get_nan_state (),
		FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION);

Would this work?  I'd be happy to whip up something this week, or if 
preferred, leave it to the next release.

Aldy

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Jakub Jelinek]

On Tue, Mar 21, 2023 at 02:28:31PM +0100, Aldy Hernandez wrote:
> >   /* Temporarily disable -ffinite-math-only, so that frange::set doesn't
> >      reduce the range back to real_min_representable (type) as lower bound
> >      or real_max_representable (type) as upper bound.  */
> >   bool save_flag_finite_math_only = flag_finite_math_only;
> >   flag_finite_math_only = false;
> >   ret.set (type, lb, ub);
> >   if (lhs.kind () != VR_VARYING)
> >     {
> >       ret.clear_nan ();
> >       ret.union_ (lhs);
> >     }
> >   flag_finite_math_only = save_flag_finite_math_only;
> 
> It looks like what you want to do is be able to create a range with a known
> NAN state, but without the setter reducing the range to
> min/max_representable.
> 
> How about we enhance the API to provide:
> 
> 1. Constructor with a known NAN state.
> 2. Setter with a flag to keep it from canonicalizing into
> min/max_representable.
> 
> The flag in 2 could in the future be saved in the frange object to keep
> union and friends from further canonicalization.
> 
> So the above could be written as:
> 
> 	// Construct [lb, ub] with a known NAN state.
> 	frange tmp (lb, ub, lhs.get_nan_state ());
> 
> 	// Set RET without dropping/reducing the range to MIN/MAX.
> 	ret.set (tmp, FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION);
> 
> An alternative is to allow the setter to set everything:
> 
> 	ret.set (type, lb, ub,
> 		lhs.get_nan_state (),
> 		FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION);
> 
> Would this work?  I'd be happy to whip up something this week, or if
> preferred, leave it to the next release.

The latter would be better, I really don't need a temporary range in that
spot, the union_ is only to copy the NaN state.
Though, I think right now set actually doesn't do reduction to representable
at all, all it does is equality compare bounds against the applicable
boundaries and if both are equal and NaN state is appropriate, change it
into VR_VARYING.
So maybe for now all we need is the 4 argument set.

	Jakub

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Aldy Hernandez]

On 3/21/23 14:39, Jakub Jelinek wrote:
> On Tue, Mar 21, 2023 at 02:28:31PM +0100, Aldy Hernandez wrote:
>>>    /* Temporarily disable -ffinite-math-only, so that frange::set doesn't
>>>       reduce the range back to real_min_representable (type) as lower bound
>>>       or real_max_representable (type) as upper bound.  */
>>>    bool save_flag_finite_math_only = flag_finite_math_only;
>>>    flag_finite_math_only = false;
>>>    ret.set (type, lb, ub);
>>>    if (lhs.kind () != VR_VARYING)
>>>      {
>>>        ret.clear_nan ();
>>>        ret.union_ (lhs);
>>>      }
>>>    flag_finite_math_only = save_flag_finite_math_only;
>>
>> It looks like what you want to do is be able to create a range with a known
>> NAN state, but without the setter reducing the range to
>> min/max_representable.
>>
>> How about we enhance the API to provide:
>>
>> 1. Constructor with a known NAN state.
>> 2. Setter with a flag to keep it from canonicalizing into
>> min/max_representable.
>>
>> The flag in 2 could in the future be saved in the frange object to keep
>> union and friends from further canonicalization.
>>
>> So the above could be written as:
>>
>> 	// Construct [lb, ub] with a known NAN state.
>> 	frange tmp (lb, ub, lhs.get_nan_state ());
>>
>> 	// Set RET without dropping/reducing the range to MIN/MAX.
>> 	ret.set (tmp, FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION);
>>
>> An alternative is to allow the setter to set everything:
>>
>> 	ret.set (type, lb, ub,
>> 		lhs.get_nan_state (),
>> 		FRANGE_FLAG_NO_REPRESENTABLE_REDUCTION);
>>
>> Would this work?  I'd be happy to whip up something this week, or if
>> preferred, leave it to the next release.
> 
> The latter would be better, I really don't need a temporary range in that
> spot, the union_ is only to copy the NaN state.
> Though, I think right now set actually doesn't do reduction to representable
> at all, all it does is equality compare bounds against the applicable
> boundaries and if both are equal and NaN state is appropriate, change it
> into VR_VARYING.
> So maybe for now all we need is the 4 argument set.

So, this?

frange::set (tree type,
	const REAL_VALUE_TYPE &min,
	const REAL_VALUE_TYPE &max,
	const nan_state &,
	value_range_kind kind = VR_RANGE)

If so, I'll start poking at it.

Aldy

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Jakub Jelinek]

On Tue, Mar 21, 2023 at 02:49:49PM +0100, Aldy Hernandez wrote:
> So, this?
> 
> frange::set (tree type,
> 	const REAL_VALUE_TYPE &min,
> 	const REAL_VALUE_TYPE &max,
> 	const nan_state &,
> 	value_range_kind kind = VR_RANGE)
> 
> If so, I'll start poking at it.

Yes.

	Jakub

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Aldy Hernandez]

[-- Attachment #1: Type: text/plain, Size: 372 bytes --]



On 3/21/23 14:56, Jakub Jelinek wrote:
> On Tue, Mar 21, 2023 at 02:49:49PM +0100, Aldy Hernandez wrote:
>> So, this?
>>
>> frange::set (tree type,
>> 	const REAL_VALUE_TYPE &min,
>> 	const REAL_VALUE_TYPE &max,
>> 	const nan_state &,
>> 	value_range_kind kind = VR_RANGE)
>>
>> If so, I'll start poking at it.
> 
> Yes.
> 
> 	Jakub
> 

Tested on x86-64 Linux.

OK?
Aldy

[-- Attachment #2: 0001-frange-Implement-nan_state-class-PR109008.patch --]
[-- Type: text/x-patch, Size: 4446 bytes --]

From 64b9fabc0f31c661bb72029440227fe319566654 Mon Sep 17 00:00:00 2001
From: Aldy Hernandez <aldyh@redhat.com>
Date: Wed, 8 Mar 2023 10:58:01 +0100
Subject: [PATCH] frange: Implement nan_state class [PR109008]

This patch implements a nan_state class, that allows us to query or
pass around the NANness of an frange.  We can store +NAN, -NAN, +-NAN,
or not-a-NAN with it.

I tried to touch as little as possible, leaving other cleanups to the
next release.  For example, we should replace the m_*_nan fields in
frange with nan_state, and provide relevant accessors to nan_state
(isnan, etc).

gcc/ChangeLog:

	* value-range.cc (frange::set): Add nan_state argument.
	* value-range.h (class nan_state): New.
	(frange::get_nan_state): New.
---
 gcc/value-range.cc | 18 +++++++++++---
 gcc/value-range.h  | 62 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/gcc/value-range.cc b/gcc/value-range.cc
index a535337c47a..f71554b7d7c 100644
--- a/gcc/value-range.cc
+++ b/gcc/value-range.cc
@@ -284,7 +284,7 @@ frange::flush_denormals_to_zero ()
 void
 frange::set (tree type,
 	     const REAL_VALUE_TYPE &min, const REAL_VALUE_TYPE &max,
-	     value_range_kind kind)
+	     const nan_state &nan, value_range_kind kind)
 {
   switch (kind)
     {
@@ -316,8 +316,8 @@ frange::set (tree type,
   m_max = max;
   if (HONOR_NANS (m_type))
     {
-      m_pos_nan = true;
-      m_neg_nan = true;
+      m_pos_nan = nan.pos_p ();
+      m_neg_nan = nan.neg_p ();
     }
   else
     {
@@ -367,6 +367,18 @@ frange::set (tree type,
     verify_range ();
 }
 
+// Setter for an frange defaulting the NAN possibility to +-NAN when
+// HONOR_NANS.
+
+void
+frange::set (tree type,
+	     const REAL_VALUE_TYPE &min, const REAL_VALUE_TYPE &max,
+	     value_range_kind kind)
+{
+  nan_state nan;
+  set (type, min, max, nan, kind);
+}
+
 void
 frange::set (tree min, tree max, value_range_kind kind)
 {
diff --git a/gcc/value-range.h b/gcc/value-range.h
index f4ac73b499f..ec50346826c 100644
--- a/gcc/value-range.h
+++ b/gcc/value-range.h
@@ -268,6 +268,56 @@ public:
   virtual void accept (const vrange_visitor &v) const override;
 };
 
+// The NAN state as an opaque object.  The default constructor is +-NAN.
+
+class nan_state
+{
+public:
+  nan_state ();
+  nan_state (bool pos_nan, bool neg_nan);
+  bool neg_p () const;
+  bool pos_p () const;
+private:
+  bool m_pos_nan;
+  bool m_neg_nan;
+};
+
+// Default constructor initializing the object to +-NAN.
+
+inline
+nan_state::nan_state ()
+{
+  m_pos_nan = true;
+  m_neg_nan = true;
+}
+
+// Constructor initializing the object to +NAN if POS_NAN is set, -NAN
+// if NEG_NAN is set, or +-NAN if both are set.  Otherwise POS_NAN and
+// NEG_NAN are clear, and the object cannot be a NAN.
+
+inline
+nan_state::nan_state (bool pos_nan, bool neg_nan)
+{
+  m_pos_nan = pos_nan;
+  m_neg_nan = neg_nan;
+}
+
+// Return if +NAN is possible.
+
+inline bool
+nan_state::pos_p () const
+{
+  return m_pos_nan;
+}
+
+// Return if -NAN is possible.
+
+inline bool
+nan_state::neg_p () const
+{
+  return m_neg_nan;
+}
+
 // A floating point range.
 //
 // The representation is a type with a couple of endpoints, unioned
@@ -295,6 +345,8 @@ public:
   virtual void set (tree, tree, value_range_kind = VR_RANGE) override;
   void set (tree type, const REAL_VALUE_TYPE &, const REAL_VALUE_TYPE &,
 	    value_range_kind = VR_RANGE);
+  void set (tree type, const REAL_VALUE_TYPE &, const REAL_VALUE_TYPE &,
+	    const nan_state &, value_range_kind = VR_RANGE);
   void set_nan (tree type);
   void set_nan (tree type, bool sign);
   virtual void set_varying (tree type) override;
@@ -315,9 +367,11 @@ public:
   bool operator!= (const frange &r) const { return !(*this == r); }
   const REAL_VALUE_TYPE &lower_bound () const;
   const REAL_VALUE_TYPE &upper_bound () const;
+  nan_state get_nan_state () const;
   void update_nan ();
   void update_nan (bool sign);
   void update_nan (tree) = delete; // Disallow silent conversion to bool.
+  void update_nan (const nan_state &);
   void clear_nan ();
 
   // fpclassify like API
@@ -358,6 +412,14 @@ frange::upper_bound () const
   return m_max;
 }
 
+// Return the NAN state.
+
+inline nan_state
+frange::get_nan_state () const
+{
+  return nan_state (m_pos_nan, m_neg_nan);
+}
+
 // is_a<> and as_a<> implementation for vrange.
 
 // Anything we haven't specialized is a hard fail.
-- 
2.39.2

Re: [PATCH] range-op-float: Fix up -ffinite-math-only range extension and don't extend into infinities [PR109008]

[Jakub Jelinek]

On Wed, Mar 22, 2023 at 07:32:44AM +0100, Aldy Hernandez wrote:
> On 3/21/23 14:56, Jakub Jelinek wrote:
> > On Tue, Mar 21, 2023 at 02:49:49PM +0100, Aldy Hernandez wrote:
> > > So, this?
> > > 
> > > frange::set (tree type,
> > > 	const REAL_VALUE_TYPE &min,
> > > 	const REAL_VALUE_TYPE &max,
> > > 	const nan_state &,
> > > 	value_range_kind kind = VR_RANGE)
> > > 
> > > If so, I'll start poking at it.
> > 
> > Yes.
> > 
> > 	Jakub
> > 
> 
> Tested on x86-64 Linux.
> 
> OK?

Yes, thanks.

	Jakub

[PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range

[Jakub Jelinek]

Hi!

On Wed, Mar 22, 2023 at 07:32:44AM +0100, Aldy Hernandez wrote:
> 	* value-range.cc (frange::set): Add nan_state argument.
> 	* value-range.h (class nan_state): New.
> 	(frange::get_nan_state): New.

The following patch makes use of those changes in float_widen_lhs_range.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2023-03-28  Jakub Jelinek  <jakub@redhat.com>

	* range-op-float.cc (float_widen_lhs_range): Use pass get_nan_state
	as 4th argument to set to avoid clear_nan and union_ calls.

--- gcc/range-op-float.cc.jj	2023-03-23 15:25:47.119740274 +0100
+++ gcc/range-op-float.cc	2023-03-27 13:28:18.847264635 +0200
@@ -2262,12 +2262,7 @@ float_widen_lhs_range (tree type, const
      or real_max_representable (type) as upper bound.  */
   bool save_flag_finite_math_only = flag_finite_math_only;
   flag_finite_math_only = false;
-  ret.set (type, lb, ub);
-  if (lhs.kind () != VR_VARYING)
-    {
-      ret.clear_nan ();
-      ret.union_ (lhs);
-    }
+  ret.set (type, lb, ub, lhs.get_nan_state ());
   flag_finite_math_only = save_flag_finite_math_only;
   return ret;
 }


	Jakub

Re: [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range

[Aldy Hernandez]

On 3/28/23 09:54, Jakub Jelinek wrote:
> Hi!
> 
> On Wed, Mar 22, 2023 at 07:32:44AM +0100, Aldy Hernandez wrote:
>> 	* value-range.cc (frange::set): Add nan_state argument.
>> 	* value-range.h (class nan_state): New.
>> 	(frange::get_nan_state): New.
> 
> The following patch makes use of those changes in float_widen_lhs_range.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

LGTM.
Aldy
> 
> 2023-03-28  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* range-op-float.cc (float_widen_lhs_range): Use pass get_nan_state
> 	as 4th argument to set to avoid clear_nan and union_ calls.
> 
> --- gcc/range-op-float.cc.jj	2023-03-23 15:25:47.119740274 +0100
> +++ gcc/range-op-float.cc	2023-03-27 13:28:18.847264635 +0200
> @@ -2262,12 +2262,7 @@ float_widen_lhs_range (tree type, const
>        or real_max_representable (type) as upper bound.  */
>     bool save_flag_finite_math_only = flag_finite_math_only;
>     flag_finite_math_only = false;
> -  ret.set (type, lb, ub);
> -  if (lhs.kind () != VR_VARYING)
> -    {
> -      ret.clear_nan ();
> -      ret.union_ (lhs);
> -    }
> +  ret.set (type, lb, ub, lhs.get_nan_state ());
>     flag_finite_math_only = save_flag_finite_math_only;
>     return ret;
>   }
> 
> 
> 	Jakub
> 

Re: [PATCH] range-op-float: Use get_nan_state in float_widen_lhs_range

[Aldy Hernandez]

On 3/28/23 10:50, Aldy Hernandez wrote:
> 
> 
> On 3/28/23 09:54, Jakub Jelinek wrote:
>> Hi!
>>
>> On Wed, Mar 22, 2023 at 07:32:44AM +0100, Aldy Hernandez wrote:
>>>     * value-range.cc (frange::set): Add nan_state argument.
>>>     * value-range.h (class nan_state): New.
>>>     (frange::get_nan_state): New.
>>
>> The following patch makes use of those changes in float_widen_lhs_range.
>>
>> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> 
> LGTM.
> Aldy
>>
>> 2023-03-28  Jakub Jelinek  <jakub@redhat.com>
>>
>>     * range-op-float.cc (float_widen_lhs_range): Use pass get_nan_state
>>     as 4th argument to set to avoid clear_nan and union_ calls.
>>
>> --- gcc/range-op-float.cc.jj    2023-03-23 15:25:47.119740274 +0100
>> +++ gcc/range-op-float.cc    2023-03-27 13:28:18.847264635 +0200
>> @@ -2262,12 +2262,7 @@ float_widen_lhs_range (tree type, const
>>        or real_max_representable (type) as upper bound.  */
>>     bool save_flag_finite_math_only = flag_finite_math_only;
>>     flag_finite_math_only = false;
>> -  ret.set (type, lb, ub);
>> -  if (lhs.kind () != VR_VARYING)
>> -    {
>> -      ret.clear_nan ();
>> -      ret.union_ (lhs);
>> -    }
>> +  ret.set (type, lb, ub, lhs.get_nan_state ());
>>     flag_finite_math_only = save_flag_finite_math_only;
>>     return ret;

I just noticed we're still doing the flag_finite_math_only hack.  We 
should add some sort of flag to the setter to avoid reducing the range. 
I suppose this can wait to the next release.

Aldy