range-op-float: frange_arithmetic tweaks for MODE_COMPOSITE_P

  Hi!

As mentioned in PR107967, ibm-ldouble-format documents that
+- has 1ulp accuracy, * 2ulps and / 3ulps.
So, even if the result is exact, we need to widen the range a little bit.

The following patch does that.  I just wonder what it means for reverse
division (the op1_range case), which we implement through multiplication,
when division has 3ulps error and multiplication just 2ulps.  In any case,
this format is a mess and for non-default rounding modes can't be trusted
at all, instead of +inf or something close to it it happily computes -inf.

2022-12-07  Jakub Jelinek  <jakub@redhat.com>

	* range-op-float.cc (frange_arithmetic): For mode_composite,
	on top of rounding in the right direction accept extra 1ulp
	error for PLUS/MINUS_EXPR, extra 2ulps error for MULT_EXPR
	and extra 3ulps error for RDIV_EXPR.


	Jakub
  

On 12/7/22 13:10, Jakub Jelinek wrote:
> Hi!
> 
> As mentioned in PR107967, ibm-ldouble-format documents that
> +- has 1ulp accuracy, * 2ulps and / 3ulps.
> So, even if the result is exact, we need to widen the range a little bit.
> 
> The following patch does that.  I just wonder what it means for reverse
> division (the op1_range case), which we implement through multiplication,
> when division has 3ulps error and multiplication just 2ulps.  In any case,
> this format is a mess and for non-default rounding modes can't be trusted
> at all, instead of +inf or something close to it it happily computes -inf.
> 
> 2022-12-07  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* range-op-float.cc (frange_arithmetic): For mode_composite,
> 	on top of rounding in the right direction accept extra 1ulp
> 	error for PLUS/MINUS_EXPR, extra 2ulps error for MULT_EXPR
> 	and extra 3ulps error for RDIV_EXPR.
> 
> --- gcc/range-op-float.cc.jj	2022-12-07 12:46:01.536123757 +0100
> +++ gcc/range-op-float.cc	2022-12-07 12:50:40.812085139 +0100
> @@ -344,22 +344,70 @@ frange_arithmetic (enum tree_code code,
>   	    }
>   	}
>       }
> -  if (round && (inexact || !real_identical (&result, &value)))
> +  if (!inexact && !real_identical (&result, &value))
> +    inexact = true;
> +  if (round && (inexact || mode_composite))
>       {
>         if (mode_composite)
>   	{
> -	  if (real_isdenormal (&result, mode)
> -	      || real_iszero (&result))
> +	  if (real_isdenormal (&result, mode) || real_iszero (&result))
>   	    {
>   	      // IBM extended denormals only have DFmode precision.
>   	      REAL_VALUE_TYPE tmp;
>   	      real_convert (&tmp, DFmode, &value);
> -	      frange_nextafter (DFmode, tmp, inf);
> +	      if (inexact)
> +		frange_nextafter (DFmode, tmp, inf);
> +	      switch (code)
> +		{
> +		case PLUS_EXPR:
> +		case MINUS_EXPR:
> +		  // ibm-ldouble-format documents 1ulp for + and -.
> +		  frange_nextafter (DFmode, tmp, inf);
> +		  break;
> +		case MULT_EXPR:
> +		  // ibm-ldouble-format documents 2ulps for *.
> +		  frange_nextafter (DFmode, tmp, inf);
> +		  frange_nextafter (DFmode, tmp, inf);
> +		  break;
> +		case RDIV_EXPR:
> +		  // ibm-ldouble-format documents 3ulps for /.
> +		  frange_nextafter (DFmode, tmp, inf);
> +		  frange_nextafter (DFmode, tmp, inf);
> +		  frange_nextafter (DFmode, tmp, inf);
> +		  break;
> +		default:
> +		  if (!inexact)
> +		    return;
> +		  break;

It looks like this chunk...


> +		}
>   	      real_convert (&result, mode, &tmp);
>   	      return;
>   	    }
>   	}
> -      frange_nextafter (mode, result, inf);
> +      if (inexact)
> +	frange_nextafter (mode, result, inf);
> +      if (mode_composite)
> +	switch (code)
> +	  {
> +	  case PLUS_EXPR:
> +	  case MINUS_EXPR:
> +	    // ibm-ldouble-format documents 1ulp for + and -.
> +	    frange_nextafter (mode, result, inf);
> +	    break;
> +	  case MULT_EXPR:
> +	    // ibm-ldouble-format documents 2ulps for *.
> +	    frange_nextafter (mode, result, inf);
> +	    frange_nextafter (mode, result, inf);
> +	    break;
> +	  case RDIV_EXPR:
> +	    // ibm-ldouble-format documents 3ulps for /.
> +	    frange_nextafter (mode, result, inf);
> +	    frange_nextafter (mode, result, inf);
> +	    frange_nextafter (mode, result, inf);
> +	    break;
> +	  default:
> +	    break;
> +	  }

...is the same as this chunk.  Plus, all this mode composite stuff is 
polluting what was a rather clean function.  Would it be possible to 
abstract this into an inline function, and then we could do:

if (mode_composite)
   frange_composite_nextafter (...);
else
   frange_nextafter (...);

or perhaps abstract the whole nextafter in frange_arithmetic into:

frange_arithmetic_nextafter () {
   if (mode_composite) { do ugly stuff }
   else frange_nextafter (...)
}

I'm most worried about maintainability, not correctness here, cause you 
obviously know what you're doing ;-).

Aldy


>       }
>   }
>   
> 
> 	Jakub
>
  

