Message ID | 20190105040702.GA98056@troutmask.apl.washington.edu |
---|---|
State | New |
Headers | show |
Series | PR fortran/69101 -- IEEE_SELECTED_REAL_KIND part I | expand |
Hi Steve, thanks for taking this on! Maybe it would be better to put the checking and argument reordering into its own routine (something like gfc_check_minloc_maxloc in check.c) so all three arguments would be present, with an expression possibly containing NULL, for the later routines. This could add some clarity and save code duplication later, when the second part of the patch is done. However, I do not feel strongly about that. If you feel that your current approach is better, that is fine by me. Regards Thomas
On Sat, Jan 05, 2019 at 01:23:14PM +0100, Thomas Koenig wrote: > > Maybe it would be better to put the checking and argument reordering > into its own routine (something like gfc_check_minloc_maxloc in > check.c) so all three arguments would be present, with an > expression possibly containing NULL, for the later routines. > This could add some clarity and save code duplication later, > when the second part of the patch is done. > > However, I do not feel strongly about that. If you feel that your > current approach is better, that is fine by me. > Thanks, Thomas. I tried a few things and finally landed on the keyword vs positional handling and argument checking in simplify_ieee_selected_real_kind. The actual heavy lifting comes from gfc_simplify_selected_real_kind where the constant expressions are extracted and gfortran tries to determine a valid kind. I see if I can moving the keyword/positional and checking into a separate function. Long term I think we need to put all functions into the table of intrinsic subprograms, so that handling these functions is almost identical to sin(), cos(), etc. This would then give the keyword vs positional handling automatically. ieee module procedures would be marked in gfc_intrinsic_sym with a bool or a bit in a bitfield, ie, gfc_intrinsic_sym *isym; isym->ieee = true; ! Member of ieee_arthmetic. or isym->ieee = 1; ! Member of ieee_arithmetic. 'isym->ieee = true' then indicates that gfortran needs to check if the ieee_arithmetic has been used and is in scope when a procedure is seen in the Fortran code. If yes, we have the module procedure. If no, we have a possibly user-defined procedure that shadows a routine from an intrinsic module.
On Sat, Jan 05, 2019 at 09:22:34AM -0800, Steve Kargl wrote: > On Sat, Jan 05, 2019 at 01:23:14PM +0100, Thomas Koenig wrote: > > > > Maybe it would be better to put the checking and argument reordering > > into its own routine (something like gfc_check_minloc_maxloc in > > check.c) so all three arguments would be present, with an > > expression possibly containing NULL, for the later routines. > > This could add some clarity and save code duplication later, > > when the second part of the patch is done. > > > > However, I do not feel strongly about that. If you feel that your > > current approach is better, that is fine by me. > > > > Thanks, Thomas. I tried a few things and finally landed on > the keyword vs positional handling and argument checking in > simplify_ieee_selected_real_kind. The actual heavy lifting > comes from gfc_simplify_selected_real_kind where the constant > expressions are extracted and gfortran tries to determine a > valid kind. I see if I can moving the keyword/positional > and checking into a separate function. > Well, that was quick. Moving code around is problematic. The hijacking of the interface checking causes gfortran to see from 1 to 3 actual arguments. A simple example is ISRK(r=3.14) (yes, an invalid call). This has exact 1 actual argument, namely, arg=expr->value.function.actual. So, gfortran see arg->next=NULL and arg->next->next obvious cannot exist. The re-ordering would require allocation of next and next->next to get the number of actual arguments to match P,R,RADIX, so that we can extract pointers to the expressions. Not worth the effort at the moment. Of I might change my mind when I try to tackle the hijacking of function rknd(n) result r use ieee_arithmetic integer n r = ieee_selected_real_kind(p=n) end function rknd
Hi Steve,
> Well, that was quick. Moving code around is problematic.
Thanks for checking. The patch is OK for trunk.
Regards
Thomas
On Tue, Jan 08, 2019 at 10:27:25PM +0100, Thomas Koenig wrote: > Hi Steve, > > > Well, that was quick. Moving code around is problematic. > > Thanks for checking. The patch is OK for trunk. > Thanks. I decided to see if long term approach would work. It almost does. The attached patch put ieee_selected_real_kind into the table of intrinsic functions. It automatically gets us a generic routine with argument chekcing and simplification. This just works: program foo use ieee_arithmetic, only : ieee_selected_real_kind integer, parameter :: n = ieee_selected_real_kind(6_2) i = 6 print *, n, ieee_selected_real_kind(6_8), ieee_selected_real_kind(i) end program foo Now, the downside. I can't finesse rename on USE. This does not work, and I'm stuck at the moment. subroutine bar use ieee_arithmetic, only : isrk => ieee_selected_real_kind integer, parameter :: n = isrk(6) i = 6 print *, n, isrk(6), isrk(i) end subroutine bar If anyone has an idea, I would be quite happy to hear about it.
On Sun, Jan 20, 2019 at 05:04:24PM -0800, Steve Kargl wrote: > On Tue, Jan 08, 2019 at 10:27:25PM +0100, Thomas Koenig wrote: > > Hi Steve, > > > > > Well, that was quick. Moving code around is problematic. > > > > Thanks for checking. The patch is OK for trunk. > > > > Thanks. I decided to see if long term approach would work. > It almost does. The attached patch put ieee_selected_real_kind > into the table of intrinsic functions. It automatically gets > us a generic routine with argument chekcing and simplification. > This just works: > > program foo > use ieee_arithmetic, only : ieee_selected_real_kind > integer, parameter :: n = ieee_selected_real_kind(6_2) > i = 6 > print *, n, ieee_selected_real_kind(6_8), ieee_selected_real_kind(i) > end program foo > > Now, the downside. I can't finesse rename on USE. This does not > work, and I'm stuck at the moment. > > subroutine bar > use ieee_arithmetic, only : isrk => ieee_selected_real_kind > integer, parameter :: n = isrk(6) > i = 6 > print *, n, isrk(6), isrk(i) > end subroutine bar > > If anyone has an idea, I would be quite happy to hear about it. > Well, I found a way to at least deal with the renaming issue. It is ugly, and won't be pursued.
Index: gcc/fortran/expr.c =================================================================== --- gcc/fortran/expr.c (revision 267591) +++ gcc/fortran/expr.c (working copy) @@ -2768,11 +2768,20 @@ gfc_check_init_expr (gfc_expr *e) mod = sym->generic->sym->from_intmod; if (mod == INTMOD_IEEE_ARITHMETIC || mod == INTMOD_IEEE_EXCEPTIONS) { + int ecnt; + gfc_expr *new_expr = gfc_simplify_ieee_functions (e); if (new_expr) { gfc_replace_expr (e, new_expr); t = true; + break; + } + gfc_get_errors (NULL, &ecnt); + if (ecnt > 0) + { + t = false; + gfc_clear_error (); break; } } Index: gcc/fortran/simplify.c =================================================================== --- gcc/fortran/simplify.c (revision 267591) +++ gcc/fortran/simplify.c (working copy) @@ -8521,25 +8521,129 @@ gfc_simplify_compiler_version (void) /* Simplification routines for intrinsics of IEEE modules. */ +/* IEEE_SELECTED_REAL_KIND is generic, but cannot be specified in the + intrinsic module ieee_arithmetic.mod due to ambiguous interfaces. When + it appears in an initialization expression, the checking must be done + here. */ + gfc_expr * simplify_ieee_selected_real_kind (gfc_expr *expr) { - gfc_actual_arglist *arg; - gfc_expr *p = NULL, *q = NULL, *rdx = NULL; + gfc_actual_arglist *arg0, *arg1, *arg2; + gfc_expr *p, *r, *rdx; - arg = expr->value.function.actual; - p = arg->expr; - if (arg->next) + arg1 = arg2 = NULL; + p = r = rdx = NULL; + + arg0 = expr->value.function.actual; + if (arg0->next) + { + arg1 = arg0->next; + if (arg1->next) arg2 = arg1->next; + } + + if (!arg0->expr && arg1 && !arg1->expr && arg2 && !arg2->expr) + { + gfc_error_now ("At least one of P, R, or RADIX must be present in " + "IEEE_SELECTED_REAL_KIND at %C"); + gfc_clear_error (); + return NULL; + } + + /* Look at first argument. */ + if (!arg0->name || strcmp(arg0->name, "p") == 0) + p = arg0->expr; + else if (strcmp(arg0->name, "r") == 0) + r = arg0->expr; + else if (strcmp(arg0->name, "radix") == 0) + rdx = arg0->expr; + else + goto invalid; + + /* Look at second argument, if it exists. */ + if (arg1) { - q = arg->next->expr; - if (arg->next->next) - rdx = arg->next->next->expr; + if (!arg1->name || strcmp(arg1->name, "r") == 0) + { + if (r) + return NULL; + r = arg1->expr; + } + else if (strcmp(arg1->name, "p") == 0) + { + if (p) + return NULL; + p = arg1->expr; + } + else if (strcmp(arg1->name, "radix") == 0) + { + if (rdx) + return NULL; + rdx = arg1->expr; + } + else + goto invalid; } - /* Currently, if IEEE is supported and this module is built, it means - all our floating-point types conform to IEEE. Hence, we simply handle - IEEE_SELECTED_REAL_KIND like SELECTED_REAL_KIND. */ - return gfc_simplify_selected_real_kind (p, q, rdx); + /* Look at third argument, if it exists. */ + if (arg2) + { + if (!arg2->name || strcmp(arg2->name, "radix") == 0) + { + if (rdx) + return NULL; + rdx = arg2->expr; + } + else if (strcmp(arg2->name, "p") == 0) + { + if (p) + return NULL; + p = arg2->expr; + } + else if (strcmp(arg2->name, "r") == 0) + { + if (r) + return NULL; + r = arg2->expr; + } + else + goto invalid; + } + + /* Check for scalar integer expressions. */ + if (p && (p->ts.type != BT_INTEGER || p->rank != 0)) + goto mismatch; + + if (r && (r->ts.type != BT_INTEGER || r->rank != 0)) + goto mismatch; + + if (rdx) + { + if (GFC_STD_F2008 & ~gfc_option.allow_std) + { + gfc_error_now ("Fortran 2008: RADIX argument specified in " + "IEEE_SELECTED_REAL_KIND at %C"); + return NULL; + } + + if (rdx->ts.type != BT_INTEGER || rdx->rank != 0) + goto mismatch; + } + + return gfc_simplify_selected_real_kind (p, r, rdx); + +mismatch: + + gfc_error_now ("Scalar integer argument expected in " + "IEEE_SELECTED_REAL_KIND at %C"); + return NULL; + +invalid: + + /* This should be unreachable. */ + gfc_fatal_error ("Invalid uses of IEEE_SELECTED_REAL_KIND at %C"); + return NULL; + } gfc_expr * Index: gcc/testsuite/gfortran.dg/ieee/ieee_7.f90 =================================================================== --- gcc/testsuite/gfortran.dg/ieee/ieee_7.f90 (revision 267591) +++ gcc/testsuite/gfortran.dg/ieee/ieee_7.f90 (working copy) @@ -11,7 +11,6 @@ ! Test IEEE_SELECTED_REAL_KIND in specification expressions - integer(kind=ieee_selected_real_kind()) :: i1 integer(kind=ieee_selected_real_kind(10)) :: i2 integer(kind=ieee_selected_real_kind(10,10)) :: i3 integer(kind=ieee_selected_real_kind(10,10,2)) :: i4 @@ -19,7 +18,6 @@ ! Test IEEE_SELECTED_REAL_KIND if (ieee_support_datatype(0.)) then - if (ieee_selected_real_kind() /= kind(0.)) STOP 1 if (ieee_selected_real_kind(0) /= kind(0.)) STOP 2 if (ieee_selected_real_kind(0,0) /= kind(0.)) STOP 3 if (ieee_selected_real_kind(0,0,2) /= kind(0.)) STOP 4 Index: gcc/testsuite/gfortran.dg/ieee/pr69101_1.f90 =================================================================== --- gcc/testsuite/gfortran.dg/ieee/pr69101_1.f90 (nonexistent) +++ gcc/testsuite/gfortran.dg/ieee/pr69101_1.f90 (working copy) @@ -0,0 +1,10 @@ +! { dg-do compile } +program foo + use ieee_arithmetic + implicit none + integer, parameter :: n(3) = [ 1, 2, 3] + integer, parameter :: i1 = ieee_selected_real_kind() ! { dg-error "must be present in" } + integer, parameter :: j2 = ieee_selected_real_kind(p=6,p=7) ! { dg-error "has already appeared in" } + integer, parameter :: j3 = ieee_selected_real_kind(r=7.) ! { dg-error "Scalar integer argument" } + integer, parameter :: j4 = ieee_selected_real_kind(n) ! { dg-error "Scalar integer argument" } +end program foo Index: gcc/testsuite/gfortran.dg/ieee/pr69101_2.f90 =================================================================== --- gcc/testsuite/gfortran.dg/ieee/pr69101_2.f90 (nonexistent) +++ gcc/testsuite/gfortran.dg/ieee/pr69101_2.f90 (working copy) @@ -0,0 +1,7 @@ +! { dg-do compile } +! { dg-additional-options "-std=f2003" } +program foo + use ieee_arithmetic + implicit none + integer, parameter :: jr = ieee_selected_real_kind(radix=2) ! { dg-error "RADIX argument" } +end program foo