Re: [PATCH] Implement range-op entry for sin/cos.

[Aldy Hernandez <aldyh@redhat.com>]

On 4/21/23 08:52, Jakub Jelinek wrote:
> On Thu, Apr 20, 2023 at 09:14:10PM -0400, Siddhesh Poyarekar wrote:
>> On 2023-04-20 13:57, Siddhesh Poyarekar wrote:
>>> For bounds that aren't representable, one could get error bounds from
>>> libm-test-ulps data in glibc, although I reckon those won't be
>>> exhaustive.  From a quick peek at the sin/cos data, the arc target seems
>>> to be among the worst performers at about 7ulps, although if you include
>>> the complex routines we get close to 13 ulps.  The very worst
>>> imprecision among all math routines (that's gamma) is at 16 ulps for
>>> power in glibc tests, so maybe allowing about 25-30 ulps error in bounds
>>> might work across the board.
>>
>> I was thinking about this a bit more and it seems like limiting ranges to
>> targets that can generate sane results (i.e. error bounds within, say, 5-6
>> ulps) and for the rest, avoid emitting the ranges altogether. Emitting a bad
>> range for all architectures seems like a net worse solution again.
> 
> Well, at least for basic arithmetics when libm functions aren't involved,
> there is no point in disabling ranges altogether.
> And, for libm functions, my plan was to introduce a target hook, which
> would have combined_fn argument to tell which function is queried,
> machine_mode to say which floating point format and perhaps a bool whether
> it is ulps for these basic math boundaries or results somewhere in between,
> and would return in unsigned int ulps, 0 for 0.5ulps precision.

I wonder if we could export frange_nextafter() to take a final argument 
for the number of ulps, making it possible to extend in either direction 
by a number of ulps.  And perhaps add a generic function that calls the 
target hook and extends the range accordingly:

extern int TARGET_ULP_ERROR (combined_fn, machine_mode);
extern void frange_nextafter (machine_mode,
			      REAL_VALUE_TYPE &value,
			      const REAL_VALUE_TYPE &direction,
			      int ulps);
void
adjust_frange_for_op (frange &r, combined_fn fn)
{
   ...
   int ulps = TARGET_ULP_ERROR (fn, mode);
   frange_nextafter (mode, lb, frange_val_min (type), ulps);
   frange_nextafter (mode, ub, frange_val_max (type), ulps);
   ...
}

bool
cfn_sincos::fold_range (...)
{
...
  if (cond)
     {
       r.set (type, dconstm1, const1);
       adjust_frange_for_op (r, m_cfn);
     }
...
}

Or if adjusting by ULPS is a common enough idiom, we could promote it to 
an frange method:

	void frange::adjust_ulps (int ulps);

Would that work, or did you have something else in mind?

Aldy