On Wed, Dec 07, 2022 at 04:21:09PM +0100, Aldy Hernandez wrote:
> On 12/7/22 13:10, Jakub Jelinek wrote:
> > +	      switch (code)
> > +		{
> > +		case PLUS_EXPR:
> > +		case MINUS_EXPR:
> > +		  // ibm-ldouble-format documents 1ulp for + and -.
> > +		  frange_nextafter (DFmode, tmp, inf);
> > +		  break;
> > +		case MULT_EXPR:
> > +		  // ibm-ldouble-format documents 2ulps for *.
> > +		  frange_nextafter (DFmode, tmp, inf);
> > +		  frange_nextafter (DFmode, tmp, inf);
> > +		  break;
> > +		case RDIV_EXPR:
> > +		  // ibm-ldouble-format documents 3ulps for /.
> > +		  frange_nextafter (DFmode, tmp, inf);
> > +		  frange_nextafter (DFmode, tmp, inf);
> > +		  frange_nextafter (DFmode, tmp, inf);
> > +		  break;
> > +		default:
> > +		  if (!inexact)
> > +		    return;
> > +		  break;
> 
> It looks like this chunk...
> 
> 
> > +	switch (code)
> > +	  {
> > +	  case PLUS_EXPR:
> > +	  case MINUS_EXPR:
> > +	    // ibm-ldouble-format documents 1ulp for + and -.
> > +	    frange_nextafter (mode, result, inf);
> > +	    break;
> > +	  case MULT_EXPR:
> > +	    // ibm-ldouble-format documents 2ulps for *.
> > +	    frange_nextafter (mode, result, inf);
> > +	    frange_nextafter (mode, result, inf);
> > +	    break;
> > +	  case RDIV_EXPR:
> > +	    // ibm-ldouble-format documents 3ulps for /.
> > +	    frange_nextafter (mode, result, inf);
> > +	    frange_nextafter (mode, result, inf);
> > +	    frange_nextafter (mode, result, inf);
> > +	    break;
> > +	  default:
> > +	    break;
> > +	  }
> 
> ...is the same as this chunk.  Plus, all this mode composite stuff is

It is not the same, there is the DFmode, tmp vs. mode, result difference.
But sure, we could either add an inline function which for
(code, mode, result, inf) set of options (or (code, DFmode, tmp, inf))
do those 0, 1, 2, 3 frange_nextafter calls (and return bool if it did any
- there is also the if (!inexact) return; case), or as you suggest
perhaps change frange_nextafter to handle MODE_COMPOSITE_P differently
and do there
  if (mode_composite && (real_isdenormal (&result, mode) || real_iszero (&result)))
    {
      // IBM extended denormals only have DFmode precision.
      REAL_VALUE_TYPE tmp;
      real_convert (&tmp, DFmode, &result);
      frange_nextafter (DFmode, tmp, inf);
      real_convert (&result, mode, &tmp);
    }
  else
    frange_nextafter (mode, result, inf);
Though, that somewhat changes behavior, it will convert to DFmode and back
for every nextafter rather than just once (just slower compile time),
but also right now we start from value rather than result.

So, perhaps a combination of that, change frange_nextafter to do the above
and change frange_arithmetic for the initial inexact rounding only to
do it by hand using range_nextafter and starting from value.

Anyway, this patch is far less important than the previous one...

	Jakub
  

On 12/7/22 16:31, Jakub Jelinek wrote:
> On Wed, Dec 07, 2022 at 04:21:09PM +0100, Aldy Hernandez wrote:
>> On 12/7/22 13:10, Jakub Jelinek wrote:
>>> +	      switch (code)
>>> +		{
>>> +		case PLUS_EXPR:
>>> +		case MINUS_EXPR:
>>> +		  // ibm-ldouble-format documents 1ulp for + and -.
>>> +		  frange_nextafter (DFmode, tmp, inf);
>>> +		  break;
>>> +		case MULT_EXPR:
>>> +		  // ibm-ldouble-format documents 2ulps for *.
>>> +		  frange_nextafter (DFmode, tmp, inf);
>>> +		  frange_nextafter (DFmode, tmp, inf);
>>> +		  break;
>>> +		case RDIV_EXPR:
>>> +		  // ibm-ldouble-format documents 3ulps for /.
>>> +		  frange_nextafter (DFmode, tmp, inf);
>>> +		  frange_nextafter (DFmode, tmp, inf);
>>> +		  frange_nextafter (DFmode, tmp, inf);
>>> +		  break;
>>> +		default:
>>> +		  if (!inexact)
>>> +		    return;
>>> +		  break;
>>
>> It looks like this chunk...
>>
>>
>>> +	switch (code)
>>> +	  {
>>> +	  case PLUS_EXPR:
>>> +	  case MINUS_EXPR:
>>> +	    // ibm-ldouble-format documents 1ulp for + and -.
>>> +	    frange_nextafter (mode, result, inf);
>>> +	    break;
>>> +	  case MULT_EXPR:
>>> +	    // ibm-ldouble-format documents 2ulps for *.
>>> +	    frange_nextafter (mode, result, inf);
>>> +	    frange_nextafter (mode, result, inf);
>>> +	    break;
>>> +	  case RDIV_EXPR:
>>> +	    // ibm-ldouble-format documents 3ulps for /.
>>> +	    frange_nextafter (mode, result, inf);
>>> +	    frange_nextafter (mode, result, inf);
>>> +	    frange_nextafter (mode, result, inf);
>>> +	    break;
>>> +	  default:
>>> +	    break;
>>> +	  }
>>
>> ...is the same as this chunk.  Plus, all this mode composite stuff is
> 
> It is not the same, there is the DFmode, tmp vs. mode, result difference.
> But sure, we could either add an inline function which for
> (code, mode, result, inf) set of options (or (code, DFmode, tmp, inf))
> do those 0, 1, 2, 3 frange_nextafter calls (and return bool if it did any
> - there is also the if (!inexact) return; case), or as you suggest
> perhaps change frange_nextafter to handle MODE_COMPOSITE_P differently
> and do there
>    if (mode_composite && (real_isdenormal (&result, mode) || real_iszero (&result)))
>      {
>        // IBM extended denormals only have DFmode precision.
>        REAL_VALUE_TYPE tmp;
>        real_convert (&tmp, DFmode, &result);
>        frange_nextafter (DFmode, tmp, inf);
>        real_convert (&result, mode, &tmp);
>      }
>    else
>      frange_nextafter (mode, result, inf);
> Though, that somewhat changes behavior, it will convert to DFmode and back
> for every nextafter rather than just once (just slower compile time),
> but also right now we start from value rather than result.
> 
> So, perhaps a combination of that, change frange_nextafter to do the above
> and change frange_arithmetic for the initial inexact rounding only to
> do it by hand using range_nextafter and starting from value.

Either way is fine.  Whatever is cleaner.

Aldy

> 
> Anyway, this patch is far less important than the previous one...
> 
> 	Jakub
>