Re: [PATCH] gimple-range-op: Improve handling of sqrt ranges

[Aldy Hernandez <aldyh@redhat.com>]

On Fri, May 5, 2023 at 11:14 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, May 05, 2023 at 11:06:31AM +0200, Aldy Hernandez wrote:
> > > +/* Compute FUNC (ARG) where FUNC is a mpfr function.  If RES_LOW is non-NULL,
> > > +   set it to low bound of possible range if the function is expected to have
> > > +   ULPS precision and similarly if RES_HIGH is non-NULL, set it to high bound.
> > > +   If the function returns false, the results weren't set.  */
> > > +
> > > +static bool
> > > +frange_mpfr_arg1 (REAL_VALUE_TYPE *res_low, REAL_VALUE_TYPE *res_high,
> > > +             int (*func) (mpfr_ptr, mpfr_srcptr, mpfr_rnd_t),
> > > +             const REAL_VALUE_TYPE &arg, tree type, unsigned ulps)
> > > +{
> >
> > Since you're returning a range of sorts [low, high], would it be cleaner to
> > return an frange, or is always calculating low/high too expensive?  I notice
> > you avoid it when passing NULL.
>
> The point was that the caller can tell which bound it needs, low, high or
> both and we don't waste time calculating ones we don't need (especially with
> larger values of ulps).  E.g. for the sqrt case we only need one of them,
> but when I thought about the sin/cos case, I'll probably need both and
> calling the function twice would mean repeating the even more expensive mpfr
> call.
>
> > Would you mind adding a typedef for the (*func) callback above?  I always
> > find C callbacks a pain to read.
>
> I can, what I used comes from elsewhere (builtins.cc/fold-const-call.cc
> which use it like that).

It would be my preference to have a typedef in
builtins.cc/fold-const-call.cc as well, as we could clean everything
up.  But I defer to you as a global maintainer, whether we want to do
that or not.  If not, then don't bother with just cleaning up
gimple-range-op.cc.

LGTM.

Aldy