[Bug fortran/49010] Result of MOD and MODULO intrinsic has wrong sign

[jb at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49010

--- Comment #11 from Janne Blomqvist <jb at gcc dot gnu.org> 2011-05-17 16:18:41 UTC ---
(In reply to comment #10)
> On Tue, May 17, 2011 at 02:17:22PM +0000, jb at gcc dot gnu.org wrote:
> >
> > So does the fallback path actually ever get used? AFAICS the builtins are
> > always available, and if the builtin results in a call to fmod{f,,l,Q} we have
> > fallback implementations in c99_functions.c for non-C99 functions.  See PR
> > 29810.
> > 
> 
> I don't know all the targets on which gfortran can run.  Of course,
> if all target have the builtin, then the fallback code won't be used.
> We can garbage collect the code if you're confident that it is unneeded.

I'm fairly confident. AFAICS the idea is that the builtins are always
available, but if the compiler cannot optimize it in some meaningful way (e.g.
constant folding) then a call to the corresponding library function is emitted,
and this is the situation where things can go wrong as not all targets provide
a C99 libm. However, as we provide fallbacks in c99_functions.c this case is
covered.

This is, in short, the story behind PR 29810; when we started using
BUILT_IN_FMOD{F,,L} the float and long double builtins were available also on
targets where fmod{f,l} were not.

> > Taken together, it seems the proper approach would be to just remove the
> > fallback path (not really related to this PR per se, just as a general
> > janitorial patch), and fix the result if it's negative zero. E.g. something
> > like the following pseudocode for MOD(a,p):
> > 
> > if (!options.fast_math && fabs(p) == 0.0)
> >   generate_error(...) // TBH, aborting the program here
> 
> The restriction that "P shall not be zero" is on the user
> not the compiler, and the compiler is not required to 
> diagnosis that problem.

Ah, good point.

>  I think we can simply issue a
> warning and let the builtin set res = NaN. 

Well, I was thinking of not testing whether p == 0 at all; As the NaN will
presumably propagate the user will find out that something went wrong quickly
enough (if the user bothers to check the output, that is! :) ), and finding the
place where the NaN occurred is as simple as recompiling with
-ffpe-trap=invalid.

> > res = __builtin_fmod (a, p)
> > if (options.signed_zeros)
> > {
> >   if (res == -0.0)
> >     res = 0.0
> > }
> 
> I don't have n1256.pdf handy (draft of C standard), but
> IIRC, this check is expensive because zero and negative zero
> compare equal.  One needs to explicitly check the sign
> bit or simply do
> 
>    if (res == 0.0) res = abs(res)
> 
> to clear the sign.

Ah, so it seems.