Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
Hello world, I found an ommission in primary.c which prevented issuing a more specific error instead of "syntax error" for the case when a function was declared in an EXTERNAL statement, and I have now gone for the "Unexpected junk after foo" variant. Regression-tested. OK for trunk? Regards Thomas 2020-01-18 Thomas König PR fortran/44960 * primary.c (gfc_match_rvalue): Break after setting MATCH_ERROR. * resolve.c (resolve_function): Issue error when a function call contains a reference. 2020-01-18 Thomas König PR fortran/44960 * gfortran.dg/function_reference_1.f90: New test. diff --git a/gcc/fortran/primary.c b/gcc/fortran/primary.c index 07b8ac08ba2..bd50827bb15 100644 --- a/gcc/fortran/primary.c +++ b/gcc/fortran/primary.c @@ -3661,6 +3661,7 @@ gfc_match_rvalue (gfc_expr **result) gfc_error ("The leftmost part-ref in a data-ref cannot be a " "function reference at %C"); m = MATCH_ERROR; + break; } m = MATCH_YES; diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index 6f2a4c4d65a..697afadb378 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -3129,6 +3129,13 @@ resolve_function (gfc_expr *expr) || sym->intmod_sym_id == GFC_ISYM_CAF_SEND)) return true; + if (expr->ref) +{ + gfc_error ("Unexpected junk after %qs at %L", expr->symtree->n.sym->name, + &expr->where); + return false; +} + if (sym && sym->attr.intrinsic && !gfc_resolve_intrinsic (sym, &expr->where)) return false; diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90 new file mode 100644 index 000..be634c9dd4b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR 44960 - this was erroneusly accepted. +! Original test case by Daniel Franke. + +type t + integer :: a +end type t +type(t) :: foo +print *, foo(1)%a ! { dg-error "Unexpected junk" } +end + diff --git a/gcc/testsuite/gfortran.dg/function_reference_2.f90 b/gcc/testsuite/gfortran.dg/function_reference_2.f90 new file mode 100644 index 000..375c58bb6d2 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/function_reference_2.f90 @@ -0,0 +1,10 @@ +! { dg-do compile } +! PR 44960 - improve the error message +program main + type t + integer :: a +end type t +type(t) :: foo +external foo +i = foo(1)%1 ! { dg-error "leftmost part-ref in a data-ref cannot be a function reference" } +end ! { dg-do compile } ! PR 44960 - improve the error message program main type t integer :: a end type t type(t) :: foo external foo i = foo(1)%1 ! { dg-error "leftmost part-ref in a data-ref cannot be a function reference" } end ! { dg-do compile } ! PR 44960 - this was erroneusly accepted. ! Original test case by Daniel Franke. type t integer :: a end type t type(t) :: foo print *, foo(1)%a ! { dg-error "Unexpected junk" } end
Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
Am 18.01.20 um 12:44 schrieb Tobias Burnus: function_reference_1.f90:9:8: 9 | print *, foo(1)%a ! { dg-error "Syntax error" } | 1 Error: Syntax error in expression at (1) The error message is not helpful at all. Well, yes. It is no better than what we currently have for the slightly different test case type t integer :: a end type t type(t) :: foo external foo print *, foo(1)%a end which is a.f90:6:16: 6 | print *, foo(1)%a |1 Error: Syntax error in PRINT statement at (1) (but at least that points to the right place). I can see if I can also replace this with something more useful (have to find the place where this is issued first, though). Regards Thomas
Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
Am 17.01.20 um 15:42 schrieb Steve Kargl: Gfortran probably should not try to guess what the user thought s/he wanted. The generic "Syntax error" would seem to apply here. To me, foo(1)%a looks much more like an array reference rather than a function reference. OK, so here's a patch which does just that. The error message low looks like function_reference_1.f90:9:8: 9 | print *, foo(1)%a ! { dg-error "Syntax error" } |1 Error: Syntax error in expression at (1) The location information is a bit off, but in the absence of location information for the reference (which we do not collect), I think this is the best I can do. So, OK for trunk (with the old ChangeLog)? Regards Thomas diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index 6f2a4c4d65a..a846677b770 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -3129,6 +3129,12 @@ resolve_function (gfc_expr *expr) || sym->intmod_sym_id == GFC_ISYM_CAF_SEND)) return true; + if (expr->ref) +{ + gfc_error ("Syntax error in expression at %L", &expr->where); + return false; +} + if (sym && sym->attr.intrinsic && !gfc_resolve_intrinsic (sym, &expr->where)) return false; diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90 new file mode 100644 index 000..1b7f4809c5c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR 44960 - this was erroneusly accepted. +! Original test case by Daniel Franke. + +type t + integer :: a +end type t +type(t) :: foo +print *, foo(1)%a ! { dg-error "Syntax error" } +end + ! { dg-do compile } ! PR 44960 - this was erroneusly accepted. ! Original test case by Daniel Franke. type t integer :: a end type t type(t) :: foo print *, foo(1)%a ! { dg-error "Syntax error" } end
[patch, fortran] Fix PR 44960, accepts invalid reference on function
Hello world, here is my first patch made from the git world. It certainly took enough time to work out how to to this, but I think I have it figured out now... Anyway, the fix is rather straightforward. We cannot have a reference on a function. If this slipped through on parsing, let's issue the error during resolution. Regression-tested. OK for trunk? Regards Thomas 2020-01-16 Thomas König PR fortran/44960 * resolve.c (resolve_function): Issue error when a function call contains a reference. 2020-01-16 Thomas König PR fortran/44960 * gfortran.dg/function_reference_1.f90: New test. diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index 6f2a4c4d65a..1525c00ea4c 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -3129,6 +3129,13 @@ resolve_function (gfc_expr *expr) || sym->intmod_sym_id == GFC_ISYM_CAF_SEND)) return true; + if (expr->ref) +{ + gfc_error ("Function call can not contain a reference at %L", + &expr->where); + return false; +} + if (sym && sym->attr.intrinsic && !gfc_resolve_intrinsic (sym, &expr->where)) return false; diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90 new file mode 100644 index 000..78c92a6f20d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR 44960 - this was erroneusly accepted. +! Original test case by Daniel Franke. + +type t + integer :: a +end type t +type(t) :: foo +print *, foo(1)%a ! { dg-error "Function call can not contain a reference" } +end + ! { dg-do compile } ! PR 44960 - this was erroneusly accepted. ! Original test case by Daniel Franke. type t integer :: a end type t type(t) :: foo print *, foo(1)%a ! { dg-error "Function call can not contain a reference" } end
Re: [patch, fortran] Fix PR 65428, ICE on nested empty array constructors
Hi Tobias, I am not completely happy about the introduction of yet another two global variables, but I also do not see an easy way out. Hence: OK. Actually, I wasn't too happy myself, but, like you, I didn't find anything better. I was playing around with the following test case – you might consider to add them as well. (I would exclude the error item, however.) I have also added this as a test case. Committed as r280063 (without the spaces at the end of the lines that you pointed out in a separate mail). Thanks for the review! Regards Thomas
*ping* [patch, fortran] Fix PR 65428, ICE on nested empty array constructors
Am 02.01.20 um 23:35 schrieb Thomas Koenig: Hello world, the attached patch fixes an ICE where an array constructor containing an empty array constructor with a type didn't get the type from the inner constructor. The solution is to stash the type away in yet another variable and only use it if the constructor turns out to be empty, and the type has not been set some other way. Regression-tested. OK for trunk? Ping?
Fortran patches to be reviewed (was: [Patch, Fortran] PR91640 – Fix call to contiguous dummy)
Hi Tobias, PS: I lost a bit the overview. Is there any patch pending review or otherwise pending? From my side, there is the patch for PR 65428, https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00040.html Apart from that, I don't see any outstanding patches. Regards Thomas
Re: [Patch, Fortran] PR 92994 – add more ASSOCIATE checks
Hi Tobias, Build on x86-64-gnu-linux. OK for the trunk? Looks good. Thanks for the patch! Regards Thomas
[patch, fortran] Fix PR 65428, ICE on nested empty array constructors
Hello world, the attached patch fixes an ICE where an array constructor containing an empty array constructor with a type didn't get the type from the inner constructor. The solution is to stash the type away in yet another variable and only use it if the constructor turns out to be empty, and the type has not been set some other way. Regression-tested. OK for trunk? Regards Thomas Save typespec for empty array constructor. 2020-01-02 Thomas Koenig PR fortran/65428 * array.c (empty_constructor): New variable. (empty_ts): New variable. (expand_constructor): Save typespec in empty_ts. Unset empty_constructor if there is an element. (gfc_expand_constructor): Initialize empty_constructor and empty_ts. If there was no explicit constructor type and the constructor is empty, take the type from empty_ts. 2020-01-02 Thomas Koenig PR fortran/65428 * gfortran.dg/zero_sized_11.f90: New test. Index: array.c === --- array.c (Revision 279821) +++ array.c (Arbeitskopie) @@ -1759,7 +1759,12 @@ cleanup: return t; } +/* Variables for noticing if all constructors are empty, and + if any of them had a type. */ +static bool empty_constructor; +static gfc_typespec empty_ts; + /* Expand a constructor into constant constructors without any iterators, calling the work function for each of the expanded expressions. The work function needs to either save or free the @@ -1782,6 +1787,9 @@ expand_constructor (gfc_constructor_base base) e = c->expr; + if (empty_constructor) + empty_ts = e->ts; + if (e->expr_type == EXPR_ARRAY) { if (!expand_constructor (e->value.constructor)) @@ -1790,6 +1798,7 @@ expand_constructor (gfc_constructor_base base) continue; } + empty_constructor = false; e = gfc_copy_expr (e); if (!gfc_simplify_expr (e, 1)) { @@ -1873,6 +1882,8 @@ gfc_expand_constructor (gfc_expr *e, bool fatal) iter_stack = NULL; + empty_constructor = true; + gfc_clear_ts (&empty_ts); current_expand.expand_work_function = expand; if (!expand_constructor (e->value.constructor)) @@ -1882,6 +1893,13 @@ gfc_expand_constructor (gfc_expr *e, bool fatal) goto done; } + /* If we don't have an explicit constructor type, and there + were only empty constructors, then take the type from + them. */ + + if (constructor_ts.type == BT_UNKNOWN && empty_constructor) +e->ts = empty_ts; + gfc_constructor_free (e->value.constructor); e->value.constructor = current_expand.base; ! { dg-do compile } ! PR 65428 - this used to ICE. Original test case by FX Coudert. program p integer :: i print *, [shape(1)] print *, [[ integer :: ]] print *, (/ (/ (i, i=1,0) /) /) end
[patch, fortran, committed] Fix dependency for %re and %im
Hello world, New year, new bug, new patch :-) I have just committed as obvious and simple the attached patch as r279821, where we failed to account for %re and %im in dependency checking. This is a 10 regression, gcc 9 works. Regards Thomas Handle REF_INQUIRY for dependency checking. 2020-01-01 Thomas Koenig PR fortran/93113 * dependency.c (gfc_dep_resolver): Handle REF_INQUIRY in switch for ref types. 2020-01-01 Thomas Koenig PR fortran/93113 * gfortran.dg/dependency_58.f90: New test. Index: dependency.c === --- dependency.c (Revision 279765) +++ dependency.c (Arbeitskopie) @@ -2286,6 +2286,12 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf subsequent references also overlap. */ break; + case REF_INQUIRY: + if (lref->u.i != rref->u.i) + return 0; + + break; + default: gcc_unreachable (); } ! { dg-do run } ! { dg-additional-options "-ffrontend-optimize -Warray-temporaries" } ! PR 93113 - this used to ICE, and should not generate a temporary. program main integer, parameter :: n = 10 complex, dimension(n,n) :: a, b, c real, dimension(n,n) :: r call random_number (r) c%re = r call random_number (r) c%im = r a = c b = c b%re = a%re - 0.5 b%im = a%im - 0.5 a%re = a%re - 0.5 a%im = a%im - 0.5 if (any (a /= b)) stop 1 b%im = a%re a%im = a%re if (any (a /= b)) stop 2 a = c b = c b(2:n,:)%re = a(1:n-1,:)%re a(2:n,:)%re = a(1:n-1,:)%re if (any (a /= b)) stop 3 a = c b = c b(1:n-1,:)%im = a(2:,:)%im a(1:n-1,:)%im = a(2:,:)%im if (any (a /= b)) stop 3 end program main
Re: [patch, libfortran] Fortran 2018: Support d0.d, e0.d, es0.d, en0.d, g0.d and ew.d e0 edit descriptors
Hi Jerry, OK for trunk? Looks good. I also think that your approach that DEC stuff should be checked later is good. If it passes the testsuite, that's good enough for a commit. Thanks for the patch! Regards Thomas
*ping*[patch, fortran] Fix PR 91541, ICE on valid for INDEX
Am 19.12.19 um 08:23 schrieb Thomas Koenig: Regression-tested. OK for trunk? Ping?
*ping* [patch, fortran] Updated fix PR 92961, ICE on division by zero error in array bounds
Am 22.12.19 um 16:28 schrieb Thomas Koenig: here is an update for the fix for PR 92961, which also takes care of the second test case in the PR (included in the first one). The patch itself should be clear enough - make sure that there is a MATCH_ERROR on matching an array spec which contains 0/(0). Rather than pass around information several calls deep, I chose to use a global variable. Regression-tested. OK for trunk? Ping? I'd like to get the bug count at least go to 902 in the old year (if 900 cannot be achieved :-) Regards Thomas
Re: [Patch] OpenACC – support "if" + "if_present" clauses with "host_data"
Hi Tobias, Build on x86-64-gnu-linux without offloading and with nvptx offloading. OK for the trunk? I can't really say a lot about OpenACC, but the changes do look reasonable. So, OK for trunk. Regards Thomas
[patch, fortran] Updated fix PR 92961, ICE on division by zero error in array bounds
Hello world, here is an update for the fix for PR 92961, which also takes care of the second test case in the PR (included in the first one). The patch itself should be clear enough - make sure that there is a MATCH_ERROR on matching an array spec which contains 0/(0). Rather than pass around information several calls deep, I chose to use a global variable. Regression-tested. OK for trunk? (Only a few bugs to fix to be at least below 900 bugs at the end of the year, by the way - we are at 389 submitted bugs vs. 461 closed, which is not bad). Regards Thomas 2019-12-22 Thomas Koenig PR fortran/92961 * gfortran.h (gfc_seen_div0): Add declaration. * arith.h (gfc_seen_div0): Add definition. (eval_intrinsic): For integer division by zero, set gfc_seen_div0. * decl.c (variable_decl): If resolution resp. simplification fails for array spec and a division of zero error has been seen, return MATCH_ERROR. 2019-12-22 Thomas Koenig PR fortran/92961 * gfortran.dg/arith_divide_2.f90: New test. Index: gfortran.h === --- gfortran.h (Revision 279639) +++ gfortran.h (Arbeitskopie) @@ -2995,6 +2995,8 @@ void gfc_arith_done_1 (void); arith gfc_check_integer_range (mpz_t p, int kind); bool gfc_check_character_range (gfc_char_t, int); +extern bool gfc_seen_div0; + /* trans-types.c */ bool gfc_check_any_c_kind (gfc_typespec *); int gfc_validate_kind (bt, int, bool); Index: arith.c === --- arith.c (Revision 279639) +++ arith.c (Arbeitskopie) @@ -32,6 +32,8 @@ along with GCC; see the file COPYING3. If not see #include "target-memory.h" #include "constructor.h" +bool gfc_seen_div0; + /* MPFR does not have a direct replacement for mpz_set_f() from GMP. It's easily implemented with a few calls though. */ @@ -1620,6 +1622,10 @@ eval_intrinsic (gfc_intrinsic_op op, gfc_error (gfc_arith_error (rc), &op1->where); if (rc == ARITH_OVERFLOW) goto done; + + if (rc == ARITH_DIV0 && op2->ts.type == BT_INTEGER) + gfc_seen_div0 = true; + return NULL; } Index: decl.c === --- decl.c (Revision 279639) +++ decl.c (Arbeitskopie) @@ -2551,7 +2551,12 @@ variable_decl (int elem) for (int i = 0; i < as->rank; i++) { e = gfc_copy_expr (as->lower[i]); - gfc_resolve_expr (e); + if (!gfc_resolve_expr (e) && gfc_seen_div0) + { + m = MATCH_ERROR; + goto cleanup; + } + gfc_simplify_expr (e, 0); if (e && (e->expr_type != EXPR_CONSTANT)) { @@ -2561,7 +2566,12 @@ variable_decl (int elem) gfc_free_expr (e); e = gfc_copy_expr (as->upper[i]); - gfc_resolve_expr (e); + if (!gfc_resolve_expr (e) && gfc_seen_div0) + { + m = MATCH_ERROR; + goto cleanup; + } + gfc_simplify_expr (e, 0); if (e && (e->expr_type != EXPR_CONSTANT)) { @@ -2587,7 +2597,12 @@ variable_decl (int elem) if (e->expr_type != EXPR_CONSTANT) { n = gfc_copy_expr (e); - gfc_simplify_expr (n, 1); + if (!gfc_simplify_expr (n, 1) && gfc_seen_div0) + { + m = MATCH_ERROR; + goto cleanup; + } + if (n->expr_type == EXPR_CONSTANT) gfc_replace_expr (e, n); else @@ -2597,7 +2612,12 @@ variable_decl (int elem) if (e->expr_type != EXPR_CONSTANT) { n = gfc_copy_expr (e); - gfc_simplify_expr (n, 1); + if (!gfc_simplify_expr (n, 1) && gfc_seen_div0) + { + m = MATCH_ERROR; + goto cleanup; + } + if (n->expr_type == EXPR_CONSTANT) gfc_replace_expr (e, n); else @@ -2934,6 +2954,7 @@ variable_decl (int elem) cleanup: /* Free stuff up and return. */ + gfc_seen_div0 = false; gfc_free_expr (initializer); gfc_free_array_spec (as); ! { dg-do compile } ! This used to ICE. Original test case by Gerhard Steinmetz. program p integer :: a((0)/0)! { dg-error "Division by zero" } integer :: b(0/(0))! { dg-error "Division by zero" } integer :: c((0)/(0)) ! { dg-error "Division by zero" } integer :: d(0/0) ! { dg-error "Division by zero" } integer :: x = ubound(a,1) ! { dg-error "must be an array" } end
Re: [Patch] PR92990 - fix error message for invalid argument of NULLIFY
Am 20.12.19 um 22:33 schrieb Harald Anlauf: The fix for PR70853 changed an ICE-on-invalid for NULLIFY into a misleading error message. The patch below rectifies that. OK for trunk? OK. Thanks for the patch! Regards Thomas
Re: *Ping* Introduce -finline-arg-packing
Hi Jakub, This patch broke: +FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]\$" absent from output: " -finline-arg-packingPerform argument packing inline" That test verifies that the help texts of all options are terminated with dot or semicolon. Thanks, and sorry for the breakage. Note to self: Try not to forget that dot. This was not caught with "make check-fortran" from the gcc build directory. Maybe it would be a good idea to add that test to check-fortran too. Does anybody have an idea how to do that? Regards Thomas
Re: [pach, fortran] Fix PR 92961, ICE on division by zero error in array bounds
the attached patch fixes an ICE in an array declaration where the specified size came from 0/0. This is an 8/9/10 regression. Actually, it does not fix all testcases in the PR, so some more tweaking is still needed. Regards Thomas
OpenACC regression and development pace
Hi, I just saw this: FAIL: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times gimple "(?n)#pragma omp target oacc_enter_exit_data map\\(delete:MEM\\[\\(c_char \\*\\)[^\\]]+\\] \\[len: [^\\]]+\\]\\) map\\(to:del_f_p \\[pointer set, len: [0-9]+\\]\\) map\\(alloc:del_f_p\\.data \\[pointer assign, bias: [^\\]]+\\]\\) finalize$" 1 FAIL: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times gimple "(?n)#pragma omp target oacc_enter_exit_data map\\(force_from:MEM\\[\\(c_char \\*\\)[^\\]]+\\] \\[len: [^\\]]+\\]\\) map\\(to:cpo_f_p \\[pointer set, len: [0-9]+\\]\\) map\\(alloc:cpo_f_p\\.data \\[pointer assign, bias: [^\\]]+\\]\\) finalize$" 1 Regarding what is currently going on with OpenACC: I do not claim to understand this area of the compiler, but it certainly seems that the current development is too hasty - too many patches flying around, too many regressions occurring. It might be better to slow this down somewhat, and to conduct a more throrough review process before committing. Regards Thomas
[pach, fortran] Fix PR 92961, ICE on division by zero error in array bounds
Hello world, the attached patch fixes an ICE in an array declaration where the specified size came from 0/0. This is an 8/9/10 regression. The actual ICE fix was the NULL check in simplify_intrinsic_op, which in turn led to strange error messages, which are then corrected by the rest of the patch. Rather than try to transport state across I do not know how many levels of calls, I chose a global variable. Regression-tested. OK for all affected branches? Regards Thomas 2019-12-20 Thomas Koenig PR fortran/92961 * gfortran.h (gfc_seen_div0): Add declaration. * arith.h (gfc_seen_div0): Add definition. (eval_intrinsic): For integer division by zero, issue gfc_error_now and set gfc_seen_div0. * decl.c (variable_decl): If a division by zero error has been seen previously, do not issue an addtional error. * expr.c (simplify_intrinsic_op): Return NULL if op1 is NULL. 2019-12-20 Thomas Koenig PR fortran/92961 * gfortran.dg/arith_divide_2.f90: New test. Index: gfortran.h === --- gfortran.h (Revision 279639) +++ gfortran.h (Arbeitskopie) @@ -2995,6 +2995,8 @@ void gfc_arith_done_1 (void); arith gfc_check_integer_range (mpz_t p, int kind); bool gfc_check_character_range (gfc_char_t, int); +extern bool gfc_seen_div0; + /* trans-types.c */ bool gfc_check_any_c_kind (gfc_typespec *); int gfc_validate_kind (bt, int, bool); Index: arith.c === --- arith.c (Revision 279639) +++ arith.c (Arbeitskopie) @@ -32,6 +32,8 @@ along with GCC; see the file COPYING3. If not see #include "target-memory.h" #include "constructor.h" +bool gfc_seen_div0; + /* MPFR does not have a direct replacement for mpz_set_f() from GMP. It's easily implemented with a few calls though. */ @@ -1617,7 +1619,15 @@ eval_intrinsic (gfc_intrinsic_op op, if (rc != ARITH_OK) { - gfc_error (gfc_arith_error (rc), &op1->where); + if (rc == ARITH_DIV0 && op2->ts.type == BT_INTEGER) + { + gfc_error_now (gfc_arith_error (rc), &op2->where); + gfc_seen_div0 = true; + return NULL; + } + else + gfc_error (gfc_arith_error (rc), &op1->where); + if (rc == ARITH_OVERFLOW) goto done; return NULL; Index: decl.c === --- decl.c (Revision 279639) +++ decl.c (Arbeitskopie) @@ -2535,6 +2535,10 @@ variable_decl (int elem) goto cleanup; } + /* eval_intrinsic may signal a division by zero. */ + + gfc_seen_div0 = false; + /* F2018:C830 (R816) An explicit-shape-spec whose bounds are not constant expressions shall appear only in a subprogram, derived type definition, BLOCK construct, or interface body. */ @@ -2573,7 +2577,15 @@ variable_decl (int elem) if (not_constant) { - gfc_error ("Explicit shaped array with nonconstant bounds at %C"); + /* If there is a division by zero error, it will have been reported + previously using gfc_error_now in eval_intrinsic. */ + + if (!gfc_seen_div0) + gfc_error ("Explicit shaped array with nonconstant bounds at " + "%C"); + + gfc_seen_div0 = false; + m = MATCH_ERROR; goto cleanup; } Index: expr.c === --- expr.c (Revision 279639) +++ expr.c (Arbeitskopie) @@ -1153,6 +1153,9 @@ simplify_intrinsic_op (gfc_expr *p, int type) op2 = p->value.op.op2; op = p->value.op.op; + if (op1 == NULL) +return false; + if (!gfc_simplify_expr (op1, type)) return false; if (!gfc_simplify_expr (op2, type)) ! { dg-do compile } ! This used to ICE. Original test case by Gerhard Steinmetz. program p integer :: a((0)/0)! { dg-error "Division by zero" } integer :: b(0/(0))! { dg-error "Division by zero" } integer :: c((0)/(0)) ! { dg-error "Division by zero" } integer :: d(0/0) ! { dg-error "Division by zero" } end
Re: [Patch, Fortran] PR 92996 – fix rank resolution EXPR_ARRAY
Hi Tobias, One has the choice between (a) Using the location where the expression was defined (in the scoping unit) – currently done (i.e. replacing expr->where by expr->symtree->n.sym->where) (b) using the location where the parameter is used, i.e. keeping expr->where despite simplification. Or you could use both. With the nice colorization of error messages that David introduced gcc10, this is quite readable. Hm... this would require an additional member in the gfc_expr structure. While this may be a nice addition, it is certainly not necessary to get this patch in. So, OK for trunk, and thanks for the patch! Regards Thomas
[patch, fortran] Fix PR 91541, ICE on valid for INDEX
Hello world, the attached patch fixes an ICE on valid for INDEX (see test case). The problem was that the KIND argument was still present during scalarization, which caused the ICE. The fix is to remove the KIND argument, and the best place to do this is in resolution. I did try to do this in gfc_conv_intrinsic_index_scan_verify, but it is too late by then. Removing the KIND argument required changing the call signature of gfc_resolve_index_func, which in turn required the rest of the changes (including the one in trans-decl.c - I am not convinced that what we are doing there is right, but for this bug fix, I left the functionality as is). Regression-tested. OK for trunk? Regards Thomas 2019-12-19 Thomas Koenig PR fortran/91541 * intrinsic.c (add_sym_4ind): New function. (add_functions): Use it for INDEX. (resolve_intrinsic): Also call f1m for INDEX. * intrinsic.h (gfc_resolve_index_func): Adjust prototype to take a gfc_arglist instead of individual arguments. * iresolve.c (gfc_resolve_index_func): Adjust arguments. Remove KIND argument if present, and make sure this is not done twice. * trans-decl.c: Include "intrinsic.h". (gfc_get_extern_function_decl): Special case for resolving INDEX. 2019-12-19 Thomas Koenig PR fortran/91541 * gfortran.dg/index_3.f90: New test. Index: intrinsic.c === --- intrinsic.c (Revision 279405) +++ intrinsic.c (Arbeitskopie) @@ -851,7 +851,40 @@ add_sym_4 (const char *name, gfc_isym_id id, enum (void *) 0); } +/* Add a symbol to the function list where the function takes 4 + arguments and resolution may need to change the number or + arrangement of arguments. This is the case for INDEX, which needs + its KIND argument removed. */ +static void +add_sym_4ind (const char *name, gfc_isym_id id, enum klass cl, int actual_ok, + bt type, int kind, int standard, + bool (*check) (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *), + gfc_expr *(*simplify) (gfc_expr *, gfc_expr *, gfc_expr *, + gfc_expr *), + void (*resolve) (gfc_expr *, gfc_actual_arglist *), + const char *a1, bt type1, int kind1, int optional1, + const char *a2, bt type2, int kind2, int optional2, + const char *a3, bt type3, int kind3, int optional3, + const char *a4, bt type4, int kind4, int optional4 ) +{ + gfc_check_f cf; + gfc_simplify_f sf; + gfc_resolve_f rf; + + cf.f4 = check; + sf.f4 = simplify; + rf.f1m = resolve; + + add_sym (name, id, cl, actual_ok, type, kind, standard, cf, sf, rf, + a1, type1, kind1, optional1, INTENT_IN, + a2, type2, kind2, optional2, INTENT_IN, + a3, type3, kind3, optional3, INTENT_IN, + a4, type4, kind4, optional4, INTENT_IN, + (void *) 0); +} + + /* Add a symbol to the subroutine list where the subroutine takes 4 arguments. */ @@ -2153,11 +2186,11 @@ add_functions (void) /* The resolution function for INDEX is called gfc_resolve_index_func because the name gfc_resolve_index is already used in resolve.c. */ - add_sym_4 ("index", GFC_ISYM_INDEX, CLASS_ELEMENTAL, ACTUAL_YES, - BT_INTEGER, di, GFC_STD_F77, - gfc_check_index, gfc_simplify_index, gfc_resolve_index_func, - stg, BT_CHARACTER, dc, REQUIRED, ssg, BT_CHARACTER, dc, REQUIRED, - bck, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL); + add_sym_4ind ("index", GFC_ISYM_INDEX, CLASS_ELEMENTAL, ACTUAL_YES, + BT_INTEGER, di, GFC_STD_F77, + gfc_check_index, gfc_simplify_index, gfc_resolve_index_func, + stg, BT_CHARACTER, dc, REQUIRED, ssg, BT_CHARACTER, dc, REQUIRED, + bck, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL); make_generic ("index", GFC_ISYM_INDEX, GFC_STD_F77); @@ -4434,9 +4467,10 @@ resolve_intrinsic (gfc_intrinsic_sym *specific, gf arg = e->value.function.actual; - /* Special case hacks for MIN and MAX. */ + /* Special case hacks for MIN, MAX and INDEX. */ if (specific->resolve.f1m == gfc_resolve_max - || specific->resolve.f1m == gfc_resolve_min) + || specific->resolve.f1m == gfc_resolve_min + || specific->resolve.f1m == gfc_resolve_index_func) { (*specific->resolve.f1m) (e, arg); return; Index: intrinsic.h === --- intrinsic.h (Revision 279405) +++ intrinsic.h (Arbeitskopie) @@ -517,8 +517,7 @@ void gfc_resolve_ibits (gfc_expr *, gfc_expr *, gf void gfc_resolve_ibset (gfc_expr *, gfc_expr *, gfc_expr *); void gfc_resolve_image_index (gfc_expr *, gfc_expr *, gfc_expr *); void gfc_resolve_image_status (gfc_expr *, gfc_expr *, gfc_expr *); -void gfc_resolve_index_func (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *, - gfc_expr *); +void gfc_resolve_index_func (gfc_expr *, gfc_actual_arglist *); vo
Re: [Patch, fortran] PR92753 - [9/10 Regression] ICE in gfc_trans_call, at fortran/trans-stmt.c:392
Hi Paul, Regtested on FC31/x86_64 - OK for 9- and 10- branches? OK. Thanks for the patch! Regards Thomas
*Ping* Introduce -finline-arg-packing
Am 10.12.19 um 22:22 schrieb Thomas Koenig: Steve made an excellent suggestion: -finline-arg-packing . So, OK with that change? In other words, is https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00485.html OK with renaming the option to -finline-arg-packing ? Regards Thomas
Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in gfc_check_is_contiguous, at fortran/check.c:7157
Hello Harald, Index: gcc/fortran/check.c === --- gcc/fortran/check.c (Revision 279183) +++ gcc/fortran/check.c (Arbeitskopie) @@ -7154,7 +7154,9 @@ bool gfc_check_is_contiguous (gfc_expr *array) { if (array->expr_type == EXPR_NULL - && array->symtree->n.sym->attr.pointer == 1) + && (!array->symtree || + (array->symtree->n.sym && + array->symtree->n.sym->attr.pointer == 1))) I have to admit I do not understand the original code here, nor do I quite understand your fix. Is there any circumstance where array->expr_type == EXPR_NULL, but is_contiguous is valid? What would go wrong if the other tests were removed? Index: gcc/testsuite/gfortran.dg/pr91641.f90 === --- gcc/testsuite/gfortran.dg/pr91641.f90 (Revision 279183) +++ gcc/testsuite/gfortran.dg/pr91641.f90 (Arbeitskopie) @@ -1,7 +1,9 @@ ! { dg-do compile } ! PR fortran/91641 -! Code conyributed by Gerhard Steinmetz +! PR fortran/92898 +! Code contributed by Gerhard Steinmetz program p real, pointer :: z(:) print *, is_contiguous (null(z))! { dg-error "shall be an associated" } + print *, is_contiguous (null()) ! { dg-error "shall be an associated" } end Sometimes, it is necessary to change test cases, when error messages change. If this is not the case, it is better to add new tests to new test cases - this makes regression hunting much easier. Regards Thomas
[patch, fortran, committed] Fix PR 91643, repacking of assumed rank argument
Hello world, this fixes a regression introduced by my inline repacking patch. With the test case, it is simple and obvious enough - do not repack an assumed rank argument (which makes no sense). Committed as obvious and simple as r279203 after regression-testing. 2019-12-10 Thomas Koenig PR fortran/91643 * trans-array.c (gfc_conv_array_parameter): Do not repack an assume dummy argument. 2019-12-10 Thomas Koenig PR fortran/91643 * gfortran.dg/assumed_rank_18.f90: New test. Index: trans-array.c === --- trans-array.c (Revision 279064) +++ trans-array.c (Arbeitskopie) @@ -8141,6 +8141,8 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * if (g77 && optimize && !optimize_size && expr->expr_type == EXPR_VARIABLE && !is_pointer (expr) && ! gfc_has_dimen_vector_ref (expr) + && !(expr->symtree->n.sym->as + && expr->symtree->n.sym->as->type == AS_ASSUMED_RANK) && (fsym == NULL || fsym->ts.type != BT_ASSUMED)) { gfc_conv_subref_array_arg (se, expr, g77, ! { dg-do run } ! PR 91643 - this used to cause an ICE. ! Original test case by Gerhard Steinmetz. program p real :: z(3) = [1.0, 2.0, 3.0] call g(z) contains subroutine g(x) real :: x(..) call h(x) end subroutine h(x) real :: x(*) if (x(1) /= 1.0) stop 1 end end
Re: [patch, fortran] Introduce -finline-pack
Am 09.12.19 um 17:30 schrieb Thomas Koenig: Maybe -finline-repack would be a better name? -finline-internal-pack? Steve made an excellent suggestion: -finline-arg-packing . So, OK with that change? Regards Thomas
[patch, fortran, committed] Fix PR 92863, 'ICE in gfc_typename
Hello world, I have just committed the attached patch as obvious and simple as r279180. The ICE for the test case occurred because a previous error had left the derived field of the typespec NULL. Just returning "invalid type" or "invalid class" in such a case is enough to cure the ICE. Regards Thomas 2019-12-10 Thomas Koenig PR fortran/92863 * misc.c (gfc_typename): If derived component is NULL for derived or class, return "invalid type" or "invalid class", respectively. 2019-12-10 Thomas Koenig PR fortran/92863 * gfortran.dg/interface_45.f90: New test. Index: misc.c === --- misc.c (Revision 279064) +++ misc.c (Arbeitskopie) @@ -164,9 +164,19 @@ gfc_typename (gfc_typespec *ts) sprintf (buffer, "UNION(%s)", ts->u.derived->name); break; case BT_DERIVED: + if (ts->u.derived == NULL) + { + sprintf (buffer, "invalid type"); + break; + } sprintf (buffer, "TYPE(%s)", ts->u.derived->name); break; case BT_CLASS: + if (ts->u.derived == NULL) + { + sprintf (buffer, "invalid class"); + break; + } ts1 = ts->u.derived->components ? &ts->u.derived->components->ts : NULL; if (ts1 && ts1->u.derived && ts1->u.derived->attr.unlimited_polymorphic) sprintf (buffer, "CLASS(*)"); ! { dg-do compile } ! PR 92863 - this used to ICE ! Test case by Arseny Solokha. type(l1) function mp() ! { dg-error "type for function" } call sub(mp) ! { dg-error "Type mismatch" } end function mp function bi(ry) call sub(ry) ! { dg-error "Type mismatch" } end function bi
Re: [patch, fortran] Introduce -finline-pack
Hi Richard, Just as a suggestion, maybe we'd want to extend this to other intrinsics in future so a -fno-inline-intrinsic=pack[,...] is more future proof? (I'd inline all intrinsics by default thus only provide the negative form). You can avoid the extra option parsing complexity by only literally adding -fno-inline-intrinsic=pack for now. I agree that such an option would make sense, I think this is something we should consider for gcc 11. In this instance, your reply shows that the option is poorly named, because it is actually not about the PACK intrinsic, but the internal packing that happens for arguments. Maybe -finline-repack would be a better name? -finline-internal-pack? Regards Thomas
[patch, fortran] Add NULL check before checking interfaces
Hello world, the attached patch fixes an ICE where a NULL check was missing. Committed as obvious and simple after regression-testing as r279087. Since this is an ICE after an error has already been emitted, I don't see any particular need to backport. Regards Thomas 2018-12-08 Thomas Koenig PR fortran/92764 * interface.c (gfc_procedure_use): Check for existence of derived component before using (twice). 2018-12-08 Thomas Koenig PR fortran/92764 * gfortran.dg/interface_44.f90: New test. Index: interface.c === --- interface.c (Revision 279064) +++ interface.c (Arbeitskopie) @@ -3888,6 +3888,7 @@ gfc_procedure_use (gfc_symbol *sym, gfc_actual_arg /* F2008, C1303 and C1304. */ if (a->expr && (a->expr->ts.type == BT_DERIVED || a->expr->ts.type == BT_CLASS) + && a->expr->ts.u.derived && ((a->expr->ts.u.derived->from_intmod == INTMOD_ISO_FORTRAN_ENV && a->expr->ts.u.derived->intmod_sym_id == ISOFORTRAN_LOCK_TYPE) || gfc_expr_attr (a->expr).lock_comp)) @@ -3901,6 +3902,7 @@ gfc_procedure_use (gfc_symbol *sym, gfc_actual_arg if (a->expr && (a->expr->ts.type == BT_DERIVED || a->expr->ts.type == BT_CLASS) + && a->expr->ts.u.derived && ((a->expr->ts.u.derived->from_intmod == INTMOD_ISO_FORTRAN_ENV && a->expr->ts.u.derived->intmod_sym_id == ISOFORTRAN_EVENT_TYPE) ! { dg-do compile } ! PR 92964 - this used to ICE. ! Original test case by Arseny Solokha type(e6) function dn() ! { dg-error "The type for function" } call sub(dn) end function dn
[patch, fortran] Fix PR 92755
Hello world, In the fix for PR 91783, I had not considered the case that the _data ref could also be somewhere inside. The solution was obvious and simple: Move the check into the loop over the refs. Committed as r279086 after regression-testing. Regards Thomas 2019-12-08 Thomas Koenig PR fortran/92755 * dependency.c (gfc_dep_resolver): Move skipping of _data ref into the loop. 2019-12-08 Thomas Koenig PR fortran/92755 * gfortran.dg/dependency_57.f90: New test. Index: dependency.c === --- dependency.c (Revision 279064) +++ dependency.c (Arbeitskopie) @@ -2098,18 +2098,6 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf gfc_dependency this_dep; bool same_component = false; - /* The refs might come in mixed, one with a _data component and one - without. Look at their next reference in order to avoid an - ICE. */ - - if (lref && lref->type == REF_COMPONENT && lref->u.c.component - && strcmp (lref->u.c.component->name, "_data") == 0) -lref = lref->next; - - if (rref && rref->type == REF_COMPONENT && rref->u.c.component - && strcmp (rref->u.c.component->name, "_data") == 0) -rref = rref->next; - this_dep = GFC_DEP_ERROR; fin_dep = GFC_DEP_ERROR; /* Dependencies due to pointers should already have been identified. @@ -2117,6 +2105,18 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf while (lref && rref) { + /* The refs might come in mixed, one with a _data component and one + without. Look at their next reference in order to avoid an + ICE. */ + + if (lref && lref->type == REF_COMPONENT && lref->u.c.component + && strcmp (lref->u.c.component->name, "_data") == 0) + lref = lref->next; + + if (rref && rref->type == REF_COMPONENT && rref->u.c.component + && strcmp (rref->u.c.component->name, "_data") == 0) + rref = rref->next; + /* We're resolving from the same base symbol, so both refs should be the same type. We traverse the reference chain until we find ranges that are not equal. */ ! { dg-do compile } ! PR 92755 - this used to cause an ICE. ! Original test case by Gerhard Steinmetz program p type t end type type t2 class(t), allocatable :: a(:) end type type(t2) :: z z%a = [z%a] end
[patch, fortran] Introduce -finline-pack
Hello world, the attached patch introduces a new option, -finline-pack. Since the fix for PR88821, we now do inline packing of arguments (if required) via the scalarizer, instead of using _gfortran_internal_[un]pack when optimizing, but not when optimizing for size. This introduces (really) large performance gains for some test cases because now the middle end can see through the packing. On the other hand, for test cases which do a _lot_ of this, compile time and code size can increase by quite a bit. So, this patch introduces an option to control that behavior, so that people can turn it off on a by-file basis if they don't want it. OK for trunk? Regards Thomas Introduce -finline-pack. 2019-12-07 Thomas Koenig PR middle-end/91512 PR fortran/92738 * invoke.texi: Document -finline-pack. * lang.opt: Add -finline-pack. * options.c (gfc_post_options): Handle -finline-pack. * trans-array.c (gfc_conv_array_parameter): Use flag_inline_pack instead of checking for optimize and optimize_size. 2019-12-07 Thomas Koenig PR middle-end/91512 PR fortran/92738 * gfortran.dg/inline_pack_25.f90: New test. Index: invoke.texi === --- invoke.texi (Revision 279064) +++ invoke.texi (Arbeitskopie) @@ -192,8 +192,9 @@ and warnings}. -ffrontend-loop-interchange -ffrontend-optimize @gol -finit-character=@var{n} -finit-integer=@var{n} -finit-local-zero @gol -finit-derived -finit-logical=@var{} @gol --finit-real=@var{} @gol --finline-matmul-limit=@var{n} -fmax-array-constructor=@var{n} @gol +-finit-real=@var{} +-finline-matmul-limit=@var{n} @gol +-finline-pack -fmax-array-constructor=@var{n} @gol -fmax-stack-var-size=@var{n} -fno-align-commons -fno-automatic @gol -fno-protect-parens -fno-underscoring -fsecond-underscore @gol -fpack-derived -frealloc-lhs -frecursive -frepack-arrays @gol @@ -1779,6 +1780,34 @@ compiled with the @option{-fshort-enums} option. GNU Fortran choose the smallest @code{INTEGER} kind a given enumerator set will fit in, and give all its enumerators this kind. +@item -finline-pack +@opindex @code{finline-pack} +When passing an assumed-shape argument of a procedure as actual +argument to an assumed-size or explicit size or as argument to a +procedure that does not have an explicit interface, the argument may +have to be packed, that is put into contiguous memory. An example is +the call to @code{foo} in +@smallexample + subroutine foo(a) + real, dimension(*) :: a + end subroutine foo + subroutine bar(b) + real, dimension(:) :: b + call foo(b) + end subroutine bar +@end smallexample + +When @option{-finline-pack} is in effect, this packing will be +performed by inline code. This allows for more optimization while +increasing code size. + +@option{-finlie-pack} is implied by any of the @option{-O} options +except when optimizing for size via @option{-Os}. If the code +contains a very large number of argument that have to be packed, code +size and also compilation time may become excessive. If that is the +case, it may be better to disable this option. Instances of packing +can be found by using by using @option{-Warray-temporaries}. + @item -fexternal-blas @opindex @code{fexternal-blas} This option will make @command{gfortran} generate calls to BLAS functions Index: lang.opt === --- lang.opt (Revision 279064) +++ lang.opt (Arbeitskopie) @@ -647,6 +647,10 @@ Enum(gfc_init_local_real) String(inf) Value(GFC_IN EnumValue Enum(gfc_init_local_real) String(-inf) Value(GFC_INIT_REAL_NEG_INF) +finline-pack +Fortran Var(flag_inline_pack) Init(-1) +-finline-pack Perform argument packing inline + finline-matmul-limit= Fortran RejectNegative Joined UInteger Var(flag_inline_matmul_limit) Init(-1) -finline-matmul-limit= Specify the size of the largest matrix for which matmul will be inlined. Index: options.c === --- options.c (Revision 279064) +++ options.c (Arbeitskopie) @@ -467,6 +467,11 @@ gfc_post_options (const char **pfilename) if (flag_frontend_loop_interchange == -1) flag_frontend_loop_interchange = optimize; + /* Do inline packing by default if optimizing, but not if + optimizing for size. */ + if (flag_inline_pack == -1) +flag_inline_pack = optimize && !optimize_size; + if (flag_max_array_constructor < 65535) flag_max_array_constructor = 65535; Index: trans-array.c === --- trans-array.c (Revision 279064) +++ trans-array.c (Arbeitskopie) @@ -8139,7 +8139,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * making the packing and unpacking operation visible to the optimizers. */ - if (g77 && optimize && !optimize_size && expr-&g
*ping* [patch, fortran] Fix PR 91783
Am 24.11.19 um 18:09 schrieb Thomas Koenig: Hello world, this patch fixes a 10 regression in dependency checking. The approach is simple - if gfc_dep_resolver is handed references with _data, remove that. Regression-tested. OK for trunk? Ping?
[patch, fortran] Fix PR 91783
Hello world, this patch fixes a 10 regression in dependency checking. The approach is simple - if gfc_dep_resolver is handed references with _data, remove that. Regression-tested. OK for trunk? Regards Thomas Do not look at _data component in gfc_dep_resolver. 2019-11-24 Thomas Koenig PR fortran/91783 * dependency.c (gfc_dep_resolver): Do not look at _data component if present. 2019-11-24 Thomas Koenig PR fortran/91783 * gfortran.dg/dependency_56.f90: New test. Index: fortran/dependency.c === --- fortran/dependency.c (Revision 278025) +++ fortran/dependency.c (Arbeitskopie) @@ -2098,6 +2098,18 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf gfc_dependency this_dep; bool same_component = false; + /* The refs might come in mixed, one with a _data component and one + without. Look at their next reference in order to avoid an + ICE. */ + + if (lref && lref->type == REF_COMPONENT && lref->u.c.component + && strcmp (lref->u.c.component->name, "_data") == 0) +lref = lref->next; + + if (rref && rref->type == REF_COMPONENT && rref->u.c.component + && strcmp (rref->u.c.component->name, "_data") == 0) +rref = rref->next; + this_dep = GFC_DEP_ERROR; fin_dep = GFC_DEP_ERROR; /* Dependencies due to pointers should already have been identified. ! { dg-do compile } ! PR 91783 - used to cause an ICE in dependency checking. ! Test case by Gerhard Steinmetz. program p class(*), allocatable :: a(:) a = [1, 2, 3] a = f(a) contains function f(x) result(y) class(*), allocatable, intent(in) :: x(:) class(*), allocatable :: y(:) y = x end end
Update: [patch, libfortran] Fix EOF handling in array I/O
Here's an update to the previous patch. Upon reflection, I think it is better for performance to have two versions of the loop so the test is only performed when it is needed. So, OK for trunk? Regards Thomas Fix EOF handling for arrays. 2019-11-23 Thomas Koenig Harald Anlauf PR fortran/92569 * io/transfer.c (transfer_array_inner): If position is at AFTER_ENDFILE in current unit, return from data loop. 2019-11-23 Thomas Koenig Harald Anlauf PR fortran/92569 * gfortran.dg/eof_4.f90: New test. Index: io/transfer.c === --- io/transfer.c (Revision 278025) +++ io/transfer.c (Arbeitskopie) @@ -2542,26 +2542,62 @@ transfer_array_inner (st_parameter_dt *dtp, gfc_ar data = GFC_DESCRIPTOR_DATA (desc); - while (data) + /* When reading, we need to check endfile conditions so we do not miss + an END=label. Make this separate so we do not have an extra test + in a tight loop when it is not needed. */ + + if (dtp->u.p.current_unit && dtp->u.p.mode == READING) { - dtp->u.p.transfer (dtp, iotype, data, kind, size, tsize); - data += stride0 * tsize; - count[0] += tsize; - n = 0; - while (count[n] == extent[n]) + while (data) { - count[n] = 0; - data -= stride[n] * extent[n]; - n++; - if (n == rank) + if (unlikely (dtp->u.p.current_unit->endfile == AFTER_ENDFILE)) + return; + + dtp->u.p.transfer (dtp, iotype, data, kind, size, tsize); + data += stride0 * tsize; + count[0] += tsize; + n = 0; + while (count[n] == extent[n]) { - data = NULL; - break; + count[n] = 0; + data -= stride[n] * extent[n]; + n++; + if (n == rank) + { + data = NULL; + break; + } + else + { + count[n]++; + data += stride[n]; + } } - else + } +} + else +{ + while (data) + { + dtp->u.p.transfer (dtp, iotype, data, kind, size, tsize); + data += stride0 * tsize; + count[0] += tsize; + n = 0; + while (count[n] == extent[n]) { - count[n]++; - data += stride[n]; + count[n] = 0; + data -= stride[n] * extent[n]; + n++; + if (n == rank) + { + data = NULL; + break; + } + else + { + count[n]++; + data += stride[n]; + } } } } ! { dg-do run } ! { dg-options "-ffrontend-optimize" } ! PR 92569 - the EOF condition was not recognized with ! -ffrontend-optimize. Originjal test case by Bill Lipa. program main implicit none real(kind=8) :: tdat(1000,10) real(kind=8) :: res (10, 3) integer :: i, j, k, np open (unit=20, status="scratch") res = reshape([(real(i),i=1,30)], shape(res)) write (20,'(10G12.5)') res rewind 20 do j = 1,1000 read (20,*,end=1)(tdat(j,k),k=1,10) end do 1 continue np = j-1 if (np /= 3) stop 1 if (any(transpose(res) /= tdat(1:np,:))) stop 2 end program main
Re: [patch, libfortran] Fix EOF handling in array I/O
Am 23.11.19 um 14:15 schrieb Thomas Koenig: * gfortran.dg/eof_4.f90: New test. This should be eof_6.f90 (and will be on commit). Regards Thomas
Re: Committed test case for PR 92442
Actually, that was 92422. ChangeLog changed correspondingly.
Committed test case for PR 92442
Hi, the PR is a gcc-9 only regression, for which I just committed a test case to trunk. We can then check what fixed this... 2019-11-23 Thomas Koenig PR fortran/92442 * gfortran.dg/bounds_check_21.f90: New test. ! { dg-do compile } ! { dg-options "-Warray-bounds -O2" } ! PR 92422 - this complained about an array subscript out of bounds. PROGRAM character_warning CHARACTER(len=16) :: word word = 'hi' WRITE(*,*) word END PROGRAM character_warning
[patch, libfortran] Fix EOF handling in array I/O
Hello world, the attached patch fixes a case where transforming do j = 1,1000 read (20,*,end=1)(tdat(j,k),k=1,10) end do 1 continue which on straight transformation yields DO main:j=1 1000 1 READ UNIT=20 FMT=-1 DO main:k=1 10 1 TRANSFER main:tdat(main:j , main:k) END DO DT_END END=1 END DO 1 CONTINUE into DO main:j=1 1000 1 READ UNIT=20 FMT=-1 TRANSFER main:tdat(main:j , 1:10:1) DT_END END=1 END DO 1 CONTINUE with front-end optimization led to a case where the END statement was not interpreted correctly. The solution, to jump out of transfer_array_inner when the file was at its end, was found by Harald; I just added a NULL pointer check to make some regressions go away. Regression-tested. OK for all affected branches (trunk, gcc 9 and gcc 8)? Regards Thomas Fix EOF handling for arrays. 2019-11-23 Thomas Koenig Harald Anlauf PR fortran/92569 * io/transfer.c (transfer_array_inner): If position is at AFTER_ENDFILE in current unit, return from data loop. 2019-11-23 Thomas Koenig Harald Anlauf PR fortran/92569 * gfortran.dg/eof_4.f90: New test. Index: io/transfer.c === --- io/transfer.c (Revision 278025) +++ io/transfer.c (Arbeitskopie) @@ -2544,6 +2544,10 @@ transfer_array_inner (st_parameter_dt *dtp, gfc_ar while (data) { + if (unlikely (dtp->u.p.current_unit + && dtp->u.p.current_unit->endfile == AFTER_ENDFILE)) + return; + dtp->u.p.transfer (dtp, iotype, data, kind, size, tsize); data += stride0 * tsize; count[0] += tsize; ! { dg-do run } ! { dg-options "-ffrontend-optimize" } ! PR 92569 - the EOF condition was not recognized with ! -ffrontend-optimize. Originjal test case by Bill Lipa. program main implicit none real(kind=8) :: tdat(1000,10) real(kind=8) :: res (10, 3) integer :: i, j, k, np open (unit=20, status="scratch") res = reshape([(real(i),i=1,30)], shape(res)) write (20,'(10G12.5)') res rewind 20 do j = 1,1000 read (20,*,end=1)(tdat(j,k),k=1,10) end do 1 continue np = j-1 if (np /= 3) stop 1 if (any(transpose(res) /= tdat(1:np,:))) stop 2 end program main
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Am 19.11.19 um 11:39 schrieb Bernhard Reutner-Fischer: + char name[GFC_MAX_SYMBOL_LEN + 1]; + snprintf (name, GFC_MAX_SYMBOL_LEN, "__dummy_%d_%s", var_num++, + f->sym->name); + + if (gfc_get_sym_tree (name, ns, &symtree, false) != 0) (I) you should + sizeof(__dummy__) + 10 for unsigned long %d or the like. GFC_MAX_SYMBOL_LEN is the maximum length of a gfortran symbol. AFAIK, it is not possible to use a longer symbol name than that, so it needs to be truncated. Uniqueness of the variable name is guaranteed by the var_num variable. If the user puts dummy arguments Supercalifragilisticexpialidociousa and Supercalifragilisticexpialidociousb into the argument list of a procedure, he will have to look at the numbers to differentiate them :-) (II) s/__dummy/__intent_in/ for clarity? It's moved away a bit from INTENT(IN) now, because an argument which cannot be modified (even by passing to a procedure with a dummy argument with unknown intent) is now also handled. Regards Thomas
Re: [Patch, Fortran] dec comparisons - for review
Hi Steve, On Fri, Nov 15, 2019 at 10:40:56AM +, Mark Eggleston wrote: This patch allows comparison of numeric values with Holleriths. This feature is not guarded by a compiler option as it is preferred that extra options should avoided, this seems reasonable as current Hollerith support does not have such an option. IMHO. Has the comparison of a numeric value and a hollerith ever been allowed in a Fortran standard? If the answer is 'No', then you should (1) put the comparison behind an option, and (2) have gfortran issue an error without the option. If this is a DEC extension, then put it behind -fdec. If this misfeature is not a DEC extension but allow some ancient piece of code to compile, then put it behind -std=lagacy. I concur. Additionally, please put in a test case which confirms that an error is indeed emitted without that particular option. Regards Thomas
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Hello world, here is an update to the patch. I have now included variables where the user did not specify INTENT(IN) by checking that the dummy variables to be replaced by temporaries are not, indeed, assigned a value. This also includes being passed as an actual argument to a non-INTENT(IN) dummy argument. Extending this led to being able to catch a few more bugs. I have addes one test case to check where the new temporaries are added. Regression-tested. The only change I see in the testsuite now is XPASS: gfortran.dg/goacc/kernels-loop-n.f95 -O scan-tree-dump-times parloops1 "(?n)__attribute__\\(\\(oacc kernels parallelized, oacc function \\(, , \\), oacc kernels, omp target entrypoint\\)\\)" 1 So, OK for trunk? Regards Thomas 2019-11-11 Thomas Koenig PR fortran/67202 * dump-parse-tree.c (debug): Add for gfc_namespace. (show_code_node): Add INIT_ on dumping EXEC_INIT_ASSIGN. * frontent-passes.c (replace_intent_in): Add prototype. New function. (optimize_namespace): Call it. (sym_replacement): New struct. (defined_code_callback): New function. (defined_expr_callback): New function. (replace_symbol_in_expr): New function. 2019-11-11 Thomas Koenig PR fortran/67202 * gfortran.dg/intent_optimize_3.f90: New test. * gfortran.dg/intent_optimize_4.f90: New test. * gfortran.dg/pr26246_2.f90: Add -fno-frontend-optimize to flags. Index: fortran/dump-parse-tree.c === --- fortran/dump-parse-tree.c (Revision 278025) +++ fortran/dump-parse-tree.c (Arbeitskopie) @@ -57,6 +57,15 @@ static void show_attr (symbol_attribute *, const c /* Allow dumping of an expression in the debugger. */ void gfc_debug_expr (gfc_expr *); +void debug (gfc_namespace *ns) +{ + FILE *tmp = dumpfile; + dumpfile = stderr; + show_namespace (ns); + fputc ('\n', dumpfile); + dumpfile = tmp; +} + void debug (symbol_attribute *attr) { FILE *tmp = dumpfile; @@ -1889,6 +1898,9 @@ show_code_node (int level, gfc_code *c) break; case EXEC_INIT_ASSIGN: + fputs ("INIT_", dumpfile); + /* Fallthrough */ + case EXEC_ASSIGN: fputs ("ASSIGN ", dumpfile); show_expr (c->expr1); Index: fortran/frontend-passes.c === --- fortran/frontend-passes.c (Revision 278025) +++ fortran/frontend-passes.c (Arbeitskopie) @@ -57,6 +57,7 @@ static int call_external_blas (gfc_code **, int *, static int matmul_temp_args (gfc_code **, int *,void *data); static int index_interchange (gfc_code **, int*, void *); static bool is_fe_temp (gfc_expr *e); +static void replace_intent_in (gfc_namespace *); #ifdef CHECKING_P static void check_locus (gfc_namespace *); @@ -1467,6 +1468,7 @@ optimize_namespace (gfc_namespace *ns) if (flag_frontend_optimize) { + replace_intent_in (ns); gfc_code_walker (&ns->code, simplify_io_impl_do, dummy_expr_callback, NULL); gfc_code_walker (&ns->code, convert_do_while, dummy_expr_callback, NULL); gfc_code_walker (&ns->code, convert_elseif, dummy_expr_callback, NULL); @@ -4969,7 +4971,7 @@ gfc_expr_walker (gfc_expr **e, walk_expr_fn_t expr if ((*e)->expr_type != EXPR_ARRAY) break; - /* Fall through to the variable case in order to walk the + /* Fall through to the variable case in order to walk the reference. */ gcc_fallthrough (); @@ -5503,3 +5505,330 @@ gfc_check_externals (gfc_namespace *ns) gfc_errors_to_warnings (false); } + +/* For scalar INTENT(IN) variables or for variables where we know +their value is not changed, we can replace them by an auxiliary +variable whose value is set on procedure entry. */ + +typedef struct sym_replacement +{ + gfc_symbol *original; + gfc_symtree *replacement_symtree; + bool referenced; + +} sym_replace; + +/* Callback function - replace expression if possible, and set + sr->referenced if this was done (so we know we need to generate + the assignment statement). */ + +static int +replace_symbol_in_expr (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + gfc_expr *expr = *e; + sym_replacement *sr; + + if (expr->expr_type != EXPR_VARIABLE || expr->symtree == NULL) +return 0; + + sr = (sym_replacement *) data; + + if (expr->symtree->n.sym == sr->original) +{ + expr->symtree = sr->replacement_symtree; + sr->referenced = true; +} + + return 0; +} + +/* Callback to check if the symbol passed as data could be redefined. + Return 1 if this is the case. */ + +#define CHECK_TAG(member,tag) \ + do \ + { \ + if (co->ext.member->tag && co->ext.member->tag->symtree \ + && co->ext.member->t
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Am 11.11.19 um 22:55 schrieb Thomas König: Regression-tested. OK for trunk? Of course, better with a ChangeLog entry. 2019-11-11 Thomas Koenig PR fortran/67202 * dump-parse-tree.c (debug): Add for gfc_namespace. (show_code_node): Add INIT_ on dumping EXEC_INIT_ASSIGN. * frontent-passes.c (replace_intent_in): Add prototype. New function. (optimize_namespace): Call it. (sym_replacement): New struct. (replace_symbol_in_expr): New function. 2019-11-11 Thomas Koenig PR fortran/67202 * gfortran.dg/intent_optimize_3.f90: New test.
Re: [PATCH] Bump minimum MPFR version to 3.1.0
Hi Janne, Bump the minimum MPFR version to 3.1.0, released 2011-10-03. With this requirement one can still build GCC with the operating system provided MPFR on old but still supported operating systems like SLES 12 (MPFR 3.1.2) or RHEL/CentOS 7.x (MPFR 3.1.1). OK for trunk. Can you also make a note in https://gcc.gnu.org/gcc-10/changes.html ? Regards Thomas
[patch, fortran, committed] Commit symbol for external BLAS routine when translating MATMUL to *GEMM.
Hi, I just committed the patch below as obvious to fix a 9/10 regression when directly calling BLAS routines for matmul. Will backport to gcc-9 soon. Regards Thomas Commit symbol for external BLAS routine when translating MATMUL to *GEMM. 2019-11-09 Thomas Koenig PR fortran/92321 * frontend-passes.c (call_external_blas): Commit symbol for external BLAS routine. 2019-11-09 Thomas Koenig PR fortran/92321 * gfortran.dg/matmul_blas_2.f90: New test. ! { dg-do compile } ! { dg-options "-O3 -fdump-tree-original -fexternal-blas" } ! PR fortran/92321 - this used to cause an ICE. Original test case ! by Nathan Wukie. module mod_badmatmul implicit none contains subroutine test(c) real, intent(inout) :: c(3,3) real :: a(3,3), b(3,3) c = matmul(a, b) end subroutine test end module mod_badmatmul program main use mod_badmatmul, only: test implicit none real :: a(3,3) call test(a) end program main Index: frontend-passes.c === --- frontend-passes.c (Revision 277999) +++ frontend-passes.c (Arbeitskopie) @@ -4635,6 +4635,7 @@ call_external_blas (gfc_code **c, int *walk_subtre call->symtree->n.sym->attr.procedure = 1; call->symtree->n.sym->attr.flavor = FL_PROCEDURE; call->resolved_sym = call->symtree->n.sym; + gfc_commit_symbol (call->resolved_sym); /* Argument TRANSA. */ next = gfc_get_actual_arglist ();
Re: [patch, fortran] Fix PR 92113
Hi Steve, OK for trunk/9/8? OK for all three. Thanks, committed to trunk as r277760. I'll be AFK for a few days, so I will have to wait before committing this to gcc-9. Given the convoluted history of this bug, this might not be a bad thing. > It is, as you have indicated, troublesome that a segfaulting > testcase isn't caught by the testsuite. It certainly is, but I have no solution for this at the moment. Thanks for the review! Regards Thomas
[patch, fortran] Fix PR 92113
Hello world, the attached patch fixes an 8/9/10 regression where, to fix PR 84487 by not putting the initializers and vtabs into the read-only section (for reasons of size, which could grow enormously) led to a regression on POWER9 and other non-x86 architectures, where the initializer was sometimes optimized away, depending on optimization levels. This was a strange beast to hunt down. This only showed up on the testresults for gcc 8, so I tried to find out what commit had fixed this on trunk, in order to backport. However, bisecting this I found that the test case actually segfaults all the way up to current trunk when run by hand. By running the testsuite, I didn't see it. This is strange, and raises some issues about the testsuite and the possibility of a latent issue, but I lack the knowledge to hunt this down. In the meantime, here is this patch, which puts the vtabs and the initializers where the user actually specified something into the read-only section again. Test case: Well, theoretically it is already there, so it makes little sense to add a new one. Regression-tested on powerpc64le-unknown-linux-gnu, also verified by hand that pr51434.f90 now passes with -O2 there. OK for trunk/9/8? Regards Thomas 2019-11-02 Thomas Koenig PR fortran/92133 * trans-decl.c (gfc_get_symbol_decl): If __def_init actually contains a value, put it into the read-only section. Index: trans-decl.c === --- trans-decl.c (Revision 277486) +++ trans-decl.c (Arbeitskopie) @@ -1911,14 +1911,19 @@ gfc_get_symbol_decl (gfc_symbol * sym) if (sym->attr.associate_var) GFC_DECL_ASSOCIATE_VAR_P (decl) = 1; - /* We no longer mark __def_init as read-only so it does not take up - space in the read-only section and dan go into the BSS instead, - see PR 84487. Marking this as artificial means that OpenMP will - treat this as predetermined shared. */ - if (sym->attr.vtab - || (sym->name[0] == '_' && gfc_str_startswith (sym->name, "__def_init"))) -DECL_ARTIFICIAL (decl) = 1; + /* We only mark __def_init as read-only if it actually has an + initializer so it does not needlessly take up space in the + read-only section and can go into the BSS instead, see PR 84487. + Marking this as artificial means that OpenMP will treat this as + predetermined shared. */ + if (sym->attr.vtab || gfc_str_startswith (sym->name, "__def_init")) +{ + DECL_ARTIFICIAL (decl) = 1; + if (sym->attr.vtab || sym->value) + TREE_READONLY (decl) = 1; +} + return decl; }
Re: [Patch][Fortran] PR 92208 don't use function-result dummy variable as actual argument
Hi Tobias, OK for the trunk and GCC 9? As far as I can see, this looks good. So, OK for trunk. If it later turns out that there are problems caused by this, I suspect we will hear about them soon enough :-) Thanks for taking this on! Regards Thomas
Re: Argument-mismatch fallout
However, passing a scalar instead of an array/array element worked/works with (nearly?) all compilers. Hence, passing a scalar is seemingly common pattern. Thus, I wonder whether we should do something about this. Maybe we could mention -fallow-argument-mismatch in the error message. (Really, the changes needed are quite trivial, and the code is in fact invalid. I actually wrote the patch for LAPACK which removed all the fallout from their TESTING routines; it didn't take long). Regards Thomas
Re: [patch, fortran] Fix PR 92004, restore Lapack compilation
Committed, with that nitch, r276972. OK with a minor nit. — Thanks for the patch. Thanks a lot for the review! Regards Thomas
Re: [patch, fortran] Fix PR 92004, restore Lapack compilation
OK, so here's the update. There was a problem with uninitialized variables, which for some reason was not detected on compilation. OK for trunk? 2019-10-13 Thomas Koenig PR fortran/92004 * array.c (expand_constructor): Set from_constructor on expression. * gfortran.h (gfc_symbol): Add maybe_array. (gfc_expr): Add from_constructor. * interface.c (maybe_dummy_array_arg): New function. (compare_parameter): If the formal argument is generated from a call, check the conditions where an array element could be passed to an array. Adjust error message for assumed-shape or pointer array. Use correct language for assumed shaped arrays. (gfc_get_formal_from_actual_arglist): Set maybe_array on the symbol if the actual argument is an array element fulfilling the conditions of 15.5.2.4. 2019-10-13 Thomas Koenig PR fortran/92004 * gfortran.dg/argument_checking_24.f90: New test. * gfortran.dg/abstract_type_6.f90: Add error message. * gfortran.dg/argument_checking_11.f90: Correct wording in error message. * gfortran.dg/argumeent_checking_13.f90: Likewise. * gfortran.dg/interface_40.f90: Add error message. Index: fortran/array.c === --- fortran/array.c (Revision 276506) +++ fortran/array.c (Arbeitskopie) @@ -1763,6 +1763,7 @@ expand_constructor (gfc_constructor_base base) gfc_free_expr (e); return false; } + e->from_constructor = 1; current_expand.offset = &c->offset; current_expand.repeat = &c->repeat; current_expand.component = c->n.component; Index: fortran/gfortran.h === --- fortran/gfortran.h (Revision 276506) +++ fortran/gfortran.h (Arbeitskopie) @@ -1614,6 +1614,9 @@ typedef struct gfc_symbol /* Set if a previous error or warning has occurred and no other should be reported. */ unsigned error:1; + /* Set if the dummy argument of a procedure could be an array despite + being called with a scalar actual argument. */ + unsigned maybe_array:1; int refs; struct gfc_namespace *ns; /* namespace containing this symbol */ @@ -2194,6 +2197,11 @@ typedef struct gfc_expr /* Set this if no warning should be given somewhere in a lower level. */ unsigned int do_not_warn : 1; + + /* Set this if the expression came from expanding an array constructor. */ + + unsigned int from_constructor : 1; + /* If an expression comes from a Hollerith constant or compile-time evaluation of a transfer statement, it may have a prescribed target- memory representation, and these cannot always be backformed from Index: fortran/interface.c === --- fortran/interface.c (Revision 276506) +++ fortran/interface.c (Arbeitskopie) @@ -2229,6 +2229,67 @@ argument_rank_mismatch (const char *name, locus *w } +/* Under certain conditions, a scalar actual argument can be passed + to an array dummy argument - see F2018, 15.5.2.4, paragraph 14. + This function returns true for these conditions so that an error + or warning for this can be suppressed later. Always return false + for expressions with rank > 0. */ + +bool +maybe_dummy_array_arg (gfc_expr *e) +{ + gfc_symbol *s; + gfc_ref *ref; + bool array_pointer = false; + bool assumed_shape = false; + bool scalar_ref = true; + + if (e->rank > 0) +return false; + + if (e->ts.type == BT_CHARACTER && e->ts.kind == 1) +return true; + + /* If this comes from a constructor, it has been an array element + originally. */ + + if (e->expr_type == EXPR_CONSTANT) +return e->from_constructor; + + if (e->expr_type != EXPR_VARIABLE) +return false; + + s = e->symtree->n.sym; + + if (s->attr.dimension) +{ + scalar_ref = false; + array_pointer = s->attr.pointer; +} + + if (s->as && s->as->type == AS_ASSUMED_SHAPE) +assumed_shape = true; + + for (ref=e->ref; ref; ref=ref->next) +{ + if (ref->type == REF_COMPONENT) + { + symbol_attribute *attr; + attr = &ref->u.c.component->attr; + if (attr->dimension) + { + array_pointer = attr->pointer; + assumed_shape = false; + scalar_ref = false; + } + else + scalar_ref = true; + } +} + + return !(scalar_ref || array_pointer || assumed_shape); +} + /* Given a symbol of a formal argument list and an expression, see if the two are compatible as arguments. Returns true if compatible, false if not compatible. */ @@ -2544,7 +2605,9 @@ compare_parameter (gfc_symbol *formal, gfc_expr *a || (actual->rank == 0 && formal->attr.dimension && gfc_is_coindexed (actual))) { -
Re: [patch, fortran] Fix PR 92004, restore Lapack compilation
Hm, my trunk is doing strange things (debugging not working), and I think I have found an additional problem. I'll need some time to work this out, and will resubmit. Regards Thomas
Re: [patch, fortran] Fix PR 92004, restore Lapack compilation
Hi, I think I have resolved all the issues (see attached patch and test case). Basically, the patch now walks through the refs and looks at the latest thing that could be an array or a scalar. Regarding CLASS in argument lists without an explicit interface: I think that this is disallowed because an explicit interface is required for a polymorphic dummy argument, and I see no way of passing a polymorphic argument to a procedure without having a polymorphic argument as a dummy argument. While I was at it, I also changed some language to match the language of the standard more closely. As you can see in the test case, I tried to cover all relevant cases. Regression-tested. OK for trunk? Regards Thomas 2019-10-12 Thomas Koenig PR fortran/92004 * array.c (expand_constructor): Set from_constructor on expression. * gfortran.h (gfc_symbol): Add maybe_array. (gfc_expr): Add from_constructor. * interface.c (maybe_dummy_array_arg): New function. (compare_parameter): If the formal argument is generated from a call, check the conditions where an array element could be passed to an array. Adjust error message for assumed-shape or pointer array. Use correct language for assumed shaped arrays. (gfc_get_formal_from_actual_arglist): Set maybe_array on the symbol if the actual argument is an array element fulfilling the conditions of 15.5.2.4. 2019-10-12 Thomas Koenig PR fortran/92004 * gfortran.dg/argument_checking_24.f90: New test. * gfortran.dg/abstract_type_6.f90: Add error message. * gfortran.dg/argument_checking_11.f90: Correct wording in error message. * gfortran.dg/argumeent_checking_13.f90: Likewise. * gfortran.dg/interface_40.f90: Add error message. Index: fortran/array.c === --- fortran/array.c (Revision 276506) +++ fortran/array.c (Arbeitskopie) @@ -1763,6 +1763,7 @@ expand_constructor (gfc_constructor_base base) gfc_free_expr (e); return false; } + e->from_constructor = 1; current_expand.offset = &c->offset; current_expand.repeat = &c->repeat; current_expand.component = c->n.component; Index: fortran/gfortran.h === --- fortran/gfortran.h (Revision 276506) +++ fortran/gfortran.h (Arbeitskopie) @@ -1614,6 +1614,9 @@ typedef struct gfc_symbol /* Set if a previous error or warning has occurred and no other should be reported. */ unsigned error:1; + /* Set if the dummy argument of a procedure could be an array despite + being called with a scalar actual argument. */ + unsigned maybe_array:1; int refs; struct gfc_namespace *ns; /* namespace containing this symbol */ @@ -2194,6 +2197,11 @@ typedef struct gfc_expr /* Set this if no warning should be given somewhere in a lower level. */ unsigned int do_not_warn : 1; + + /* Set this if the expression came from expanding an array constructor. */ + + unsigned int from_constructor : 1; + /* If an expression comes from a Hollerith constant or compile-time evaluation of a transfer statement, it may have a prescribed target- memory representation, and these cannot always be backformed from Index: fortran/interface.c === --- fortran/interface.c (Revision 276506) +++ fortran/interface.c (Arbeitskopie) @@ -2229,6 +2229,64 @@ argument_rank_mismatch (const char *name, locus *w } +/* Under certain conditions, a scalar actual argument can be passed + to an array dummy argument - see F2018, 15.5.2.4, paragraph 14. + This function returns true for these conditions so that an error + or warning for this can be suppressed later. Always return false + for expressions with rank > 0. */ + +bool +maybe_dummy_array_arg (gfc_expr *e) +{ + gfc_symbol *s; + gfc_ref *ref; + bool array_pointer, assumed_shape, scalar_ref; + + if (e->rank > 0) +return false; + + if (e->ts.type == BT_CHARACTER && e->ts.kind == 1) +return true; + + /* If this comes from a constructor, it has been an array element + originally. */ + + if (e->expr_type == EXPR_CONSTANT) +return e->from_constructor; + + if (e->expr_type != EXPR_VARIABLE) +return false; + + s = e->symtree->n.sym; + + if (s->attr.dimension) +array_pointer = s->attr.pointer; + else +scalar_ref = true; + + if (s->as && s->as->type == AS_ASSUMED_SHAPE) +assumed_shape = true; + + for (ref=e->ref; ref; ref=ref->next) +{ + if (ref->type == REF_COMPONENT) + { + symbol_attribute *attr; + attr = &ref->u.c.component->attr; + if (attr->dimension) + { + array_pointer = attr->pointer; + assumed_shape = false;
Re: [PATCH] PR fortran/91715 -- Detect invalid type-spec
Am 11.10.19 um 05:54 schrieb Steve Kargl: The attach patch has been tested on x86_64-*-freebsd. OK to commit. Yes. Again, thanks a lot for the patch"
Re: [PATCH] PR fortran/91649 -- Add additional checking for FINDLOC
Steve, The attached patch has been tested on x86_64-*-freebsd. OK to commit? OK. Thanks a lot for the patch! Regards Thomas
Re: [patch, fortran] Fix PR 92004, restore Lapack compilation
Hi Tobias, function ("o" missing); I think it is not clause 14 but paragraph 14. Fixed. (That one was easy :-) + warning for this can be suppressed later. */ + +bool +maybe_dummy_array_arg (gfc_expr *e) +{ + gfc_symbol *s; + + if (e->rank > 0) + return false; + + if (e->ts.type == BT_CHARACTER && e->ts.kind == 1) + return true; + + if (e->expr_type != EXPR_VARIABLE) + return false; What about PARAMETER? :-) Good catch. I found that, by the time the code is reached, an element of a parameter array is already simplified; so I added a flag during constructor expansion. + s = e->symtree->n.sym; + if (s->as == NULL) + return false; This looks wrong. You also want to permit dt%array(1) – but not dt(1)%scalar Fixed. + if (s->ts.type == BT_CLASS || s->as->type == AS_ASSUMED_SHAPE + || s->attr.pointer) + return false; dt%foo – again, "foo" can be an allocatable of polymorphic type or a pointer, but at least, it cannot be of assumed shape. Really? The paragraph reads # 14 If the actual argument is a noncoindexed scalar, the corresponding # dummy argument shall be scalar unless # * the actual argument is default character, of type character with the # C character kind (18.2.2), or is an element or substring of an # element of an array that is not an assumed-shape, pointer, or # polymorphic array, (The last two points do not apply here because they are invalid without explicit interface). Unless I have my negatives wrong, the code is correct (but I have been getting standardese wrong before). Anyway, here's an update of the patch. OK, or is there still something missing? Or how should I interpret that paragraph? :-) Regards Thomas Index: array.c === --- array.c (Revision 276506) +++ array.c (Arbeitskopie) @@ -1763,6 +1763,7 @@ expand_constructor (gfc_constructor_base base) gfc_free_expr (e); return false; } + e->from_constructor = 1; current_expand.offset = &c->offset; current_expand.repeat = &c->repeat; current_expand.component = c->n.component; Index: gfortran.h === --- gfortran.h (Revision 276506) +++ gfortran.h (Arbeitskopie) @@ -1614,6 +1614,9 @@ typedef struct gfc_symbol /* Set if a previous error or warning has occurred and no other should be reported. */ unsigned error:1; + /* Set if an interface to a procedure could actually be to an array + although the actual argument is scalar. */ + unsigned maybe_array:1; int refs; struct gfc_namespace *ns; /* namespace containing this symbol */ @@ -2194,6 +2197,11 @@ typedef struct gfc_expr /* Set this if no warning should be given somewhere in a lower level. */ unsigned int do_not_warn : 1; + + /* Set this if the expression came from expanding an array constructor. */ + + unsigned int from_constructor : 1; + /* If an expression comes from a Hollerith constant or compile-time evaluation of a transfer statement, it may have a prescribed target- memory representation, and these cannot always be backformed from Index: interface.c === --- interface.c (Revision 276506) +++ interface.c (Arbeitskopie) @@ -2229,6 +2229,46 @@ argument_rank_mismatch (const char *name, locus *w } +/* Under certain conditions, a scalar actual argument can be passed + to an array dummy argument - see F2018, 15.5.2.4, clause 14. This + functin returns true for these conditions so that an error or + warning for this can be suppressed later. */ + +bool +maybe_dummy_array_arg (gfc_expr *e) +{ + gfc_symbol *s; + gfc_ref *ref; + bool last_array_ref; + + if (e->rank > 0) +return false; + + if (e->ts.type == BT_CHARACTER && e->ts.kind == 1) +return true; + + if (e->expr_type == EXPR_CONSTANT) +return e->from_constructor; + + if (e->expr_type != EXPR_VARIABLE) +return false; + + s = e->symtree->n.sym; + if (s->as == NULL) +return false; + + if (s->ts.type == BT_CLASS || s->as->type == AS_ASSUMED_SHAPE + || s->attr.pointer) +return false; + + last_array_ref = false; + + for (ref=e->ref; ref; ref=ref->next) +last_array_ref = ref->type == REF_ARRAY; + + return last_array_ref; +} + /* Given a symbol of a formal argument list and an expression, see if the two are compatible as arguments. Returns true if compatible, false if not compatible. */ @@ -2544,7 +2584,9 @@ compare_parameter (gfc_symbol *formal, gfc_expr *a || (actual->rank == 0 && formal->attr.dimension && gfc_is_coindexed (actual))) { - if (where) + if (where + && (!formal->attr.artificial || (!formal->maybe_array + && !maybe_dummy_array_arg (actual { locus *where_formal; if (formal->attr.artificial) @@ -2594,9 +2636,17 @@ compare_parameter (gfc_symbol *formal, gfc_expr *a &&
Ping ** (2./7.) Fix PR 92004, restore Lapack compilation
Hi, this patch fixes an overzealous interpretation of F2018 15.5.2.4, where an idiom of passing an array element to an array was rejected. This also restores Lapack compilation without warning. Regression-tested. OK for trunk? Would it be possible to get a speedy review on this? I'd like to get this working again as soon as possible. Regards Thomas
Re: [PATCH] PR fortran/91801 -- convert assert to an error
Hi Steve, Tested on x86_64-*-freebsd. OK to commit? OK. Thanks a lot for the patch!
Re: [patch, fortran] Fix PR 92004, restore Lapack compilation
Am 06.10.19 um 17:26 schrieb Thomas Koenig: This also restores Lapack compilation without warning. Well, up to an error in the testing routines, at least. TESTING/LIN/sdrvls.f has REAL, ALLOCATABLE :: WORK (:) ... REAL RESULT( NTESTS ), WQ and calls CALL SGELS( TRANS, M, N, NRHS, A, LDA, $B, LDB, WQ, -1, INFO ) [...] CALL SGELS( TRANS, M, N, NRHS, A, LDA, B, $LDB, WORK, LWORK, INFO ) so that one really is illegal and should be flagged.
[patch, fortran] Fix PR 92004, restore Lapack compilation
Hello world, this patch fixes an overzealous interpretation of F2018 15.5.2.4, where an idiom of passing an array element to an array was rejected. This also restores Lapack compilation without warning. Regression-tested. OK for trunk? Regards Thomas 2019-10-06 Thomas Koenig PR fortran/92004 * gfortran.h (gfc_symbol): Add maybe_array. * interface.c (maybe_dummy_array_arg): New function. (compare_parameter): If the formal argument is generated from a call, check the conditions where an array element could be passed to an array. Adjust error message for assumed-shape or pointer array. (gfc_get_formal_from_actual_arglist): Set maybe_array on the symbol if the actual argument is an array element fulfilling the conditions of 15.5.2.4. 2019-10-06 Thomas Koenig PR fortran/92004 * gfortran.dg/argument_checking_24.f90: New test. Index: gfortran.h === --- gfortran.h (Revision 276506) +++ gfortran.h (Arbeitskopie) @@ -1614,6 +1614,9 @@ typedef struct gfc_symbol /* Set if a previous error or warning has occurred and no other should be reported. */ unsigned error:1; + /* Set if an interface to a procedure could actually be to an array + although the actual argument is scalar. */ + unsigned maybe_array:1; int refs; struct gfc_namespace *ns; /* namespace containing this symbol */ Index: interface.c === --- interface.c (Revision 276506) +++ interface.c (Arbeitskopie) @@ -2229,6 +2229,36 @@ argument_rank_mismatch (const char *name, locus *w } +/* Under certain conditions, a scalar actual argument can be passed + to an array dummy argument - see F2018, 15.5.2.4, clause 14. This + functin returns true for these conditions so that an error or + warning for this can be suppressed later. */ + +bool +maybe_dummy_array_arg (gfc_expr *e) +{ + gfc_symbol *s; + + if (e->rank > 0) +return false; + + if (e->ts.type == BT_CHARACTER && e->ts.kind == 1) +return true; + + if (e->expr_type != EXPR_VARIABLE) +return false; + + s = e->symtree->n.sym; + if (s->as == NULL) +return false; + + if (s->ts.type == BT_CLASS || s->as->type == AS_ASSUMED_SHAPE + || s->attr.pointer) +return false; + + return true; +} + /* Given a symbol of a formal argument list and an expression, see if the two are compatible as arguments. Returns true if compatible, false if not compatible. */ @@ -2544,7 +2574,9 @@ compare_parameter (gfc_symbol *formal, gfc_expr *a || (actual->rank == 0 && formal->attr.dimension && gfc_is_coindexed (actual))) { - if (where) + if (where + && (!formal->attr.artificial || (!formal->maybe_array + && !maybe_dummy_array_arg (actual { locus *where_formal; if (formal->attr.artificial) @@ -2594,9 +2626,17 @@ compare_parameter (gfc_symbol *formal, gfc_expr *a && (is_pointer || ref->u.ar.as->type == AS_ASSUMED_SHAPE)) { if (where) - gfc_error ("Element of assumed-shaped or pointer " - "array passed to array dummy argument %qs at %L", - formal->name, &actual->where); + { + if (formal->attr.artificial) + gfc_error ("Element of assumed-shaped or pointer array " + "as actual argument at %L can not correspond to " + "actual argument at %L ", + &actual->where, &formal->declared_at); + else + gfc_error ("Element of assumed-shaped or pointer " + "array passed to array dummy argument %qs at %L", + formal->name, &actual->where); + } return false; } @@ -2625,7 +2665,9 @@ compare_parameter (gfc_symbol *formal, gfc_expr *a if (ref == NULL && actual->expr_type != EXPR_NULL) { - if (where) + if (where + && (!formal->attr.artificial || (!formal->maybe_array + && !maybe_dummy_array_arg (actual { locus *where_formal; if (formal->attr.artificial) @@ -5228,6 +5270,8 @@ gfc_get_formal_from_actual_arglist (gfc_symbol *sy s->as->upper[0] = NULL; s->as->type = AS_ASSUMED_SIZE; } + else + s->maybe_array = maybe_dummy_array_arg (a->expr); } s->attr.dummy = 1; s->declared_at = a->expr->where; ! { dg-do compile } ! PR module x implicit none contains subroutine foo(a) real, dimension(:) :: a call ext_1(a(1)) ! { dg-error "Rank mismatch" } call ext_1(a) ! { dg-error "Rank mismatch" } call ext_2(a) ! { dg-error "Element of assumed-shaped or pointer" } call ext_2(a(1)) ! { dg-error "Element of assumed-shaped or pointer" } end subroutine foo subroutine bar(a) real, dimension(*) :: a ! None of the ones below should issue an error. call ext_3 (a) call ext_3 (a(1)) call ext_4 (a(1)) call ext_4 (a) end subroutine bar end module x
Re: [PATCH] PR fortran/91959 -- Re-arrange matching of %FILL
Hi Steve, The attached patch has been tested on x86_64-*-freebsd. OK to commit. OK. Thanks a lot for the patch! Since this seems to be a regression to before when %FILL was introduced, also OK to backport as far as you care to do it. Regards Thomas
Re: [PATCH] PR fortran/91497 -- Silence conversion warnings
Steve, The attach patch silences -Wconversion and -Wconversion-extra warnings that had previously been issued for explicit conversions (see testcase for examples). The patch has been tested on x86-*-freebsd. OK to commit? OK. Thanks a lot for the patch! Regards Thomas
Re: [patch, fortran] PR 84487
Hi, Jakub, do you have any comments? After Seve's OK I plan to commit in a couple of days unless I read anything to the contrary. Regards Thomas this patch makes sure that the __def_init variables, which have been generated for normal allocatable arrays for quite some time, do not fill up huge amounts of space in the object files with zeros. This is done by not marking them read-only, which means that they are put into the BSS. Setting DECL_ARTIFICIAL on the __def_init variable makes sure it is handled as predetermined shared in gfc_omp_predetermined_sharing . This is not an optimum solution. As the xfail shows, we are now missing out on an optimization (as seen by the xfail that is now needed), and having large all-zero variables seems wrong. However, this patch solves the most urgent problem in this respect. This is an 8/9/10 regression, so I would like to commit this to all of these branches (waiting before gcc 9 reopens, of course). I wold then close the PR and open an enchancement PR for the xfail and the design improvement. Test case... I'm not sure what to test for. Regression-tested. OK for all affected branches? Your approach seems reasonable to me. You may want to ping Jakub or Tobias as this affects openmp.
[patch, fortran] PR 84487
Hello world, this patch makes sure that the __def_init variables, which have been generated for normal allocatable arrays for quite some time, do not fill up huge amounts of space in the object files with zeros. This is done by not marking them read-only, which means that they are put into the BSS. Setting DECL_ARTIFICIAL on the __def_init variable makes sure it is handled as predetermined shared in gfc_omp_predetermined_sharing . This is not an optimum solution. As the xfail shows, we are now missing out on an optimization (as seen by the xfail that is now needed), and having large all-zero variables seems wrong. However, this patch solves the most urgent problem in this respect. This is an 8/9/10 regression, so I would like to commit this to all of these branches (waiting before gcc 9 reopens, of course). I wold then close the PR and open an enchancement PR for the xfail and the design improvement. Test case... I'm not sure what to test for. Regression-tested. OK for all affected branches? Regards Thomas 2019-09-25 Thomas Koenig PR fortran/84487 * trans-decl.c (gfc_get_symbol_decl): For __def_init, set DECL_ARTIFICAL and do not set TREE_READONLY. 2019-09-25 Thomas Koenig PR fortran/84487 * gfortran.dg/typebound_call_22.f03: xfail. Index: fortran/trans-decl.c === --- fortran/trans-decl.c (Revision 275719) +++ fortran/trans-decl.c (Arbeitskopie) @@ -1911,9 +1911,13 @@ gfc_get_symbol_decl (gfc_symbol * sym) if (sym->attr.associate_var) GFC_DECL_ASSOCIATE_VAR_P (decl) = 1; + /* We no longer mark __def_init as read-only so it does not take up + space in the read-only section and dan go into the BSS instead, + see PR 84487. Marking this as artificial means that OpenMP will + treat this as predetermined shared. */ if (sym->attr.vtab || (sym->name[0] == '_' && gfc_str_startswith (sym->name, "__def_init"))) -TREE_READONLY (decl) = 1; +DECL_ARTIFICIAL (decl) = 1; return decl; } Index: testsuite/gfortran.dg/typebound_call_22.f03 === --- testsuite/gfortran.dg/typebound_call_22.f03 (Revision 275713) +++ testsuite/gfortran.dg/typebound_call_22.f03 (Arbeitskopie) @@ -26,4 +26,4 @@ program test call x%bar () end program -! { dg-final { scan-tree-dump-times "base \\(\\);" 1 "optimized" } } +! { dg-final { scan-tree-dump-times "base \\(\\);" 1 "optimized" { xfail *-*-* } } }
Re: [PATCH] PR fortran/91426: Colorize %L text to match diagnostic_show_locus
Hi David, does this look sane? Yes. OK for trunk, and thanks a lot! Regards Thomas
Re: [PATCH] PR fortran/91426: Colorize %L text to match diagnostic_show_locus
Hi David, I think technically I can self-approve this, but I'm not a day-to-day user of fortran; does this look sane? Very much so, I also find this more readable. I'd wait another day or so for comitting this, so that other people with different aesthetic sense can also chime in if they want to :-) Regards Thomas
Re: [patch, fortran] Fix ICE on invalid with DO steps
Hi Steve, s/previusly/previously Fixed, committed. Thanks for the review! Do you it gfortran should skip the front-end optimization pass if error(s) have already been reported? On entry ito this pass, you could test for the error count and simply return. I thought about this for doing this here. It's a front-end pass, but for error checking and warnings, not an optimization one, so I thought throwing a few additional errors / warnings would not hurt. Regards Thomas
[patch, fortran] Fix ICE on invalid with DO steps
Hello world, the attached patch fixes an ICE on invalid, where the fact that the step in do i = 1, 3, .1 is actually zero slipped through. Regression-tested. OK for all affected branches (trunk, 9 and 8)? Regards Thomas 2019-09-15 Thomas Koenig PR fortran/91550 * frontend-passes.c (do_subscript): If step equals zero, a previuos error has been reported; do nothing in this case. * resolve.c (gfc_resolve_iterator): Move error checking after type conversion. 2019-09-15 Thomas Koenig PR fortran/91550 * gfortran.dg/do_subscript_6.f90: New test. Index: frontend-passes.c === --- frontend-passes.c (Revision 275719) +++ frontend-passes.c (Arbeitskopie) @@ -2578,6 +2578,7 @@ do_subscript (gfc_expr **e) bool have_do_start, have_do_end; bool error_not_proven; int warn; + int sgn; dl = lp->c; if (dl == NULL) @@ -2606,7 +2607,16 @@ do_subscript (gfc_expr **e) Do not warn in this case. */ if (dl->ext.iterator->step->expr_type == EXPR_CONSTANT) - mpz_init_set (do_step, dl->ext.iterator->step->value.integer); + { + sgn = mpz_cmp_ui (dl->ext.iterator->step->value.integer, 0); + /* This can happen, but then the error has been + reported previusly. */ + if (sgn == 0) + continue; + + mpz_init_set (do_step, dl->ext.iterator->step->value.integer); + } + else continue; @@ -2632,9 +2642,8 @@ do_subscript (gfc_expr **e) /* No warning inside a zero-trip loop. */ if (have_do_start && have_do_end) { - int sgn, cmp; + int cmp; - sgn = mpz_cmp_ui (do_step, 0); cmp = mpz_cmp (do_end, do_start); if ((sgn > 0 && cmp < 0) || (sgn < 0 && cmp > 0)) break; Index: resolve.c === --- resolve.c (Revision 275719) +++ resolve.c (Arbeitskopie) @@ -7105,6 +7105,19 @@ gfc_resolve_iterator (gfc_iterator *iter, bool rea "Step expression in DO loop")) return false; + /* Convert start, end, and step to the same type as var. */ + if (iter->start->ts.kind != iter->var->ts.kind + || iter->start->ts.type != iter->var->ts.type) +gfc_convert_type (iter->start, &iter->var->ts, 1); + + if (iter->end->ts.kind != iter->var->ts.kind + || iter->end->ts.type != iter->var->ts.type) +gfc_convert_type (iter->end, &iter->var->ts, 1); + + if (iter->step->ts.kind != iter->var->ts.kind + || iter->step->ts.type != iter->var->ts.type) +gfc_convert_type (iter->step, &iter->var->ts, 1); + if (iter->step->expr_type == EXPR_CONSTANT) { if ((iter->step->ts.type == BT_INTEGER @@ -7118,19 +7131,6 @@ gfc_resolve_iterator (gfc_iterator *iter, bool rea } } - /* Convert start, end, and step to the same type as var. */ - if (iter->start->ts.kind != iter->var->ts.kind - || iter->start->ts.type != iter->var->ts.type) -gfc_convert_type (iter->start, &iter->var->ts, 1); - - if (iter->end->ts.kind != iter->var->ts.kind - || iter->end->ts.type != iter->var->ts.type) -gfc_convert_type (iter->end, &iter->var->ts, 1); - - if (iter->step->ts.kind != iter->var->ts.kind - || iter->step->ts.type != iter->var->ts.type) -gfc_convert_type (iter->step, &iter->var->ts, 1); - if (iter->start->expr_type == EXPR_CONSTANT && iter->end->expr_type == EXPR_CONSTANT && iter->step->expr_type == EXPR_CONSTANT) ! { dg-do compile } ! { dg-options "-std=legacy" } ! PR 91550 - this used to cause an ICE ! Test case by Gerhard Steinmetz program p real :: a(3) integer :: i do i = 1, 3, .1 ! { dg-error "cannot be zero" } a(i) = i end do end
[patch, fortran] Improve error messages for mismatched arguments
Hello world, the attached patch improves the rather hard to read error messages for argument mismatches. With this patch, this reads argument_checking_21.f90:7:11: 6 | call foo(1.0) ! { dg-warning "Rank mismatch" } | 2 7 | call foo(b) ! { dg-warning "Rank mismatch" } | 1 Fehler: Rank mismatch between actual argument at (1) and actual argument at (2) (scalar and rank-2) which I think is fairly clear. It also makes sure that warnings are always emitted by -fallow-argument-mismatch by removing -Wargument-mismatch. Finally, for people who do not want to have too many warnings cluttering up their logs, a type mismatch is only shown once if it is a warning. While I was going on about fixing warnings, I also fixed PR 91557 with the bit in trans-expr.c. This part is trivial, I will backport it to the other affected branches. After this, I think we can close PR 91556. Regression-tested. OK for trunk? 2019-09-13 Thomas Koenig PR fortran/91557 PR fortran/91556 * frontend-passes.c (check_externals_procedure): Reformat argument list. Use gfc_compare_actual_formal instead of gfc_procedure_use. * gfortran.h (gfc_symbol): Add flag error. * interface.c (gfc_compare_interfaces): Reformat. (argument_rank_mismatch): Add where_formal argument. If it is present, note that the error is between different calls. (compare_parameter): Change warnings that previously dependended on -Wargument-mismatch to unconditional. Issue an error / warning on type mismatch only once. Pass where_formal to argument_rank_mismatch for artificial variables. (compare_actual_formal): Change warnings that previously dependeded on -Wargument-mismatch to unconditional. (gfc_check_typebound_override): Likewise. (gfc_get_formal_from_actual_arglist): Set declared_at for artificial symbol. * invoke.texi: Extend description of -fallow-argument-mismatch. Delete -Wargument-mismatch. * lang.opt: Change -Wargument-mismatch to do-nothing option. * resolve.c (resolve_structure_cons): Change warnings that previously depended on -Wargument-mismatch to unconditional. * trans-decl.c (generate_local_decl): Do not warn if the symbol is artificial. 2019-09-13 Thomas Koenig PR fortran/91557 PR fortran/91556 * gfortran.dg/argument_checking_20.f90: New test. * gfortran.dg/argument_checking_21.f90: New test. * gfortran.dg/argument_checking_22.f90: New test. * gfortran.dg/argument_checking_23.f90: New test. * gfortran.dg/warn_unused_dummy_argument_5.f90: New test. * gfortran.dg/bessel_3.f90: Add pattern for type mismatch. * gfortran.dg/g77/20010519-1.f: Adjust dg-warning messages to new handling. * gfortran.dg/pr24823.f: Likewise. * gfortran.dg/pr39937.f: Likewise. Index: fortran/frontend-passes.c === --- fortran/frontend-passes.c (Revision 275713) +++ fortran/frontend-passes.c (Arbeitskopie) @@ -5373,7 +5373,8 @@ gfc_code_walker (gfc_code **c, walk_code_fn_t code /* Common tests for argument checking for both functions and subroutines. */ static int -check_externals_procedure (gfc_symbol *sym, locus *loc, gfc_actual_arglist *actual) +check_externals_procedure (gfc_symbol *sym, locus *loc, + gfc_actual_arglist *actual) { gfc_gsymbol *gsym; gfc_symbol *def_sym = NULL; @@ -5396,7 +5397,7 @@ static int if (def_sym) { - gfc_procedure_use (def_sym, &actual, loc); + gfc_compare_actual_formal (&actual, def_sym->formal, 0, 0, 0, loc); return 0; } Index: fortran/gfortran.h === --- fortran/gfortran.h (Revision 275713) +++ fortran/gfortran.h (Arbeitskopie) @@ -1610,6 +1610,9 @@ typedef struct gfc_symbol /* Set if this is a module function or subroutine with the abreviated declaration in a submodule. */ unsigned abr_modproc_decl:1; + /* Set if a previous error or warning has occurred and no other + should be reported. */ + unsigned error:1; int refs; struct gfc_namespace *ns; /* namespace containing this symbol */ Index: fortran/interface.c === --- fortran/interface.c (Revision 275713) +++ fortran/interface.c (Arbeitskopie) @@ -1807,9 +1807,9 @@ gfc_compare_interfaces (gfc_symbol *s1, gfc_symbol if (!compare_rank (f2->sym, f1->sym)) { if (errmsg != NULL) - snprintf (errmsg, err_len, "Rank mismatch in argument '%s' " - "(%i/%i)", f1->sym->name, symbol_rank (f1->sym), - symbol_rank (f2->sym)); + snprintf (errmsg, err_len, "Rank mismatch in argument " +
[patch, fortran] Introduce -fallow-argument-mismatch
Hello world, attached is a patch introducing the -fallow-argument-mismatch option, to separate this from the general kitchen sink -std=legacy. As discussed, this can only be turned off by -w. Regression-tested on powerpc64le-unknown-linux-gnu. Documentation tested by "make dvi" and "make pdf". OK for trunk? Regads Thomas 2019-08-25 Thomas Koenig PR fortran/91390 PR fortran/91473 * frontend-passes.c (gfc_check_externals): Make gfc_errors_to_warnings conditional on -fallow-argument-mismatch. * invoke.texi: Document -fallow-argument-mismatch. * lang.opt: Add -fallow-argument-mismatch. 2019-08-25 Thomas Koenig PR fortran/91390 PR fortran/91473 * gfortran.dg/used_before_typed_4.f90: Change warning to error. * gfortran.dg/argument_checking_20.f90: New test. Index: gcc/fortran/frontend-passes.c === --- gcc/fortran/frontend-passes.c (revision 274907) +++ gcc/fortran/frontend-passes.c (working copy) @@ -5477,9 +5477,9 @@ gfc_check_externals (gfc_namespace *ns) gfc_clear_error (); - /* Turn errors into warnings if -std=legacy is given by the user. */ + /* Turn errors into warnings if the user indicated this. */ - if (!pedantic && !(gfc_option.warn_std & GFC_STD_LEGACY)) + if (!pedantic && flag_allow_argument_mismatch) gfc_errors_to_warnings (true); gfc_code_walker (&ns->code, check_externals_code, check_externals_expr, NULL); Index: gcc/fortran/invoke.texi === --- gcc/fortran/invoke.texi (revision 274907) +++ gcc/fortran/invoke.texi (working copy) @@ -116,12 +116,12 @@ by type. Explanations are in the following sectio @table @emph @item Fortran Language Options @xref{Fortran Dialect Options,,Options controlling Fortran dialect}. -@gccoptlist{-fall-intrinsics -fallow-invalid-boz -fbackslash -fcray-pointer @gol --fd-lines-as-code -fd-lines-as-comments -fdec -fdec-structure @gol --fdec-intrinsic-ints -fdec-static -fdec-math -fdec-include @gol +@gccoptlist{-fall-intrinsics -fallow-argument-mismatch -fallow-invalid-boz @gol +-fbackslash -fcray-pointer -fd-lines-as-code -fd-lines-as-comments -fdec @gol +-fdec-structure-fdec-intrinsic-ints -fdec-static -fdec-math -fdec-include @gol -fdec-format-defaults -fdec-blank-format-item -fdefault-double-8 @gol -fdefault-integer-8 -fdefault-real-8 -fdefault-real-10 -fdefault-real-16 @gol --fdollar-ok @gol -ffixed-line-length-@var{n} -ffixed-line-length-none @gol +-fdollar-ok -ffixed-line-length-@var{n} -ffixed-line-length-none @gol -fpad-source -ffree-form -ffree-line-length-@var{n} -ffree-line-length-none @gol -fimplicit-none -finteger-4-integer-8 -fmax-identifier-length @gol -fmodule-private -ffixed-form -fno-range-check -fopenacc -fopenmp @gol @@ -232,6 +232,14 @@ available with @command{gfortran}. As a consequen will be ignored and no user-defined procedure with the same name as any intrinsic will be called except when it is explicitly declared @code{EXTERNAL}. +@item -fallow-argument-mismatch +@opindex @code{fallow-argument-mismatch} +Some code contains calls to external procedures whith mismatches +between the calls and the procedure definition, or with mismatches +between different calls. Such code is non-conforming, and will usually +be flagged with an error. This options degrades the error to a +warning. This option is implied by @option{-std=legacy}. + @item -fallow-invalid-boz @opindex @code{allow-invalid-boz} A BOZ literal constant can occur in a limited number of context in Index: gcc/fortran/lang.opt === --- gcc/fortran/lang.opt (revision 274907) +++ gcc/fortran/lang.opt (working copy) @@ -365,6 +365,10 @@ d Fortran Joined ; Documented in common.opt +fallow-argument-mismatch +Fortran Var(flag_allow_argument_mismatch) LangEnabledBy(Fortran,std=legacy) +Accept argument mismatches in procedure calls. + faggressive-function-elimination Fortran Var(flag_aggressive_function_elimination) Eliminate multiple function invocations also for impure functions. Index: gcc/testsuite/gfortran.dg/used_before_typed_4.f90 === --- gcc/testsuite/gfortran.dg/used_before_typed_4.f90 (revision 274907) +++ gcc/testsuite/gfortran.dg/used_before_typed_4.f90 (working copy) @@ -22,5 +22,5 @@ END SUBROUTINE test PROGRAM main IMPLICIT NONE INTEGER :: arr1(42), arr2(42) - CALL test (3, arr1, 2, arr2) ! { dg-warning "Type mismatch in argument" } + CALL test (3, arr1, 2, arr2) ! { dg-error "Type mismatch in argument" } END PROGRAM main ! { dg-do compile } ! { dg-options "-fallow-argument-mismatch" } ! PR 91390 - test the argument -fallow-argument-mismatch ! Original test case by Valera Veryazov. program test n
Re: [patch, fortran] Fix PR 91390 - treatment of extra parameter in a subroutine call
Am 20.08.19 um 22:32 schrieb Thomas König: here is the next installment of checking for mismatched calls, this time for mismatching CALLs. The reorganization of the code also means that https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91519 (a rejects-valid regression) is fixed by this patch. So, OK for trunk? Regards Thomas
What to do with argument mismatches in Fortran (was: [patch, fortran] Fix PR 91443)
I wrote: Committed as r274551. Well, this revision appears to have woken quite a few bugs from their slumber. While argument mismatch was always illegal, it seems to have been a common idiom at one time. And, like almost all bad habits of the past, SPEC also has this (see PR 91473, where you can see thatt a rather large number of SPEC tests now require -std=legacy). So, what to do? Is -std=legacy the right option, or should we add something different (like -faccept-argument-mismatch), which we could then set by -std=legacy? And is there anything special we should be doing after we have diagnoses the problem, if the user simply wants to run the code?
Re: [patch, fortran] Fix PR 91443
Hi Janne, The patch itself looks Ok. One worry, are you introducing an O(N**2)(?) algorithm (looping over all symbols for every symbol?), and does this cause performance issues when compiling some gigantic F77 project? This is a single pass over the code, so O(N) for the code size. The lookup is O(log M), where M is the number of global symbols in the file, so altogether, we're at O(N*log M), which should be OK. Thanks for the review. Committed as r274551. I will add the documentation about the change in behavior later. Regards Thomas
[patch, fortran] Fix PR 91443
Hello world, this patch fixes PR 91443, in which we did not warn about a mismatched external procedure. The problem was that the module this was called in was resolved before parsing of the procedure ever started. The approach taken here is to move the checking of external procedures to a stage after normal resolution. And, of course, fix the resulting fallout from regression-testing :-) There is also one policy change in the patch. Previously, we only warned about mismatched declarations. Now, this is a hard error unless the user specifies -std=legacy. The reason is that we have not yet solved our single declaration problem, but it cannot be solved unless all of a procedure's callers match. People who have such broken code should at least be made aware that they have a problem. However, I would like to have some sort of agreement on this point before the patch is committed. This can also be changed (see the code at the bottom of frontend-passes.c). Once this is in, the next step is to issue errors for mismatching calls where the callee is not in the same file. This can be done with the infrastructure of this patch. So, OK for trunk? Regards Thomas 2019-08-15 Thomas Koenig PR fortran/91443 * frontend-passes.c (check_externals_expr): New function. (check_externals_code): New function. (gfc_check_externals): New function. * gfortran.h (debug): Add prototypes for gfc_symbol * and gfc_expr *. (gfc_check_externals): Add prototype. * interface.c (compare_actual_formal): Do not complain about alternate returns if the formal argument is optional. (gfc_procedure_use): Handle cases when an error has been issued previously. Break long line. * parse.c (gfc_parse_file): Call gfc_check_externals for all external procedures. * resolve.c (resolve_global_procedure): Remove checking of argument list. 2019-08-15 Thomas Koenig PR fortran/91443 * gfortran.dg/argument_checking_19.f90: New test. * gfortran.dg/altreturn_10.f90: Change dg-warning to dg-error. * gfortran.dg/dec_union_11.f90: Add -std=legacy. * gfortran.dg/hollerith8.f90: Likewise. Remove warning for Hollerith constant. * gfortran.dg/integer_exponentiation_2.f90: New subroutine gee_i8; use it to avoid type mismatches. * gfortran.dg/pr41011.f: Add -std=legacy. * gfortran.dg/whole_file_1.f90: Change warnings to errors. * gfortran.dg/whole_file_2.f90: Likewise. Index: fortran/frontend-passes.c === --- fortran/frontend-passes.c (Revision 274394) +++ fortran/frontend-passes.c (Arbeitskopie) @@ -56,7 +56,6 @@ static gfc_expr* check_conjg_transpose_variable (g static int call_external_blas (gfc_code **, int *, void *); static int matmul_temp_args (gfc_code **, int *,void *data); static int index_interchange (gfc_code **, int*, void *); - static bool is_fe_temp (gfc_expr *e); #ifdef CHECKING_P @@ -5364,3 +5363,100 @@ gfc_code_walker (gfc_code **c, walk_code_fn_t code } return 0; } + +/* As a post-resolution step, check that all global symbols which are + not declared in the source file match in their call signatures. + We do this by looping over the code (and expressions). The first call + we happen to find is assumed to be canonical. */ + +/* Callback for external functions. */ + +static int +check_externals_expr (gfc_expr **ep, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + gfc_expr *e = *ep; + gfc_symbol *sym, *def_sym; + gfc_gsymbol *gsym; + + if (e->expr_type != EXPR_FUNCTION) +return 0; + + sym = e->value.function.esym; + + if (sym == NULL || sym->attr.is_bind_c) +return 0; + + if (sym->attr.proc != PROC_EXTERNAL && sym->attr.proc != PROC_UNKNOWN) +return 0; + + gsym = gfc_find_gsymbol (gfc_gsym_root, sym->name); + if (gsym == NULL) +return 0; + + gfc_find_symbol (sym->name, gsym->ns, 0, &def_sym); + + if (sym && def_sym) +gfc_procedure_use (def_sym, &e->value.function.actual, &e->where); + + return 0; +} + +/* Callback for external code. */ + +static int +check_externals_code (gfc_code **c, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + gfc_code *co = *c; + gfc_symbol *sym, *def_sym; + gfc_gsymbol *gsym; + + if (co->op != EXEC_CALL) +return 0; + + sym = co->resolved_sym; + if (sym == NULL || sym->attr.is_bind_c) +return 0; + + if (sym->attr.proc != PROC_EXTERNAL && sym->attr.proc != PROC_UNKNOWN) +return 0; + + if (sym->attr.if_source == IFSRC_IFBODY || sym->attr.if_source == IFSRC_DECL) +return 0; + + gsym = gfc_find_gsymbol (gfc_gsym_root, sym->name); + if (gsym == NULL) +return 0; + + gfc_find_symbol (sym->name,
[patch, fortran, committed] Fix PR 90563, error while warning about do subscripts
Hi, I just committed as simple and obvious the patch below. This fixes a 8/9/10 regression where a false positive with -Wdo-subscript (which we know about) was compounded by an also invalid error. Fixed by suppressing errors at the right time. I will also commit the patch to the other affected branches. Regards Thomas 2013-08-13 Thomas Koenig PR fortran/90563 * frontend-passes.c (insert_index): Suppress errors while simplifying the resulting expression. 2013-08-13 Thomas Koenig PR fortran/90563 * gfortran.dg/do_subsript_5.f90: New test. Index: testsuite/gfortran.dg/do_subscript_5.f90 === --- testsuite/gfortran.dg/do_subscript_5.f90 (Revision 274394) +++ testsuite/gfortran.dg/do_subscript_5.f90 (Arbeitskopie) @@ -1,4 +1,5 @@ ! { dg-do compile } +! { dg-additional-options "-Wdo-subscript" } ! PR 90563 - this used to be rejected, wrongly ! Original test case by Tobias Neumann program test @@ -9,9 +10,11 @@ p = 0.0 - do j=1,6 + ! The following warnings are actually bogus, but we are not yet + ! clever enough to suppress them. + do j=1,6 ! { dg-warning "out of bounds" } if (j<5) then - p(j) = p(swap(j)) + p(j) = p(swap(j)) ! { dg-warning "out of bounds" } endif enddo end program ! { dg-do compile } ! { dg-additional-options "-Wdo-subscript" } ! PR 90563 - this used to be rejected, wrongly ! Original test case by Tobias Neumann program test implicit none integer, parameter :: swap(4) = [2,1,3,4] real :: p(20) integer :: j p = 0.0 ! The following warnings are actually bogus, but we are not yet ! clever enough to suppress them. do j=1,6 ! { dg-warning "out of bounds" } if (j<5) then p(j) = p(swap(j)) ! { dg-warning "out of bounds" } endif enddo end program
Re: [patch, fortran] Some corrections for DO loop index warnings
Hi Steve, Ok. If it regression cleanly on gcc9, go ahead an commit there as well. Committed to both (after doing a regression test on gcc 9 and also waiting for gcc-testresults containing the revision). Thanks for the review!
[patch, fortran] Some corrections for DO loop index warnings
Hello world, the attached patch fixes three problems with DO loop index warnings: - DO loops in contained procedures were not checked - Zero-trip loops gave a false positive - DO loops in blocks gave the same warning twice plus it fixes the resulting fallout from the test suite. Regression-tested. OK for trunk? I also think that this is something that could be safely backported to gcc-9. 2019-08-12 Thomas Koenig PR fortran/91424 * frontend-passes.c (do_subscript): Do not warn for an expression a second time. Do not warn about a zero-trip loop. (doloop_warn): Also look at contained namespaces. 2019-08-12 Thomas Koenig PR fortran/91424 * gfortran.dg/do_subscript_3.f90: New test. * gfortran.dg/do_subscript_4.f90: New test. * gfortran.dg/pr70754.f90: Use indices that to not overflow. 2019-08-12 Thomas Koenig PR fortran/91422 * testsuite/libgomp.oacc-fortran/routine-7.f90: Correct array dimension. ! { dg-do compile } ! PR fortran/91424 ! Check that only one warning is issued inside blocks, and that ! warnings are also issued for contained subroutines. program main real :: a(5) block integer :: j do j=0, 5 ! { dg-warning "out of bounds" } a(j) = 2. ! { dg-warning "out of bounds" } end do end block call x contains subroutine x integer :: i do i=1,6 ! { dg-warning "out of bounds" } a(i) = 2. ! { dg-warning "out of bounds" } end do end subroutine x end program main Index: gcc/fortran/frontend-passes.c === --- gcc/fortran/frontend-passes.c (revision 273855) +++ gcc/fortran/frontend-passes.c (working copy) @@ -2556,6 +2556,12 @@ do_subscript (gfc_expr **e) if (in_assoc_list) return 0; + /* We already warned about this. */ + if (v->do_not_warn) +return 0; + + v->do_not_warn = 1; + for (ref = v->ref; ref; ref = ref->next) { if (ref->type == REF_ARRAY && ref->u.ar.type == AR_ELEMENT) @@ -2608,7 +2614,6 @@ do_subscript (gfc_expr **e) else have_do_start = false; - if (dl->ext.iterator->end->expr_type == EXPR_CONSTANT) { have_do_end = true; @@ -2620,6 +2625,17 @@ do_subscript (gfc_expr **e) if (!have_do_start && !have_do_end) return 0; + /* No warning inside a zero-trip loop. */ + if (have_do_start && have_do_end) + { + int sgn, cmp; + + sgn = mpz_cmp_ui (do_step, 0); + cmp = mpz_cmp (do_end, do_start); + if ((sgn > 0 && cmp < 0) || (sgn < 0 && cmp > 0)) + break; + } + /* May have to correct the end value if the step does not equal one. */ if (have_do_start && have_do_end && mpz_cmp_ui (do_step, 1) != 0) @@ -2761,6 +2777,12 @@ static void doloop_warn (gfc_namespace *ns) { gfc_code_walker (&ns->code, doloop_code, do_function, NULL); + + for (ns = ns->contained; ns; ns = ns->sibling) +{ + if (ns->code == NULL || ns->code->op != EXEC_BLOCK) + doloop_warn (ns); +} } /* This selction deals with inlining calls to MATMUL. */ Index: gcc/testsuite/gfortran.dg/pr70754.f90 === --- gcc/testsuite/gfortran.dg/pr70754.f90 (revision 273855) +++ gcc/testsuite/gfortran.dg/pr70754.f90 (working copy) @@ -18,12 +18,13 @@ contains integer (ii4), dimension(40,40) :: c integer i, j -do i=1,20 - b(i,j) = 123 * a(i,j) + 34 * a(i,j+1) & - + 34 * a(i,j-1) + a(i+1,j+1) & - + a(i+1,j-1) + a(i-1,j+1) & - + a(i-1,j-1) - c(i,j) = 123 +j = 10 +do i=11,30 + b(i,j) = 123 * a(i,j) + 34 * a(i,j+1) & ++ 34 * a(i,j-1) + a(i+1,j+1) & ++ a(i+1,j-1) + a(i-1,j+1) & ++ a(i-1,j-1) + c(i,j) = 123 end do where ((xyz(:,:,2) /= 0) .and. (c /= 0)) Index: libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90 === --- libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90 (revision 273855) +++ libgomp/testsuite/libgomp.oacc-fortran/routine-7.f90 (working copy) @@ -109,7 +109,7 @@ end subroutine gang subroutine seq (a) !$acc routine seq - integer, intent (inout) :: a(M) + integer, intent (inout) :: a(N) integer :: i do i = 1, N ! { dg-do compile } ! PR 91424 - this used to warn although the DO loop is zero trip. program main implicit none integer :: i real :: a(2) do i=1,3,-1 a(i) = 2. end do print *,a end program main
Re: [PATCH,Fortran] -- Tidy up the BOZ rewrite
Hi Steve, OK to commit? OK with %s/rational/relational/g and with Mark's nit. Thanks a lot for going down that road. I like the approach of -fallow-invalid-boz to downgrade the error to a warning, which then cannot be turned off. Regards Thomas
Re: [PATCH] PR fortran/91296 -- Prevent ICE in aliasing check
Hi Steve, The attach patch has been regression tested on x86_64-*-freebsd. There were no regression. The patch prevents an ICE when checking for aliasing and the actual arguments are the real and imaginary parts of a complex entity. See the testcase for more information. OK to commit to both trunk and 9-branch? OK. Thanks for the patch! Regards Thomas
Re: [patch, fortran] Fix PR 90813
Hi Paul, That is very well done. Thanks for picking it up and running with it. OK on both the fix and the dumping of the gsymbols. Committed, thanks. You might consider back porting both this patch and my fix for the original bug to 9-branch. Both patches apply cleanly together on gcc-9 (well, except for the whitespace fix of a comment that didn't exist yet in gcc-9, so I think we can ignore than), and pass regression-tests on POWER9. So, I will commit this tonight. Regards Thomas
[patch, fortran] Fix PR 90813
Hello world, the attached patch fixes PR 90813, a regression with proc pointers. The problem was quite complex, and I'd like to thank the people who helped debug this; the most important clue came from Richard. The problem was that, for a procedure pointer variable declared in a module in the same file, we were using a different backend decl in the module than in the main program. This led to the later parts of the compiler to think that the procedure pointer was actually two variables which could not alias. Optimization on some architectures such as Aarch64 and POWER (but not on x86_64) then led to reordering of stores, leading to a segfault. The solution is to put the mangled names into the global variable table, and to look for it when getting its backend declaration. While debugging it, I also put in an option to dump the global symbol table to standard output. I have included this in this patch because I think this may not be the last bug in that area :-) Regression-tested on powerpc64le-unknown-linux-gnu, where the segfault showed up. No test case because is is already in the test suite. Doc changes checked with "make dvi" and "make pdf". OK for trunk? Regards Thomas 2019-07-28 Thomas Koenig PR fortran/90813 * dump-parse-tree.c (show_global_symbol): New function. (gfc_dump_global_symbols): New function. * gfortran.h (gfc_traverse_gsymbol): Add prototype. (gfc_dump_global_symbols): Likewise. * invoke.texi: Document -fdump-fortran-global. * lang.opt: Add -fdump-fortran-global. * parse.c (gfc_parse_file): Handle flag_dump_fortran_global. * symbol.c (gfc_traverse_gsymbol): New function. * trans-decl.c (sym_identifier): New function. (mangled_identifier): New function, doing most of the work of gfc_sym_mangled_identifier. (gfc_sym_mangled_identifier): Use mangled_identifier. Add mangled identifier to global symbol table. (get_proc_pointer_decl): Use backend decl from global identifier if present. Index: dump-parse-tree.c === --- dump-parse-tree.c (Revision 273855) +++ dump-parse-tree.c (Arbeitskopie) @@ -3462,3 +3462,36 @@ write_interop_decl (gfc_symbol *sym) else if (sym->attr.flavor == FL_PROCEDURE) write_proc (sym, true); } + +/* This section deals with dumping the global symbol tree. */ + +/* Callback function for printing out the contents of the tree. */ + +static void +show_global_symbol (gfc_gsymbol *gsym, void *f_data) +{ + FILE *out; + out = (FILE *) f_data; + + if (gsym->name) +fprintf (out, "name=%s", gsym->name); + + if (gsym->sym_name) +fprintf (out, ", sym_name=%s", gsym->sym_name); + + if (gsym->mod_name) +fprintf (out, ", mod_name=%s", gsym->mod_name); + + if (gsym->binding_label) +fprintf (out, ", binding_label=%s", gsym->binding_label); + + fputc ('\n', out); +} + +/* Show all global symbols. */ + +void +gfc_dump_global_symbols (FILE *f) +{ + gfc_traverse_gsymbol (gfc_gsym_root, show_global_symbol, (void *) f); +} Index: gfortran.h === --- gfortran.h (Revision 273855) +++ gfortran.h (Arbeitskopie) @@ -3128,6 +3128,7 @@ void gfc_enforce_clean_symbol_state (void); gfc_gsymbol *gfc_get_gsymbol (const char *, bool bind_c); gfc_gsymbol *gfc_find_gsymbol (gfc_gsymbol *, const char *); gfc_gsymbol *gfc_find_case_gsymbol (gfc_gsymbol *, const char *); +void gfc_traverse_gsymbol (gfc_gsymbol *, void (*)(gfc_gsymbol *, void *), void *); gfc_typebound_proc* gfc_get_typebound_proc (gfc_typebound_proc*); gfc_symbol* gfc_get_derived_super_type (gfc_symbol*); @@ -3471,6 +3472,7 @@ void gfc_delete_bbt (void *, void *, compare_fn); void gfc_dump_parse_tree (gfc_namespace *, FILE *); void gfc_dump_c_prototypes (gfc_namespace *, FILE *); void gfc_dump_external_c_prototypes (FILE *); +void gfc_dump_global_symbols (FILE *); /* parse.c */ bool gfc_parse_file (void); Index: invoke.texi === --- invoke.texi (Revision 273855) +++ invoke.texi (Arbeitskopie) @@ -157,7 +157,8 @@ and warnings}. @item Debugging Options @xref{Debugging Options,,Options for debugging your program or GNU Fortran}. @gccoptlist{-fbacktrace -fdump-fortran-optimized -fdump-fortran-original @gol --fdump-parse-tree -ffpe-trap=@var{list} -ffpe-summary=@var{list} +-fdump-fortran-global -fdump-parse-tree -ffpe-trap=@var{list} @gol +-ffpe-summary=@var{list} } @item Directory Options @@ -1199,6 +1200,14 @@ change between releases. This option may also gene compiler errors for features which have only recently been added. This option is deprecated; use @code{-fdump-fortran-original} instead. +@item -fdump-fortran-global +@opindex @code{fdump-fort
Re: [patch, fortran] Improve dependency checking
Hi Steve, Ah, I don't speak C++, and didn't know one could corrupt a C prototype in this manner. A quick glance of gfortran.h indeed shows a few more occurences of "bool xxx = false". I suppose the patch is then OK. I don't use many C++ features, but I use this one because I feel it should really be in C :-) PS: watch for long lines. Corrected and committed as r273807. Thanks for the review! Regards Thomas
Re: [patch, fortran] Improve dependency checking
Hi Steve, -int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *); +int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool identical = false); This is changing the prototype. I would expect to see int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool); Usig C++'s optional arguments is actually quite useful, it's used already used in a few places in the front end. The idea is that you don't need to touch the other callers, just the ones where the new argument matters. However, in this particular case, it would also be possible to ajust all other callers (exactly one), if you prefer. Regards Thomas
[patch, fortran] Improve dependency checking
Hello world, the attached pach does some more work in gfc_check_dependency for the case where an identity between arguments would also lead to problems. It does not lead to removal of the warning with -Warray-temporaries, because that is still generated by the call to library function. Instead, I checked for the names of the variables which used to be introduced by the matmul inlining code. Regression-tested. OK for trunk? Regards Thomas 2019-07-25 Thomas Koenig PR fortran/65819 * dependency.h (gfc_dep_resovler): Add optional argument identical. * dependency.c (gfc_check_dependency): Do not alway return 1 if the symbol is the same. Pass on identical to gfc_dep_resolver. (gfc_check_element_vs_element): Whitespace fix. (gfc_dep_resolver): Adjust comment for function. If identical is true, return 1 if any overlap has been found. 2019-07-25 Thomas Koenig PR fortran/65819 * gfortran.dg/dependency_54.f90: New test. Index: dependency.c === --- dependency.c (Revision 273733) +++ dependency.c (Arbeitskopie) @@ -1351,13 +1351,10 @@ gfc_check_dependency (gfc_expr *expr1, gfc_expr *e return 0; } - if (identical) - return 1; - /* Identical and disjoint ranges return 0, overlapping ranges return 1. */ if (expr1->ref && expr2->ref) - return gfc_dep_resolver (expr1->ref, expr2->ref, NULL); + return gfc_dep_resolver (expr1->ref, expr2->ref, NULL, identical); return 1; @@ -1884,6 +1881,7 @@ gfc_check_element_vs_element (gfc_ref *lref, gfc_r if (i > -2) return GFC_DEP_NODEP; + return GFC_DEP_EQUAL; } @@ -2084,13 +2082,15 @@ ref_same_as_full_array (gfc_ref *full_ref, gfc_ref /* Finds if two array references are overlapping or not. Return value - 2 : array references are overlapping but reversal of one or + 2 : array references are overlapping but reversal of one or more dimensions will clear the dependency. - 1 : array references are overlapping. - 0 : array references are identical or not overlapping. */ + 1 : array references are overlapping, or identical is true and + there is some kind of overlap. + 0 : array references are identical or not overlapping. */ int -gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse) +gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gfc_reverse *reverse, + bool identical) { int n; int m; @@ -2124,11 +2124,15 @@ int case REF_ARRAY: + /* For now, treat all coarrays as dangerous. */ + if (lref->u.ar.codimen || rref->u.ar.codimen) + return 1; + if (ref_same_as_full_array (lref, rref)) - return 0; + return identical; if (ref_same_as_full_array (rref, lref)) - return 0; + return identical; if (lref->u.ar.dimen != rref->u.ar.dimen) { @@ -2180,6 +2184,8 @@ int gcc_assert (rref->u.ar.dimen_type[n] == DIMEN_ELEMENT && lref->u.ar.dimen_type[n] == DIMEN_ELEMENT); this_dep = gfc_check_element_vs_element (rref, lref, n); + if (identical && this_dep == GFC_DEP_EQUAL) + this_dep = GFC_DEP_OVERLAP; } /* If any dimension doesn't overlap, we have no dependency. */ @@ -2240,6 +2246,9 @@ int know the worst one.*/ update_fin_dep: + if (identical && this_dep == GFC_DEP_EQUAL) + this_dep = GFC_DEP_OVERLAP; + if (this_dep > fin_dep) fin_dep = this_dep; } @@ -2253,7 +2262,7 @@ int /* Exactly matching and forward overlapping ranges don't cause a dependency. */ - if (fin_dep < GFC_DEP_BACKWARD) + if (fin_dep < GFC_DEP_BACKWARD && !identical) return 0; /* Keep checking. We only have a dependency if @@ -2267,11 +2276,14 @@ int rref = rref->next; } + /* Assume the worst if we nest to different depths. */ + if (lref || rref) +return 1; + /* If we haven't seen any array refs then something went wrong. */ gcc_assert (fin_dep != GFC_DEP_ERROR); - /* Assume the worst if we nest to different depths. */ - if (lref || rref) + if (identical && fin_dep != GFC_DEP_NODEP) return 1; return fin_dep == GFC_DEP_OVERLAP; Index: dependency.h === --- dependency.h (Revision 273733) +++ dependency.h (Arbeitskopie) @@ -37,7 +37,7 @@ int gfc_check_fncall_dependency (gfc_expr *, sym_i int gfc_check_dependency (gfc_expr *, gfc_expr *, bool); int gfc_expr_is_one (gfc_expr *, int); -int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *); +int gfc_dep_resolver(gfc_ref *, gfc_ref *, gfc_reverse *, bool identical = false); int gfc_are_equivalenced_arrays (gfc_expr *, gfc_expr *); gfc_expr * gfc_discard_nops (gfc_expr *); ! { dg-do run } ! { dg-additional-options "-fdump-tree-original
Re: [patch, libfortran] Adjust block size for libgfortran for unformatted reads
Hi Steve, On Sun, Jul 14, 2019 at 12:07:58PM +0200, Thomas Koenig wrote: OK, so here is a new version. I think the discussion has shown that enlaring the buffer makes sense, and that the buffer size for unformatted seems to be too bad. I've reversed the names of the environment variables according to Behnard's suggestion. So, OK for trunk? Also, what should we do about gcc-9? I have now come to think that we should add the environment variables to set the buffer lengths, but leave the old default (8192). What do you think? If you are inclined to back port a portion of the patch to 9-branch, then bumping up the old default would seem to be the most important part. As dje noted, users seem to have an aversion to reading the documentation, so finding the environment variables may not happen. Isn't 8192 an internal implementation detail for libgfortran? Can bumping it to larger value in 9-branch cause an issue for a normal user? Well, it allocates a bigger memory block, that's all. Upon reconsideration, I think your point about people not reading the docs is valid :-| So, I will commit the patch to trunk over the weekend and to 9.2 a few days afterwards, unless somebody objects. Regards Thomas
Re: [patch, libfortran] Adjust block size for libgfortran for unformatted reads
... of course, better with the actual patch. Index: gcc/fortran/gfortran.texi === --- gcc/fortran/gfortran.texi (Revision 273183) +++ gcc/fortran/gfortran.texi (Arbeitskopie) @@ -611,6 +611,8 @@ Malformed environment variables are silently ignor * GFORTRAN_LIST_SEPARATOR:: Separator for list output * GFORTRAN_CONVERT_UNIT:: Set endianness for unformatted I/O * GFORTRAN_ERROR_BACKTRACE:: Show backtrace on run-time errors +* GFORTRAN_FORMATTED_BUFFER_SIZE:: Buffer size for formatted files. +* GFORTRAN_UNFORMATTED_BUFFER_SIZE:: Buffer size for unformatted files. @end menu @node TMPDIR @@ -782,6 +784,20 @@ the backtracing, set the variable to @samp{n}, @sa Default is to print a backtrace unless the @option{-fno-backtrace} compile option was used. +@node GFORTRAN_FORMATTED_BUFFER_SIZE +@section @env{GFORTRAN_FORMATTED_BUFFER_SIZE}---Set buffer size for formatted I/O + +The @env{GFORTRAN_FORMATTED_BUFFER_SIZE} environment variable +specifies buffer size in bytes to be used for formatted output. +The default value is 8192. + +@node GFORTRAN_UNFORMATTED_BUFFER_SIZE +@section @env{GFORTRAN_UNFORMATTED_BUFFER_SIZE}---Set buffer size for unformatted I/O + +The @env{GFORTRAN_UNFORMATTED_BUFFER_SIZE} environment variable +specifies buffer size in bytes to be used for unformatted output. +The default value is 131072. + @c = @c PART II: LANGUAGE REFERENCE @c = Index: libgfortran/io/unix.c === --- libgfortran/io/unix.c (Revision 273183) +++ libgfortran/io/unix.c (Arbeitskopie) @@ -193,7 +193,8 @@ fallback_access (const char *path, int mode) /* Unix and internal stream I/O module */ -static const int BUFFER_SIZE = 8192; +static const int FORMATTED_BUFFER_SIZE_DEFAULT = 8192; +static const int UNFORMATTED_BUFFER_SIZE_DEFAULT = 128*1024; typedef struct { @@ -205,6 +206,7 @@ typedef struct gfc_offset file_length; /* Length of the file. */ char *buffer; /* Pointer to the buffer. */ + ssize_t buffer_size; /* Length of the buffer. */ int fd; /* The POSIX file descriptor. */ int active; /* Length of valid bytes in the buffer */ @@ -592,9 +594,9 @@ buf_read (unix_stream *s, void *buf, ssize_t nbyte && raw_seek (s, new_logical, SEEK_SET) < 0) return -1; s->buffer_offset = s->physical_offset = new_logical; - if (to_read <= BUFFER_SIZE/2) + if (to_read <= s->buffer_size/2) { - did_read = raw_read (s, s->buffer, BUFFER_SIZE); + did_read = raw_read (s, s->buffer, s->buffer_size); if (likely (did_read >= 0)) { s->physical_offset += did_read; @@ -632,11 +634,11 @@ buf_write (unix_stream *s, const void *buf, ssize_ s->buffer_offset = s->logical_offset; /* Does the data fit into the buffer? As a special case, if the - buffer is empty and the request is bigger than BUFFER_SIZE/2, + buffer is empty and the request is bigger than s->buffer_size/2, write directly. This avoids the case where the buffer would have to be flushed at every write. */ - if (!(s->ndirty == 0 && nbyte > BUFFER_SIZE/2) - && s->logical_offset + nbyte <= s->buffer_offset + BUFFER_SIZE + if (!(s->ndirty == 0 && nbyte > s->buffer_size/2) + && s->logical_offset + nbyte <= s->buffer_offset + s->buffer_size && s->buffer_offset <= s->logical_offset && s->buffer_offset + s->ndirty >= s->logical_offset) { @@ -651,7 +653,7 @@ buf_write (unix_stream *s, const void *buf, ssize_ the request is bigger than the buffer size, write directly bypassing the buffer. */ buf_flush (s); - if (nbyte <= BUFFER_SIZE/2) + if (nbyte <= s->buffer_size/2) { memcpy (s->buffer, buf, nbyte); s->buffer_offset = s->logical_offset; @@ -688,7 +690,7 @@ buf_write (unix_stream *s, const void *buf, ssize_ static int buf_markeor (unix_stream *s) { - if (s->unbuffered || s->ndirty >= BUFFER_SIZE / 2) + if (s->unbuffered || s->ndirty >= s->buffer_size / 2) return buf_flush (s); return 0; } @@ -765,11 +767,32 @@ static const struct stream_vtable buf_vtable = { }; static int -buf_init (unix_stream *s) +buf_init (unix_stream *s, bool unformatted) { s->st.vptr = &buf_vtable; - s->buffer = xmalloc (BUFFER_SIZE); + /* Try to guess a good value for the buffer size. For formatted + I/O, we use so many CPU cycles converting the data that there is + more sense in converving memory and especially cache. For + unformatted, a bigger block can have a large impact in some + environments. */ + + if (unformatted) +{ + if (options.unformatted_buffer_size > 0) + s->buffer_size = options.unformatted_buffer_size; +
Re: [patch, libfortran] Adjust block size for libgfortran for unformatted reads
OK, so here is a new version. I think the discussion has shown that enlaring the buffer makes sense, and that the buffer size for unformatted seems to be too bad. I've reversed the names of the environment variables according to Behnard's suggestion. So, OK for trunk? Also, what should we do about gcc-9? I have now come to think that we should add the environment variables to set the buffer lengths, but leave the old default (8192). What do you think? Regards Thomas 2019-07-14 Thomas König PR libfortran/91030 * gfortran.texi (GFORTRAN_FORMATTED_BUFFER_SIZE): Document (GFORTRAN_UNFORMATTED_BUFFER_SIZE): Likewise. 2019-07-14 Thomas König PR libfortran/91030 * io/unix.c (BUFFER_SIZE): Delete. (BUFFER_FORMATTED_SIZE_DEFAULT): New variable. (BUFFER_UNFORMATTED_SIZE_DEFAULT): New variable. (unix_stream): Add buffer_size. (buf_read): Use s->buffer_size instead of BUFFER_SIZE. (buf_write): Likewise. (buf_init): Add argument unformatted. Handle block sizes for unformatted vs. formatted, using defaults if provided. (fd_to_stream): Add argument unformatted in call to buf_init. * libgfortran.h (options_t): Add buffer_size_formatted and buffer_size_unformatted. * runtime/environ.c (variable_table): Add GFORTRAN_UNFORMATTED_BUFFER_SIZE and GFORTRAN_FORMATTED_BUFFER_SIZE.
[patch, libfortran] Adjust block size for libgfortran for unformatted reads
Hello world, the attached patch sets the I/O block size for unformatted files to 2**17 and makes this, and the block size for formatted files, adjustable via environment variables. The main reason is that -fconvert=big-endian was quite slow on some HPC systems. A bigger buffer should eliminate that. Also, People who use unformatted files are likely to write large amounts of data, so this seems like a good fit. Finally, some benchmarking showed that 131072 seemed like a good value to use. Thanks to Jerry for support. I didn't change the value for formatted files because, frankly, we are using a lot of CPU for converting numbers there, so any gain negligible (unless somebody comes up with a benchmark which says otherwise). As this is a change in behavior / new feature, I don't think that backporting is indicated, but if somebody feels otherwise, please speak up. Regression-tested. OK for trunk? Regards Thomas 2019-07-07 Thomas König PR libfortran/91030 * gfortran.texi (GFORTRAN_BUFFER_SIZE_FORMATTED): Document (GFORTRAN_BUFFER_SIZE_FORMATTED): Likewise. 2019-07-07 Thomas König PR libfortran/91030 * io/unix.c (BUFFER_SIZE): Delete. (BUFFER_SIZE_FORMATTED_DEFAULT): New variable. (BUFFER_SIZE_UNFORMATTED_DEFAULT): New variable. (unix_stream): Add buffer_size. (buf_read): Use s->buffer_size instead of BUFFER_SIZE. (buf_write): Likewise. (buf_init): Add argument unformatted. Handle block sizes for unformatted vs. formatted, using defaults if provided. (fd_to_stream): Add argument unformatted in call to buf_init. * libgfortran.h (options_t): Add buffer_size_formatted and buffer_size_unformatted. * runtime/environ.c (variable_table): Add GFORTRAN_BUFFER_SIZE_UNFORMATTED and GFORTRAN_BUFFER_SIZE_FORMATTED. Index: gcc/fortran/gfortran.texi === --- gcc/fortran/gfortran.texi (Revision 273183) +++ gcc/fortran/gfortran.texi (Arbeitskopie) @@ -611,6 +611,8 @@ Malformed environment variables are silently ignor * GFORTRAN_LIST_SEPARATOR:: Separator for list output * GFORTRAN_CONVERT_UNIT:: Set endianness for unformatted I/O * GFORTRAN_ERROR_BACKTRACE:: Show backtrace on run-time errors +* GFORTRAN_BUFFER_SIZE_FORMATTED:: Buffer size for formatted files. +* GFORTRAN_BUFFER_SIZE_UNFORMATTED:: Buffer size for unformatted files. @end menu @node TMPDIR @@ -782,6 +784,20 @@ the backtracing, set the variable to @samp{n}, @sa Default is to print a backtrace unless the @option{-fno-backtrace} compile option was used. +@node GFORTRAN_BUFFER_SIZE_FORMATTED +@section @env{GFORTRAN_BUFFER_SIZE_FORMATTED}---Set buffer size for formatted I/O + +The @env{GFORTRAN_BUFFER_SIZE_FORMATTED} environment variable +specifies buffer size in bytes to be used for formatted output. +The default value is 8192. + +@node GFORTRAN_BUFFER_SIZE_UNFORMATTED +@section @env{GFORTRAN_BUFFER_SIZE_UNFORMATTED}---Set buffer size for unformatted I/O + +The @env{GFORTRAN_BUFFER_SIZE_UNFORMATTED} environment variable +specifies buffer size in bytes to be used for unformatted output. +The default value is 131072. + @c = @c PART II: LANGUAGE REFERENCE @c = Index: libgfortran/io/unix.c === --- libgfortran/io/unix.c (Revision 273183) +++ libgfortran/io/unix.c (Arbeitskopie) @@ -193,7 +193,8 @@ fallback_access (const char *path, int mode) /* Unix and internal stream I/O module */ -static const int BUFFER_SIZE = 8192; +static const int BUFFER_SIZE_FORMATTED_DEFAULT = 8192; +static const int BUFFER_SIZE_UNFORMATTED_DEFAULT = 128*1024; typedef struct { @@ -205,6 +206,7 @@ typedef struct gfc_offset file_length; /* Length of the file. */ char *buffer; /* Pointer to the buffer. */ + ssize_t buffer_size; /* Length of the buffer. */ int fd; /* The POSIX file descriptor. */ int active; /* Length of valid bytes in the buffer */ @@ -592,9 +594,9 @@ buf_read (unix_stream *s, void *buf, ssize_t nbyte && raw_seek (s, new_logical, SEEK_SET) < 0) return -1; s->buffer_offset = s->physical_offset = new_logical; - if (to_read <= BUFFER_SIZE/2) + if (to_read <= s->buffer_size/2) { - did_read = raw_read (s, s->buffer, BUFFER_SIZE); + did_read = raw_read (s, s->buffer, s->buffer_size); if (likely (did_read >= 0)) { s->physical_offset += did_read; @@ -632,11 +634,11 @@ buf_write (unix_stream *s, const void *buf, ssize_ s->buffer_offset = s->logical_offset; /* Does the data fit into the buffer? As a special case, if the - buffer is empty and the request is bigger than BUFFER_SIZE/2, +
Re: [RFC] zstd as a compression algorithm for LTO
Hi Martin, LTO bytecode is not supposed to be a distributable format. One of my dreams is to make libgfortran LTO-clean. There is a lot of performance to be gained both in I/O (where a huge number of special cases could be shortcut by LTO, because hardly any program uses them all) and in array intrinsics, where seeing through the array descriptors can also lead to large benefits. This is PR 77278. Once this is achieved, it would make sense to distribute libgfortran.a as a library of fat object files. Regards Thomas
[patch, committed] Fix for PR 90937
Hello world, I have committed the attached patch to trunk as obvious to fix another of the regressions stemming from the "make up formal from actual arglist" patch, as obvious and simple. I will backport this patch to the other affected branches, probably over the weekend. Regards Thomas 2019-06-20 Thomas Koenig PR fortran/90937 * trans-types.c (get_formal_from_actual_arglist): Get symbol from current namespace so it will be freed later. If symbol is of type character, get an empty character length. 2019-06-20 Thomas Koenig PR fortran/90937 * gfortran.dg/external_procedure_4.f90: New test. Index: trans-types.c === --- trans-types.c (Revision 272479) +++ trans-types.c (Arbeitskopie) @@ -2997,7 +2997,7 @@ get_formal_from_actual_arglist (gfc_symbol *sym, g if (a->expr) { snprintf (name, GFC_MAX_SYMBOL_LEN, "_formal_%d", var_num ++); - gfc_get_symbol (name, NULL, &s); + gfc_get_symbol (name, gfc_current_ns, &s); if (a->expr->ts.type == BT_PROCEDURE) { s->attr.flavor = FL_PROCEDURE; @@ -3005,6 +3005,10 @@ get_formal_from_actual_arglist (gfc_symbol *sym, g else { s->ts = a->expr->ts; + + if (s->ts.type == BT_CHARACTER) + s->ts.u.cl = gfc_get_charlen (); + s->ts.deferred = 0; s->ts.is_iso_c = 0; s->ts.is_c_interop = 0; ! { dg-do compile } ! PR fortran/90937 - this used to cause an ICE. ! Original test case by Toon Moene. subroutine lfidiff implicit none contains subroutine grlfi(cdnom) character(len=*) cdnom(:) character(len=len(cdnom)) clnoma call lficas(clnoma) end subroutine grlfi end subroutine lfidiff
Re: [RFC] zstd as a compression algorithm for LTO
Am 20.06.19 um 11:07 schrieb Martin Liška: On the contrary, decompression of zstd with zlib will end with: lto1: internal compiler error: compressed stream: data error Sogenerating object files on one system and trying to read them on another system which does not happen to have a particular library installed would lead to failure? If that's the case, I am not sure that this is a good way of handling things.
[patch, fortran, committed] Improve internal compiler debugging
Hi, I just committed the attached patch as obvious ans simple. No impact on user code, just to make internal debugging easier. Regards Thomas 2019-06-16 Thomas Koenig * dump_parse_tree (debug): Add verison for formal arglist. Do not crash when a gfc_expr is NULL. Index: dump-parse-tree.c === --- dump-parse-tree.c (Revision 271945) +++ dump-parse-tree.c (Arbeitskopie) @@ -66,6 +66,19 @@ dumpfile = tmp; } +void debug (gfc_formal_arglist *formal) +{ + FILE *tmp = dumpfile; + dumpfile = stderr; + for (; formal; formal = formal->next) +{ + fputc ('\n', dumpfile); + show_symbol (formal->sym); +} + fputc ('\n', dumpfile); + dumpfile = tmp; +} + void debug (symbol_attribute attr) { debug (&attr); @@ -75,9 +88,15 @@ { FILE *tmp = dumpfile; dumpfile = stderr; - show_expr (e); - fputc (' ', dumpfile); - show_typespec (&e->ts); + if (e != NULL) +{ + show_expr (e); + fputc (' ', dumpfile); + show_typespec (&e->ts); +} + else +fputs ("() ", dumpfile); + fputc ('\n', dumpfile); dumpfile = tmp; }
Re: [Patch, fortran] PR90786 - [7/8/9/10 Regression] ICE on procedure pointer assignment to function with class pointer result
Hi Paul, I cannot see anything wrong with the optimized code and valgrind gives a clean bill of health on x86_64. We need help of somebody with access to an arm/aarch64 device. I'm currently running a bootstrap on an aarch64 machine. These are not known to be the fastest of machines, but it should be done sometime today. Regards Thomas
[patch, committed] PR 90744
Hello world, I have just committed the attached patch to trunk as obvious an simple. The problem, which led to wrong code, was that copying the "deferred" attribute from the dummy to the formal argument a) makes no sense b) led to wrong code. The analysis and the first working version of the patch was done by TomᚠTrnka (thanks!). I will also backport, since this is a 7/8/9/10 regression. Regards Thomas 2019-06-08 Thomas Koenig TomᚠTrnka PR fortran/90744 * trans-types.c (get_formal_from_actual_arglist): Unset typespec flags which make no sense for procedures without explicit interface. 2019-06-08 Thomas Koenig TomᚠTrnka PR fortran/90744 * gfortran.dg/deferred_character_33.f90: New test. * gfortran.dg/deferred_character_33a.f90: New test. Index: trans-types.c === --- trans-types.c (Revision 271945) +++ trans-types.c (Arbeitskopie) @@ -3005,6 +3005,9 @@ get_formal_from_actual_arglist (gfc_symbol *sym, g else { s->ts = a->expr->ts; + s->ts.deferred = 0; + s->ts.is_iso_c = 0; + s->ts.is_c_interop = 0; s->attr.flavor = FL_VARIABLE; if (a->expr->rank > 0) { ! { dg-do compile } subroutine convrs(quanty,fromto) implicit none character(*), intent(in) :: quanty,fromto if (len(fromto) /= 2) stop 1 if (fromto /= 'OK') stop 2 end subroutine ! { dg-do run } ! { dg-additional-sources deferred_character_33a.f90 } ! PR fortran/90744 - this used to pass a wrong length ! to an external function without a prototype. ! Original test case by Tomáš Trnka. module StringModule implicit none contains function getstr() character(:), allocatable :: getstr getstr = 'OK' end function end module module TestModule use StringModule implicit none contains subroutine DoTest() if (.false.) then call convrs('A',getstr()) else call convrs('B',getstr()) end if end subroutine end module program external_char_length use TestModule implicit none call DoTest() end program
[patch, fortran] Two tweaks to argument repacking
Hello world, this patch adds two tweaks to the argument repacking. First, when the size of an argument is known to be one, as in a(n1:n1), we can directly pass a pointer - the stride may not be one, but it does not matter. Second, the case where the array passed is actually contiguous is more likely in practice, so it should get the fast path. I have done this by defining a new predictor and setting the estimated likelyhood at 75%, which ensured a path without jumps when the arguments passed to bar were contiguous: module y contains subroutine bar(a,b) real, dimension(:) :: a,b call foo(a,b,size(a)) end subroutine bar end module y Test case is only for the first part - making one for the second part would have been a bit too much. Regression-tested. OK for trunk? Regards Thomas 2019-06-02 Thomas Koenig PR fortran/90539 * trans-expr.c (gfc_conv_subref_array_arg): If the size of the expression can be determined to be one, treat it as contiguous. Set likelyhood of presence of an actual argument according to PRED_FORTRAN_ABSENT_DUMMY and likelyhood of being contiguous according to PRED_FORTRAN_CONTIGUOUS. 2019-06-02 Thomas Koenig PR fortran/90539 * predict.def (PRED_FORTRAN_CONTIGUOUS): New predictor. Index: fortran/trans-expr.c === --- fortran/trans-expr.c (Revision 271751) +++ fortran/trans-expr.c (Arbeitskopie) @@ -4922,16 +4922,36 @@ class_array_fcn: gfc_se cont_se, array_se; stmtblock_t if_block, else_block; tree if_stmt, else_stmt; + mpz_t size; + bool size_set; cont_var = gfc_create_var (boolean_type_node, "contiguous"); - /* cont_var = is_contiguous (expr); . */ - gfc_init_se (&cont_se, parmse); - gfc_conv_is_contiguous_expr (&cont_se, expr); - gfc_add_block_to_block (&se->pre, &(&cont_se)->pre); - gfc_add_modify (&se->pre, cont_var, cont_se.expr); - gfc_add_block_to_block (&se->pre, &(&cont_se)->post); + /* If the size is known to be one at compile-time, set + cont_var to true unconditionally. This may look + inelegant, but we're only doing this during + optimization, so the statements will be optimized away, + and this saves complexity here. */ + size_set = gfc_array_size (expr, &size); + if (size_set && mpz_cmp_ui (size, 1) == 0) + { + gfc_add_modify (&se->pre, cont_var, + build_one_cst (boolean_type_node)); + } + else + { + /* cont_var = is_contiguous (expr); . */ + gfc_init_se (&cont_se, parmse); + gfc_conv_is_contiguous_expr (&cont_se, expr); + gfc_add_block_to_block (&se->pre, &(&cont_se)->pre); + gfc_add_modify (&se->pre, cont_var, cont_se.expr); + gfc_add_block_to_block (&se->pre, &(&cont_se)->post); + } + + if (size_set) + mpz_clear (size); + /* arrayse->expr = descriptor of a. */ gfc_init_se (&array_se, se); gfc_conv_expr_descriptor (&array_se, expr); @@ -4953,7 +4973,9 @@ class_array_fcn: /* And put the above into an if statement. */ pre_stmts = fold_build3_loc (input_location, COND_EXPR, void_type_node, - cont_var, if_stmt, else_stmt); + gfc_likely (cont_var, + PRED_FORTRAN_CONTIGUOUS), + if_stmt, else_stmt); } else { @@ -4976,11 +4998,11 @@ class_array_fcn: gfc_add_modify (&else_block, pointer, build_int_cst (type, 0)); else_stmt = gfc_finish_block (&else_block); - tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, present_var, + tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, + gfc_likely (present_var, + PRED_FORTRAN_ABSENT_DUMMY), pre_stmts, else_stmt); gfc_add_expr_to_block (&se->pre, tmp); - - } else gfc_add_expr_to_block (&se->pre, pre_stmts); @@ -4995,9 +5017,16 @@ class_array_fcn: tmp = fold_build2_loc (input_location, EQ_EXPR, boolean_type_node, cont_var, build_zero_cst (boolean_type_node)); + tmp = gfc_unlikely (tmp, PRED_FORTRAN_CONTIGUOUS); + if (pass_optional) - post_cond = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR, - boolean_type_node, present_var, tmp); + { + tree present_likely = gfc_likely (present_var, + PRED_FORTRAN_ABSENT_DUMMY); + post_cond = fold_build2_loc (input_location, TRUTH_ANDIF_EXPR, + boolean_type_node, present_likely, + tmp); + } else post_cond = tmp; } Index: predict.def === --- predict.def (Revision 271629) +++ predict.def (Arbeitskopie) @@ -229,3 +229,10 @@ DEF_PREDICTOR (PRED_FORTRAN_ABSENT_DUMMY, "Fortran to be very likely. */ DEF_PREDICTOR (PRED_FORTRAN_LOOP_P
Re: [patch, doc, fortran] Document some trans* stuff
Hi Steve, Thomas, with the above fixes, I think this a great addition to manual. OK to commit. Committed with your fixes (sometimes I think my fingers are going blind :-) as r271786. Best regards Thomas
[patch, doc, fortran] Document some trans* stuff
Hello world, I thought I would add a little bit of documentation about what trans-* is doing, in the hope that it may be useful. The scalarizer could also use some documentation, but at least we have the wiki article there. Tested with "make", "make dvi" and "make pdf". OK for trunk? Regards Thomas 2019-05-30 Thomas Koenig * gfc-internals.texi (Translating to GENERIC): New chapter. Index: gfc-internals.texi === --- gfc-internals.texi (Revision 271629) +++ gfc-internals.texi (Arbeitskopie) @@ -119,6 +119,8 @@ not accurately reflect the status of the most rece * Frontend Data Structures:: Data structures used by the frontend * Object Orientation:: Internals of Fortran 2003 OOP features. +* Translating to GENERIC:: + Generating the intermediate language for later stages. * LibGFortran::The LibGFortran Runtime Library. * GNU Free Documentation License:: How you can copy and share this manual. @@ -724,7 +726,145 @@ operator call is replaced with an internally gener type-bound procedure call to the respective definition and that call is further processed. +@c - +@c - Translating to TREE +@c - +@node Translating to GENERIC +@chapter Generating the intermediate language for later stages. + +This chapter deals with the transformation of gfortran's frontend data +structures to the intermediate language used by the later stages of +the compiler, the so-called middle end. + +Data structures relating to this are found in the source files +@file{trans*.h} and @file{trans-*.c}. + +@menu +* Basic Data Structures:: Basic data structures. +* Converting Expressions:: Converting expressions to tree. +* Translating Statements:: Translating statements. +* Accessing Declarations:: Accessing declarations. +@end menu + +@node Basic Data Structures +@section Basic data structures + +Gfortran creates GENERIC as an intermediate language for the +middle-end. Details about GENERIC can be found in the GCC manual. + +The basic data structure of GENERIC is a @code{tree}. Everything in +GENERIC is a @code{tree}, including types and statements. Fortunately +for the gfortran programmer, @code{tree} variables are +garbage-collected, so doing memory management for them is not +necessary. + +@code{tree} expressions are built using functions such as, for +example, @code{fold_build2_loc}. For two tree variables @code{a} and +@code{b}, both of which have the type @code{gfc_arry_index_type}, +calculation @code{c = a * b} would be done by + +@smallexample +c = fold_build2_loc (input_location, MULT_EXPR, + gfc_array_index_type, a, b); +@end smallexample + +The types have to agree, otherwise internal compiler errors will occur +at a later stage. Expressions can be converted to a different type +using @code{fold_convert}. + +Accessing individual members in the @code{tree} structures should not +be done. Rather, access should be done via macros. + +One basic data structure is the @code{stmtblock_t} struct. This is +used for holding a list of statements, expressed as @code{tree} +expressions. If a block is created using @code{gfc_start_block}, it +has its own scope for variables; if it is created using +@code{gfc_init_block}, it does not have its own scope. + +It is possible to +@itemize @bullet +@item Add an expression to the end of a block using + @code{gfc_add_expr_to_block} +@item Add an expression to the bebinning of a block using + @code{void gfc_prepend_expr_to_block} +@item Make a block into a single @code{tree} using + @code{gfc_finish_block}. This is needed to +@end itemize + +Variables are also @code{tree} expressions, they can be created using +@code{gfc_create_var}. Assigning to a variable can be done with +@code{gfc_add_modify}. + +An example: Creating a default integer type variable in the current +scope with the prefix ``everything'' in the @code{stmt_block} +@code{block} and assigning the value 42 would be + +@smallexample +tree var, *block; +/* Initialize block somewhere here. */ +var = gfc_create_var (integer_type_node, "everything"); +gfc_add_modify (block, var, build_int_cst (integer_type_node, 42)); +@end smallexample + +@node Converting Expressions +@section Converting Expressons to tree + +Converting expressions to @code{tree} is done by functions called +@code{gfc_conv_*}. + +The central data structure for a GENERIC expression is the +@code{gfc_se} structure. Its @code{expr} member is a @code{tree} that +holds the value of the expression. A @code{gfc_se} structure is +initialized using @code{gfc_init_se}; it needs to be embedded in an +outer @code{gfc_se}. + +Evaluating Fortran
Re: [PATCH] Further C lapack workaround tweaks
Hi Jakub, As I said earlier in the PR, I don't like -fbroken-callers option much, as the option name doesn't hint what it is doing at all. The following patch renames the option and makes it into a 3 state option, with the default being a middle-ground, where it avoids the tail calls in functions that have the hidden character length arguments only if it makes any implicit interface calls. The rationale for that is that there were no previously reported issues with older GCC versions and the change that affected the broken C/C++ wrappers was just giving prototypes to the implicit interface procedure calls, so presumably in functions that don't have any such calls nothing should have changed. Bootstrapped/regtested on x86_64-linux and i686-linux, additionally tested on the dtrtri.f testcase and on dtrtri.f testcase patched to include explicit interfaces for all called procedures (and for those two verified all the 6 ways of using these options, default, positive/negative option without = and 0/1/2 values of the = option, checking in which case there is a tail call), ok for trunk? Yep, this is a much better scheme. OK. This problem is also present on all release branches, so I think that this (which I think is the should also be backported to them, so that 7.5, 8.4 and 9.2 also can compile these LAPACK bindings again...). Regards Thomas
[patch, fortran] Fix wrong-code regression with netcdf and SPEC due to argument repacking
Hello world, the attached patch fixes the wrong-code regression due to the inline argument repacking patch, r271377. What had gone wrong? gfortran used to pack and unpack arrays unconditionally passed to old-style assumed size or . For code like module t2 implicit none contains subroutine foo(a) real, dimension(*) :: a end subroutine foo end module t2 module t1 use t2 implicit none contains subroutine bar(a) real, dimension(:) :: a call foo(a) end subroutine bar end module t1 program main use t1 call bar([1.0, 2.0]) end program main this meant that an (always contiguous) array constructor was passed down to an assumed shape array, which then passed it on to an assumed size, explicit shape or adjustable array. Packing was not problematic (apart from performance), but unpacking tried to write into the array constructor. So, this patch inserts a run-time check for contiguous arrays and does not do packing/unpacking in that case. Thanks to Toon and Martin for finding an open test case which actually failed, and for help with debugging. (Always repacking also likely impacted performance when it didn't lead to wrong code, we will have to see how performance is with this version). OK for trunk? Regards Thomas 2019-05-29 Thomas Koenig PR fortran/90539 * gfortran.h (gfc_has_dimen_vector_ref): Add prototype. * trans.h (gfc_conv_subref_array_arg): Add argument check_contiguous. (gfc_conv_is_contiguous_expr): Add prototype. * frontend-passes.c (has_dimen_vector_ref): Remove prototype, rename to (gfc_has_dimen_vector_ref): New function name. (matmul_temp_args): Use gfc_has_dimen_vector_ref. (inline_matmul_assign): Likewise. * trans-array.c (gfc_conv_array_parameter): Also check for absence of a vector subscript before calling gfc_conv_subref_array_arg. Pass additional argument to gfc_conv_subref_array_arg. * trans-expr.c (gfc_conv_subref_array_arg): Add argument check_contiguous. If that is true, check if the argument is contiguous and do not repack in that case. * trans-intrinsic.c (gfc_conv_intrinsic_is_contiguous): Split away most of the work into, and call (gfc_conv_intrinsic_is_coniguous_expr): New function. 2019-05-29 Thomas Koenig PR fortran/90539 * gfortran.dg/internal_pack_21.f90: Adjust scan patterns. * gfortran.dg/internal_pack_22.f90: New test. * gfortran.dg/internal_pack_23.f90: New test. Index: fortran/gfortran.h === --- fortran/gfortran.h (Revision 271629) +++ fortran/gfortran.h (Arbeitskopie) @@ -3532,6 +3532,7 @@ typedef int (*walk_expr_fn_t) (gfc_expr **, int *, int gfc_dummy_code_callback (gfc_code **, int *, void *); int gfc_expr_walker (gfc_expr **, walk_expr_fn_t, void *); int gfc_code_walker (gfc_code **, walk_code_fn_t, walk_expr_fn_t, void *); +bool gfc_has_dimen_vector_ref (gfc_expr *e); /* simplify.c */ Index: fortran/trans.h === --- fortran/trans.h (Revision 271629) +++ fortran/trans.h (Arbeitskopie) @@ -535,8 +535,11 @@ int gfc_conv_procedure_call (gfc_se *, gfc_symbol void gfc_conv_subref_array_arg (gfc_se *, gfc_expr *, int, sym_intent, bool, const gfc_symbol *fsym = NULL, const char *proc_name = NULL, -gfc_symbol *sym = NULL); +gfc_symbol *sym = NULL, +bool check_contiguous = false); +void gfc_conv_is_contiguous_expr (gfc_se *, gfc_expr *); + /* Generate code for a scalar assignment. */ tree gfc_trans_scalar_assign (gfc_se *, gfc_se *, gfc_typespec, bool, bool, bool c = false); Index: fortran/frontend-passes.c === --- fortran/frontend-passes.c (Revision 271629) +++ fortran/frontend-passes.c (Arbeitskopie) @@ -54,7 +54,6 @@ static gfc_code * create_do_loop (gfc_expr *, gfc_ static gfc_expr* check_conjg_transpose_variable (gfc_expr *, bool *, bool *); static int call_external_blas (gfc_code **, int *, void *); -static bool has_dimen_vector_ref (gfc_expr *); static int matmul_temp_args (gfc_code **, int *,void *data); static int index_interchange (gfc_code **, int*, void *); @@ -2868,7 +2867,7 @@ matmul_temp_args (gfc_code **c, int *walk_subtrees { if (matrix_a->expr_type == EXPR_VARIABLE && (gfc_check_dependency (matrix_a, expr1, true) - || has_dimen_vector_ref (matrix_a))) + || gfc_has_dimen_vector_ref (matrix_a))) a_tmp = true; } else @@ -2881,7 +2880,7 @@ matmul_temp_args (gfc_code **c, int *walk_subtrees { if (matrix_b->expr_type == EXPR_VARIABLE && (gfc_check_dependency (matrix_b, expr1, true) - || has_dimen_vector_ref (matrix_b))) + || gfc_has_dimen_vector_ref (matrix_b))) b_tmp = true; }
[patch, fortran, committed] Set rank and lower bound for assumed size arguments
Hello world, in the absence of a test case for PR 90539, I'm taking a shotgun approach: Fix something that appears strange in the debug logs and see if this more or less accidentally fixes the problem. If not, at least there is one fewer point to look at. I have committed the attached patch as obvious and simple, r271630. Regards Thomas 2019-05-26 Thomas Koenig PR fortran/90539 * trans-types.c (get_formal_from_actual_arglist): Set rank and lower bound for assumed size arguments. Index: trans-types.c === --- trans-types.c (Revision 271376) +++ trans-types.c (Arbeitskopie) @@ -3010,6 +3010,10 @@ get_formal_from_actual_arglist (gfc_symbol *sym, g { s->attr.dimension = 1; s->as = gfc_get_array_spec (); + s->as->rank = 1; + s->as->lower[0] = gfc_get_int_expr (gfc_index_integer_kind, + &a->expr->where, 1); + s->as->upper[0] = NULL; s->as->type = AS_ASSUMED_SIZE; } }