Re: Code handling the mod intrinsic seems broken

[Tobias Schlüter <tobias dot schlueter at physik dot uni-muenchen dot de>]

François-Xavier Coudert wrote:
> Hi all,
> 
> The code handling the mod intrinsic for real types (mod(x,y) where x
> and y are reals) seems ill-designed. In gfc_conv_intrinsic_mod(), we
> have the following code:
> 
>     case BT_REAL:
>       /* Real values we have to do the hard way.  */
>       arg = gfc_evaluate_now (arg, &se->pre);
>       arg2 = gfc_evaluate_now (arg2, &se->pre);
> 
>       tmp = build2 (RDIV_EXPR, type, arg, arg2);
>       /* Test if the value is too large to handle sensibly.  */
>       gfc_set_model_kind (expr->ts.kind);
>       mpfr_init (huge);
>       n = gfc_validate_kind (BT_INTEGER, expr->ts.kind, false);
> 
> I don't fully understand the workings of this, but it seems to rely on
> the fact that for every real kind (here, expr->ts.kind), there is
> integer kind available (since we use this real kind in a call to
> gfc_validate_kind with argument BT_INTEGER).
> 
> This assumption is not true; on i686-linux, e.g., we have real(10) and
> no integer(10). This is indeed blocking me from extending support for
> large real kinds (I'm now working on intrinsics and specifics), so I'd
> like to know if someone knows how to deal with this.

It's been a while since I touched this code, but if I understand it correctly,
the correct integer type is the largest INTEGER.  The evaluation goes along
the lines given in the comment in front of the function.  The peculiar thing
about this code is that the gcc backend doesn't implement all kinds of
rounding, and the code we have in place for this (see build_fix_expr) only
works if the result types are integers, those can of course overflow.  I'm not
convinced that the logic in gfc_conv_intrinsic_mod is correct for the case of
a large intermediate value, I'll have to think about this a little more.  A
way to fix this while doing away with the need for an intermediate integer
would probably be to insert the correct type conversions in build_fix_expr's
helpers so that they can also return floating point results.

- Tobi