Re: [fortran,patch] Inline intrinsics: EXPONENT, FRACTION, NEAREST, RRSPACING, SET_EXPONENT and SPACING

[Janne Blomqvist <blomqvist dot janne at gmail dot com>]

FX Coudert wrote:
> > 1. In libgfortran/nearest, there is a glibc bug workaround:
> >
> > GFC_REAL_4
> > nearest_r4 (GFC_REAL_4 s, GFC_REAL_4 dir)
> > {
> >   dir = copysignf (__builtin_inff (), dir);
> >   if (     != 0)
> >     {
> >       /* ??? Work around glibc bug on x86.  */
> >       volatile GFC_REAL_4 r = nextafterf (s, dir);
> >       return r;
> >     }
> >   else
> >     return nextafterf (s, dir);
> > }
> >
> > Has this been fixed in glibc, and even if it has, is the fix recent
> > enough that we want to support older glibc versions as well? Or perhaps
> > more to the point, does anyone on glibc/x86 actually compile libgfortran
> > with FLT_EVAL_METHOD != 0, and if so, why?
>
> FLT_EVAL_METHOD is a GCC builtin, which is always 0 except:
>   -- on i386 or x86_64, it is (TARGET_MIX_SSE_I387 ? -1 : 
> TARGET_SSE_MATH ? 0 : 2)
>   -- on m68k, it is ((TARGET_68040 || ! TARGET_68881) ? 0 : 2)
>
> So, it means we should see it failing on I386 without sse math, if it is 
> to fail.
>
> The code in libgfortran came straight from tree-ssa, where it was 
> written by rth. Before that, the code was emitted from the front-end, 
> and it was ugly 
> (http://gcc.gnu.org/viewcvs/branches/tree-ssa-20020619-branch/gcc/fortran/trans-intrinsic.c?r1=73974&r2=73973&pathrev=73974). 
> Google apparently has no idea what this bug was, so I'm tempted not to 
> care. After all, there are other glibc (or libc) bugs around that we 
> don't work around.
> Of course, I can also add it to the generated code, the same way it was 
> done in the library; but, now that we inline it, it's gonna be a serious 
> pessimization to have a register variable in the middle.

Fair enough. I suppose we'll hear about if it still affects anyone, in 
which case a separate patch can add back the workaround.

> > 2. You use both ldexp and scalbn. AFAICS scalbn does not provide any
> > added value since in all cases FLT_RADIX is 2 which makes it equivalent
> > to ldexp (if we ever want to support non-binary FP there are lots of
> > other changes that have to be made as well).
>
> I simply chose to do what was already done in the library. Plus, just 
> because there are some assumptions about FLT_RADIX in the current 
> codebase (not thay many, I think) doesn't mean we have to add more :)
>
> PS: if I get some free time this summer (read: be unemployed), I plan to 
> play with decimal numbers in gfortran.
>
> > As for which one should be
> > chosen, ldexp is C89, ldexpf, ldexpl and scalbn* are C99, so I don't
> > think it makes much difference from a portability standpoint. I just
> > think it's not that useful to have both.
>
>
> We already have a scalbn() fallback in libgfortran, so it truly doesn't 
> make any difference.

I think that settles it then. Patch is ok, provided you replace all 
occurences of ldexp* with scalbn*; should be slightly more portable 
since we have fallbacks (we don't have any ldexp* fallbacks), and it 
might make future decimal FP support slightly easier since scalbn* can 
work with any FLT_RADIX, whereas ldexp* is base-2 only.


--
Janne Blomqvist