[patch, fortran] Asynchronous I/O, take 3
Hello world, the attached patch is the third take on Nicolas' and my patch for implementing asynchronous I/O. Some parts have been reworked, and several bugs which caused either incorrect I/O or hangs have been fixed in the process. I have to say that getting out these bugs has been much harder than Nicolas and I originally thought, and that this has cost more working hours than any other patch I have been involved in. This has been regression-tested on x86_64-pc-linux-gnu. The new test cases have also been tested in a tight loop with n=1; while ./a.out; do echo -n $n " " ; n=$((n+1)); done or (for async_io_3.f90, which is supposed to fail) while true ; do ./a.out > /dev/null 2>&1 ; echo -n $n " " ; n=$((n+1)); done and the test cases also come up clean with valgrind --tool=drd (which is a _very_ strict tool which, after this experience, I wholeheartedly recommend for doing pthreads debugging). The interface remains as before - link in pthread to get asynchronous I/O, which matches what ifort does. So, OK for trunk? Regards Thomas 2018-07-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * gfortran.texi: Add description of asynchronous I/O. * trans-decl.c (gfc_finish_var_decl): Treat asynchronous variables as volatile. * trans-io.c (gfc_build_io_library_fndecls): Rename st_wait to st_wait_async and change argument spec from ".X" to ".w". (gfc_trans_wait): Pass ID argument via reference. 2018-07-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * gfortran.dg/f2003_inquire_1.f03: Add write statement. * gfortran.dg/f2003_io_1.f03: Add wait statement. 2018-01-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * Makefile.am: Add async.c to gfor_io_src. Add async.h to gfor_io_headers. * Makefile.in: Regenerated. * gfortran.map: Add _gfortran_st_wait_async. * io/async.c: New file. * io/async.h: New file. * io/close.c: Include async.h. (st_close): Call async_wait for an asynchronous unit. * io/file_pos.c (st_backspace): Likewise. (st_endfile): Likewise. (st_rewind): Likewise. (st_flush): Likewise. * io/inquire.c: Add handling for asynchronous PENDING and ID arguments. * io/io.h (st_parameter_dt): Add async bit. (st_parameter_wait): Correct. (gfc_unit): Add au pointer. (st_wait_async): Add prototype. (transfer_array_inner): Likewise. (st_write_done_worker): Likewise. * io/open.c: Include async.h. (new_unit): Initialize asynchronous unit. * io/transfer.c (async_opt): New struct. (wrap_scalar_transfer): New function. (transfer_integer): Call wrap_scalar_transfer to do the work. (transfer_real): Likewise. (transfer_real_write): Likewise. (transfer_character): Likewise. (transfer_character_wide): Likewise. (transfer_complex): Likewise. (transfer_array_inner): New function. (transfer_array): Call transfer_array_inner. (transfer_derived): Call wrap_scalar_transfer. (data_transfer_init): Check for asynchronous I/O. Perform a wait operation on any pending asynchronous I/O if the data transfer is synchronous. Copy PDT and enqueue thread for data transfer. (st_read_done_worker): New function. (st_read_done): Enqueue transfer or call st_read_done_worker. (st_write_done_worker): New function. (st_write_done): Enqueue transfer or call st_read_done_worker. (st_wait): Document as no-op for compatibility reasons. (st_wait_async): New function. * io/unit.c (insert_unit): Use macros LOCK, UNLOCK and TRYLOCK; add NOTE where necessary. (get_gfc_unit): Likewise. (init_units): Likewise. (close_unit_1): Likewise. Call async_close if asynchronous. (close_unit): Use macros LOCK and UNLOCK. (finish_last_advance_record): Likewise. (newunit_alloc): Likewise. * io/unix.c (find_file): Likewise. (flush_all_units_1): Likewise. (flush_all_units): Likewise. * libgfortran.h (generate_error_common): Add prototype. * runtime/error.c: Include io.h and async.h. (generate_error_common): New function. 2018-07-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * testsuite/libgfomp.fortran/async_io_1.f90: New test. * testsuite/libgfomp.fortran/async_io_2.f90: New test. * testsuite/libgfomp.fortran/async_io_3.f90: New test. Index: gcc/fortran/gfortran.texi === --- gcc/fortran/gfortran.texi (Revision 259739) +++ gcc/fortran/gfortran.texi (Arbeitskopie) @@ -882,8 +882,7 @@ than @code{(/.../)}. Type-specification for array @item Extensions to the specifica
Re: [patch, fortran] Asynchronous I/O, take 3
Hi Rainer, However, may (all?) gfortran tests now SEGV. One example is Program received signal SIGSEGV: Segmentation fault - invalid memory reference. Backtrace for this error: Segmentation Fault Thread 2 received signal SIGSEGV, Segmentation fault. [Switching to Thread 1 (LWP 1)] 0xfe1b1f03 in pthread_mutex_unlock () from /lib/libc.so.1 (gdb) where #0 0xfe1b1f03 in pthread_mutex_unlock () from /lib/libc.so.1 #1 0xfe5d1b7c in __gthread_mutex_unlock (__mutex=0x18) at ../libgcc/gthr-default.h:778 #2 _gfortran_st_rewind (fpp=0xfeffda9c) at /vol/gcc/src/hg/trunk/solaris/libgfortran/io/file_pos.c:486 #3 0x0805110f in MAIN__ () at /vol/gcc/src/hg/trunk/solaris/gcc/testsuite/gfortran.dg/backslash_2.f90:6 Ah, I see what was wrong. The attached patch should fix this. I have also attached a new test case which detects this error even on Linux systems, plus a ChangeLog which fixes the typo :-) Again regression-tested. So, OK for trunk? Regards Thomas 2018-07-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * gfortran.texi: Add description of asynchronous I/O. * trans-decl.c (gfc_finish_var_decl): Treat asynchronous variables as volatile. * trans-io.c (gfc_build_io_library_fndecls): Rename st_wait to st_wait_async and change argument spec from ".X" to ".w". (gfc_trans_wait): Pass ID argument via reference. 2018-07-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * gfortran.dg/f2003_inquire_1.f03: Add write statement. * gfortran.dg/f2003_io_1.f03: Add wait statement. 2018-01-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * Makefile.am: Add async.c to gfor_io_src. Add async.h to gfor_io_headers. * Makefile.in: Regenerated. * gfortran.map: Add _gfortran_st_wait_async. * io/async.c: New file. * io/async.h: New file. * io/close.c: Include async.h. (st_close): Call async_wait for an asynchronous unit. * io/file_pos.c (st_backspace): Likewise. (st_endfile): Likewise. (st_rewind): Likewise. (st_flush): Likewise. * io/inquire.c: Add handling for asynchronous PENDING and ID arguments. * io/io.h (st_parameter_dt): Add async bit. (st_parameter_wait): Correct. (gfc_unit): Add au pointer. (st_wait_async): Add prototype. (transfer_array_inner): Likewise. (st_write_done_worker): Likewise. * io/open.c: Include async.h. (new_unit): Initialize asynchronous unit. * io/transfer.c (async_opt): New struct. (wrap_scalar_transfer): New function. (transfer_integer): Call wrap_scalar_transfer to do the work. (transfer_real): Likewise. (transfer_real_write): Likewise. (transfer_character): Likewise. (transfer_character_wide): Likewise. (transfer_complex): Likewise. (transfer_array_inner): New function. (transfer_array): Call transfer_array_inner. (transfer_derived): Call wrap_scalar_transfer. (data_transfer_init): Check for asynchronous I/O. Perform a wait operation on any pending asynchronous I/O if the data transfer is synchronous. Copy PDT and enqueue thread for data transfer. (st_read_done_worker): New function. (st_read_done): Enqueue transfer or call st_read_done_worker. (st_write_done_worker): New function. (st_write_done): Enqueue transfer or call st_read_done_worker. (st_wait): Document as no-op for compatibility reasons. (st_wait_async): New function. * io/unit.c (insert_unit): Use macros LOCK, UNLOCK and TRYLOCK; add NOTE where necessary. (get_gfc_unit): Likewise. (init_units): Likewise. (close_unit_1): Likewise. Call async_close if asynchronous. (close_unit): Use macros LOCK and UNLOCK. (finish_last_advance_record): Likewise. (newunit_alloc): Likewise. * io/unix.c (find_file): Likewise. (flush_all_units_1): Likewise. (flush_all_units): Likewise. * libgfortran.h (generate_error_common): Add prototype. * runtime/error.c: Include io.h and async.h. (generate_error_common): New function. 2018-07-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * testsuite/libgomp.fortran/async_io_1.f90: New test. * testsuite/libgomp.fortran/async_io_2.f90: New test. * testsuite/libgomp.fortran/async_io_3.f90: New test. * testsuite/libgomp.fortran/async_io_4.f90: New test. Obviously __mutex above hasn't been properly initialized. 2018-07-02 Nicolas Koenig Thomas Koenig PR fortran/25829 * testsuite/libgfomp.fortran/async_io_1.f90: New test. * testsuite/libgfomp.fortran/async_io_2.f90: New test. * testsuite/libgfomp.fortran/async_io
Re: [PATCH][Fortran][v2] Use MIN/MAX_EXPR for min/max intrinsics
Hi Kyrlll, > Am 18.07.2018 um 13:17 schrieb Kyrill Tkachov : > > Thomas, Janne, would this relaxation of NaN handling be acceptable given the > benefits > mentioned above? If so, what would be the recommended adjustment to the > nan_1.f90 test? I would be a bit careful about changing behavior in such a major way. What would the results with NaN and infinity then be, with or without optimization? Would the results be consistent with min(nan,num) vs min(num,nan)? Would they be consistent with the new IEEE standard? In general, I think that min(nan,num) should be nan and that our current behavior is not the best. Does anybody have dats points on how this is handled by other compilers? Oh, and if anything is changed, then compile and runtime behavior should always be the same. Regards, Thomas
Re: *PING* Re: [PATCH] Fortran : ICE in build_field PR95614
Hi Mark, I haven't yet committed this. I am unfamiliar with Andre, I've checked MAINTAINERS and I find Andre in the "Write after approval" section. Is Andre's approval sufficient? If so MAINTAINERS needs to be updated. The official list of people who can review is at https://gcc.gnu.org/fortran/ but that is clearly not sufficient. We need to update it to reflect current realities, there are people who have approved patches (with nobody objecting) for a long time, and many people who are on that list are no longer active. I'm not 100% sure what is needed to update that file, if we need OK from the steering committee or not. I had already taken a straw poll I think I will simply do tomorrow moring. If anybody strenuously objects, we can always revert the patch :-) If not: OK to commit and backport? OK from my side, and thanks for the patch. Best regards Thomas
Re: [Patch][Fortran] PR 92208 don't use function-result dummy variable as actual argument
Hi Tobias, I think this is also OK for gcc 9. Backporting regression fixes should always be all right under normal circumstances. Regards Thomas
[patch, fortran] Load scalar intent-in variables at the beginning of procedures
Hello world, the attached patch loads scalar INTENT(IN) variables to a local variable at the start of a procedure, as suggested in PR 67202, in order to aid optimization. This is controlled by front-end optimization so it is easier to catch if any bugs should turn up :-) This is done to make optimization by the middle-end easier. I left in the parts for debugging that I added for this patch. Seeing the difference between EXEC_INIT_ASSIGN and EXEC_ASSIGN was particularly instructive. Regression-tested. OK for trunk? Regards Thomas Index: dump-parse-tree.c === --- dump-parse-tree.c (Revision 278025) +++ 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: frontend-passes.c === --- frontend-passes.c (Revision 278025) +++ 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); @@ -5503,3 +5505,132 @@ 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; +} + +/* Replace INTENT(IN) scalar variables by assigning their values to + temporary variables. We really only want to use this for the + simplest cases, all the fancy stuff is excluded. */ + +static void +replace_intent_in (gfc_namespace *ns) +{ + gfc_formal_arglist *f; + gfc_namespace *ns_c; + + if (ns == NULL || ns->proc_name == NULL || gfc_elemental (ns->proc_name) + || ns->proc_name->attr.entry_master) +return; + + for (f = ns->proc_name->formal; f; f = f->next) +{ + if (f->sym == NULL || f->sym->attr.dimension || f->sym->attr.allocatable + || f->sym->attr.optional || f->sym->attr.pointer + || f->sym->attr.codimension || f->sym->attr.value + || f->sym->attr.proc_pointer || f->sym->attr.target + || f->sym->attr.asynchronous + || f->sym->ts.type == BT_CHARACTER || f->sym->ts.type == BT_DERIVED + || f->sym->ts.type == BT_CLASS) + continue; + + /* TODO: It could also be possible to check if the variable can + actually not be changed by appearing in a variable + definition context or by being passed as an argument to a + procedure where it could be changed. */ + + if (f->sym->attr.intent == INTENT_IN) + { + gfc_symtree *symtree; + gfc_symbol *replacement; + sym_replace sr; + + 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) + gcc_unreachable (); + + replacement = symtree->n.sym; + replacement->ts = f->sym->ts; + replacement->attr.flavor = FL_VARIABLE; + replacement->attr.fe_temp = 1; + replacement->attr.referenced = 1; + replacement->declared_at = f->sym->declared_at; +
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Hi Janne, Wouldn't it be even better to pass scalar intent(in) variables by value? The obvious objection of course is ABI, but for procedures with an explicit interface we're not following any particular ABI anyways? The problem with that is that we don't know when we compile a procedure if it will be called with an explicit interface or not. The user can always add an interface block for a stand-alone procedure. Regards Thomas
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Hi Janne, > Ah, of course. I should have said module procedures. Or even module > procedures without bind(C)? It would probably be the latter. The change would actually be rather small: If conditions are met, just add attr.value for INTENT(IN). This is something we should probably do when we are forced into doing an ABI change by other circumstances. Regards, Thomad
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Hi Tobias, > On 11/12/19 1:42 PM, Thomas König wrote: >>> Ah, of course. I should have said module procedures. Or even module >>> procedures without bind(C)? >> It would probably be the latter. The change would actually be rather small: >> If conditions are met, just add attr.value for INTENT(IN). This is something >> we should probably do when we are forced into doing an ABI change by other >> circumstances. > > Will this still work if one does: > > module m > contains > integer function val(y) > integer, intent(in) :: y > val = 2*y > end function val > end module m > > use m > interface > integer function proc(z) >integer, intent(in) :: z > end function proc > end interface > procedure(proc), pointer :: ff > ff => val > print *, ff(10) > end You are right, it would not work. So, scratch that idea. Maybe we should commit this as a test case so nobody gets funny ideas in two year‘s time 😉 So, I think we can then discuss the original patch. Regards Thomas
[patch, fortran] Fix PR 85781, ICE on valid
Hello world, the attached patch fixes an ICE which could occur for empty substrings (see test case). In the spirit of "A patch that works beats an elegant idea every time" I simply set a substring known to be empty to (1:0) so that problems further down the road could be avoided. Regression-tested. OK for trunk? Regards Thomas 2020-01-19 Thomas König PR fortran/85781 * resolve.c (resolve_substring): If the substring is empty, set it to (1:0) explicitly. 2020-01-19 Thomas König PR fortran/85781 * gfortran.dg/substr_9.f90: New test. commit 476a7e195399d2dc76b22947dcbde59f36fe5748 Author: Thomas König Date: Sun Jan 19 19:14:41 2020 +0100 2020-01-19 Thomas König PR fortran/85781 * resolve.c (resolve_substring): If the substring is empty, set it to (1:0) explicitly. 2020-01-19 Thomas König PR fortran/85781 * gfortran.dg/substr_9.f90: New test. diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index e840aec62f2..5f644da9bf2 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -5092,6 +5092,19 @@ resolve_substring (gfc_ref *ref, bool *equal_length) && compare_bound (ref->u.ss.end, ref->u.ss.length->length) == CMP_EQ && compare_bound_int (ref->u.ss.start, 1) == CMP_EQ) *equal_length = true; + +} + + /* Set empty substrings to (1:0) to avoid subsequent ICEs. */ + if (gfc_dep_compare_expr (ref->u.ss.start, ref->u.ss.end) == 1) +{ + locus loc; + loc = ref->u.ss.start->where; + gfc_free_expr (ref->u.ss.start); + ref->u.ss.start = gfc_get_int_expr (gfc_index_integer_kind, &loc, 1); + loc = ref->u.ss.end->where; + gfc_free_expr (ref->u.ss.end); + ref->u.ss.end = gfc_get_int_expr (gfc_index_integer_kind, &loc, 0); } return true; diff --git a/gcc/testsuite/gfortran.dg/substr_9.f90 b/gcc/testsuite/gfortran.dg/substr_9.f90 new file mode 100644 index 000..60557d0cc53 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/substr_9.f90 @@ -0,0 +1,7 @@ +! { dg-do compile } +! PR 85781 - this used to cause an ICE. +subroutine s(x) bind(c) + use iso_c_binding, only: c_char + character(kind=c_char), value :: x + print *, x(2:1) +end ! { dg-do compile } ! PR 85781 - this used to cause an ICE. subroutine s(x) bind(c) use iso_c_binding, only: c_char character(kind=c_char), value :: x print *, x(2:1) end
Re: [patch, fortran] Fix PR 85781, ICE on valid
Hi Tobias, the attached patch fixes an ICE which could occur for empty substrings (see test case). I think one should rather fix the following issue. I am not sure what you mean. Does that mean that fixing the following issue will also fix PR 85781, or that the PR should not be fixed? Regards Thomas
Re: [patch, fortran] Fix PR 85781, ICE on valid
Hi Tobias, I hope my patch covers all issues. – OK for the trunk? Yep. Thanks a lot for the patch! Regards Thomas
[patch, fortran, committed] Error on Associate with a program
Hello world, yet another obvious and simple fix: You cannot really associate the main program with a variable. Committed as r279088 after regression-testing. Regards Thomas 2018-12-08 Thomas Koenig PR fortran/92780 * resolve.c (resolve_assoc_var): Issue error if the associating entity is a program. 2018-12-08 Thomas Koenig PR fortran/92780 * gfortran.dg/associate_50.f90: New test. ! { dg-do compile } ! PR 92780 - this used to ICE instead of being rejected. ! Test case by Gerhard Steinmetz. program p associate (y => p) ! { dg-error "is a PROGRAM" } end associate end program p Index: resolve.c === --- resolve.c (Revision 279064) +++ resolve.c (Arbeitskopie) @@ -8842,6 +8842,12 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_t gcc_assert (target->symtree); tsym = target->symtree->n.sym; + if (tsym->attr.flavor == FL_PROGRAM) + { + gfc_error ("Associating entity %qs at %L is a PROGRAM", + tsym->name, &target->where); + return; + } sym->attr.asynchronous = tsym->attr.asynchronous; sym->attr.volatile_ = tsym->attr.volatile_;
Re: [Patch, Fortran] PR91640 – Fix call to contiguous dummy
Hi Tobias, If one passes something which is not a variable as an argument, there is no point to do the copy-out – the copy-in is sufficient. Note that a the result of a pointer-returning function is regarded as variable. As cleanup, I also fixed the indentation (twice) and the pointless 'fsym ?' check inside a condition body where the condition already checks this. Build on x86-64-gnu-linux. OK for the trunk – and for GCC 9? [It's a 9/10 regression] The change to avoid the copy-out via INTENT(IN) makes sense. If you add this, it would be good to add a test (for example counting while statements in the *.original dump) that the copyback does not happen. Generally, if we are passing an expression, the call to gfc_conv_subref_array_arg is not needed - we will generate an array temporary for the expression anyway, and this will always be contiguous. Regards Thomas
Re: [Patch, Fortran] PR91640 – Fix call to contiguous dummy
Hi Tobias, As a variant, I now use the latter (via the else branch). Either variant produces the same original tree. One can argue which variant is clearer; I think both are fine – pick one. I think the second one (the one you just attached) looks better. So, OK for trunk. Thanks for the patch! Regards Thomas
[patch, fortran, committed] Fix PR 96556, segfault on NULL pointer dereference
Hi, in the code about DO loop warning I recently introduced, there was a hidden NULL pointer dereference found by Jürgen Reuter and fixed as obvious and simple in r11-2636. Fix NULL pointer dereference in doloop_contained_function_call. Thanks to Jürgen for the bug report and to Dominique for spotting what commit this was caused by. Unfortunately, not all bugs are quite as simple and obvious as this one :-) gcc/fortran/ChangeLog: PR fortran/96556 * frontend-passes.c (doloop_contained_function_call): Do not dereference a NULL pointer for value.function.esym. gcc/testsuite/ChangeLog: PR fortran/96556 * gfortran.dg/do_check_15.f90: New test. diff --git a/gcc/fortran/frontend-passes.c b/gcc/fortran/frontend-passes.c index 6bcb1f06b1c..83f6fd804b1 100644 --- a/gcc/fortran/frontend-passes.c +++ b/gcc/fortran/frontend-passes.c @@ -2329,7 +2329,8 @@ doloop_contained_function_call (gfc_expr **e, gfc_symbol *sym, *do_var; contained_info *info; - if (expr->expr_type != EXPR_FUNCTION || expr->value.function.isym) + if (expr->expr_type != EXPR_FUNCTION || expr->value.function.isym + || expr->value.function.esym == NULL) return 0; sym = expr->value.function.esym; diff --git a/gcc/testsuite/gfortran.dg/do_check_15.f90 b/gcc/testsuite/gfortran.dg/do_check_15.f90 new file mode 100644 index 000..f67452b4660 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/do_check_15.f90 @@ -0,0 +1,58 @@ +! { dg-do compile } +! PR fortran/96556 - this used to cause an ICE. +! Test case by Juergen Reuter. +module polarizations + + implicit none + private + + type :: smatrix_t + private + integer :: dim = 0 + integer :: n_entry = 0 + integer, dimension(:,:), allocatable :: index + contains + procedure :: write => smatrix_write + end type smatrix_t + + type, extends (smatrix_t) :: pmatrix_t + private + contains + procedure :: write => pmatrix_write + procedure :: normalize => pmatrix_normalize + end type pmatrix_t + +contains + + subroutine msg_error (string) +character(len=*), intent(in), optional :: string + end subroutine msg_error + + subroutine smatrix_write (object) +class(smatrix_t), intent(in) :: object + end subroutine smatrix_write + + subroutine pmatrix_write (object) +class(pmatrix_t), intent(in) :: object +call object%smatrix_t%write () + end subroutine pmatrix_write + + subroutine pmatrix_normalize (pmatrix) +class(pmatrix_t), intent(inout) :: pmatrix +integer :: i, hmax +logical :: fermion, ok +do i = 1, pmatrix%n_entry + associate (index => pmatrix%index(:,i)) + if (index(1) == index(2)) then +call error ("diagonal must be real") + end if + end associate +end do + contains +subroutine error (msg) + character(*), intent(in) :: msg + call pmatrix%write () +end subroutine error + end subroutine pmatrix_normalize + +end module polarizations
Re: [PATCH] analyzer: fix ICE on pointer arithmetic with incomplete types [PR 93774]
Hi David in principle, any valid test case (especially for an ICE) should count as obvious and simple, so no approval should be needed. Having said that, I think I would prefer a copy of the original test case rather than an include statement. Although we usually do not change or remove test cases, sometimes it is done for one reason or anotjer, and in this case I would prefer that the derived test case does not change automatically. So, all four of your test cases are OK if you change those which use include to a plain test case, maybe with a comment pointing to the original one. In the future, you can just commit this kind of test case as simple and obvious; we would appreciate a note to fortran@ if you do so. Regards Thomas
Re: [Patch, fortran] PR 93592 - Invalid UP/DOWN rounding with EN descriptor
Hi Dominique, I am trying t...@tkoenig.net. That works :-) I'll try to use that address in the future for my mails to the fortran mailing list, so it does not bounce for you. Best regards Thomas
Re: [wwwdocs] Fix inverted option for disabling new feature (PR 94581)
Am 14.04.20 um 12:11 schrieb Jonathan Wakely: Committed as obvious. Thomas, please fix it if this isn't what you meant to say. Yep, that's what I meant to say. Thanks a lot! Regards Thomas
[patch, fortran, committed] Fix PR 94578
Hello world, rather than touch 50% of the files in our libfortran subdirectory, I opted for the simple and obvious way - if the RHS is a pointer which may have a span, just create a temporary. (We're also qutie close to a release candidate if I count the P1 regressions correctly, so this is not a time for big changes). I think we can (and should) revisit this for gcc 11. Committed after regression-testing. I will backport to gcc 9 and gcc 8 soon. Regards Thomas Fix PR 94578. Our intrinsics do not handle spans on their return values (yet), so this creates a temporary for subref array pointers. 2020-04-25 Thomas Koenig 2020-04-25 Thomas Koenig PR fortran/94578 * trans-expr.c (arrayfunc_assign_needs_temporary): If the LHS is a subref pointer, we also need a temporary. 2020-04-25 Thomas Koenig PR fortran/94578 * gfortran.dg/pointer_assign_14.f90: New test. * gfortran.dg/pointer_assign_15.f90: New test. diff --git a/gcc/fortran/trans-expr.c b/gcc/fortran/trans-expr.c index fdca9cc5539..030edc1e5ce 100644 --- a/gcc/fortran/trans-expr.c +++ b/gcc/fortran/trans-expr.c @@ -9823,9 +9823,13 @@ arrayfunc_assign_needs_temporary (gfc_expr * expr1, gfc_expr * expr2) /* If we have reached here with an intrinsic function, we do not need a temporary except in the particular case that reallocation - on assignment is active and the lhs is allocatable and a target. */ + on assignment is active and the lhs is allocatable and a target, + or a pointer which may be a subref pointer. FIXME: The last + condition can go away when we use span in the intrinsics + directly.*/ if (expr2->value.function.isym) -return (flag_realloc_lhs && sym->attr.allocatable && sym->attr.target); +return (flag_realloc_lhs && sym->attr.allocatable && sym->attr.target) + || (sym->attr.pointer && sym->attr.subref_array_pointer); /* If the LHS is a dummy, we need a temporary if it is not INTENT(OUT). */ diff --git a/gcc/testsuite/gfortran.dg/pointer_assign_14.f90 b/gcc/testsuite/gfortran.dg/pointer_assign_14.f90 new file mode 100644 index 000..b06dd841bcc --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pointer_assign_14.f90 @@ -0,0 +1,19 @@ +! { dg-do run } +! PR fortran/94578 +! This used to give wrong results. +program main + implicit none + type foo + integer :: x, y,z + end type foo + integer :: i + integer, dimension(:), pointer :: array1d + type(foo), dimension(2), target :: solution + integer, dimension(2,2) :: a + data a /1,2,3,4/ + solution%x = -10 + solution%y = -20 + array1d => solution%x + array1d = maxval(a,dim=1) + if (any (array1d /= [2,4])) stop 1 +end program main diff --git a/gcc/testsuite/gfortran.dg/pointer_assign_15.f90 b/gcc/testsuite/gfortran.dg/pointer_assign_15.f90 new file mode 100644 index 000..7c2885910cf --- /dev/null +++ b/gcc/testsuite/gfortran.dg/pointer_assign_15.f90 @@ -0,0 +1,18 @@ +! { dg-do run } +! PR fortran/94578 +! This used to give wrong results. Original test case by Jan-Willem +! Blokland. +program main + implicit none + type foo + integer :: x, y + end type foo + integer :: i + integer, dimension (2,2) :: array2d + integer, dimension(:), pointer :: array1d + type(foo), dimension(2*2), target :: solution + data array2d /1,2,3,4/ + array1d => solution%x + array1d = reshape (source=array2d, shape=shape(array1d)) + if (any (array1d /= [1,2,3,4])) stop 1 +end program main
Re: SPEC 521.wrf_r failing due to new fortran checks
Hi Joel, I've noticed that SPEC has been failing to build on trunk since the below commit, do you have access to SPEC? None of the gfortran maintainers has access to SPEC. do you know if this is due to a bug in the patch or a bug in SPEC? The SPEC suite has quite a few bugs, which SPEC refuses to fix. This is one of them. This is one of the reasons why I have a rather low opinion of SPEC. The other one is that, as somebody who donates his time, I do draw the line at purchasing commercial software for several thousands of euros (or dollars). There is a workaround, see the updated gfortran documentation or https://gcc.gnu.org/gcc-10/changes.html . Regards Thomas
[patch, fortran, committed] Fix PR 92017
Hello world, I have committed the attached patch as obvious and simple after regression-testing. It fixes an ICE on valid for a corner case, so I don't really feel that it needs to be backported. If anybody disagrees, please speak up (or do it yourself :-) Regards Thomas 2019-10-13 Thomas Koenig PR fortran/92017 * expr.c (simplify_parameter_variable): Set the character length of the result expression from the original expression if necessary. 2019-10-13 Thomas Koenig PR fortran/92017 * gfortran.dg/minmaxloc_14.f90: New test. Index: expr.c === --- expr.c (Revision 276937) +++ expr.c (Arbeitskopie) @@ -2066,6 +2066,9 @@ simplify_parameter_variable (gfc_expr *p, int type e->rank = p->rank; + if (e->ts.type == BT_CHARACTER && e->ts.u.cl == NULL) +e->ts.u.cl = gfc_new_charlen (gfc_current_ns, p->ts.u.cl); + /* Do not copy subobject refs for constant. */ if (e->expr_type != EXPR_CONSTANT && p->ref != NULL) e->ref = gfc_copy_ref (p->ref); ! { dg-do compile } ! PR 92017 - this used to cause an ICE due do a missing charlen. ! Original test case by Gerhard Steinmetz. program p character(3), parameter :: a(4) = 'abc' integer, parameter :: b(1) = minloc(a) integer, parameter :: c = minloc(a, dim=1) integer, parameter :: bb(1) = maxloc(a) integer, parameter :: c2 = maxloc(a,dim=1) end program p
Re: [patch][Fortran] Actually permit OpenMP's 'target simd'
Hi Bernhard, > Am 14.10.2019 um 11:58 schrieb Bernhard Reutner-Fischer > : > > On 14 October 2019 10:50:55 CEST, Jakub Jelinek wrote: >>> On Tue, Oct 08, 2019 at 02:04:17PM +0200, Tobias Burnus wrote: >>>testsuite/ >>>* gfortran.dg/gomp/target-simd.f90: New. >> >> John Reporte in bugzilla the testcase fails on hppa with an error that >> looks like heap corruption, and indeed, running the test under valgrind >> fails on x86_64-linux too, the allocatable arrays are 1..100, but the >> loops >> were reading and writing 0..100. Fixed thusly, tested on x86_64-linux, >> committed to trunk. > > Which suggests that we ought to diagnose this, obviously. We already do for arrays with static bounds. Allocatable - not straightforward. This would need value propagation from the ALLOCATE statement, which in turn would suggest splitting up the FE represenation into basic blocks, which of course can be higher level than what the middle end can do after scalarization. Anyway, I have just submitted PR 92087 for this. Feel free to comment or, even better, participate 😉
Re: [Patch][Fortran] OpenMP+OpenACC: Remove bogus contigous-pointer check (PR/65438)
Hi Tobias, What about using gfc_is_not_contigous? This would allow to issue an error when we can prove the user made an error. Regards Thomas
[patch, fortran, testsuite, committed] Fix some more coarray stuff
Hi, I have just committed the attached patch to a test case that was failing on the shared coarray branch. Again, the person who wrote the test case depended on only running on a single image :-) Best regards Thomas Correct coarray indices for test case. gcc/testsuite/ChangeLog: * gfortran.dg/coarray/send_char_array_1.f90: Correct coarray indces. diff --git a/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 b/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 index 3e16fa83685..b3caf80b1ad 100644 --- a/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 +++ b/gcc/testsuite/gfortran.dg/coarray/send_char_array_1.f90 @@ -25,28 +25,28 @@ program send_convert_char_array allocate(character(kind=4, len=5)::co_str_k4_arr(4)[*]) ! First check send/copy to self - co_str_k1_scal[1] = str_k1_scal + co_str_k1_scal[this_image()] = str_k1_scal if (co_str_k1_scal /= str_k1_scal // ' ') STOP 1 - co_str_k4_scal[1] = str_k4_scal + co_str_k4_scal[this_image()] = str_k4_scal if (co_str_k4_scal /= str_k4_scal // 4_' ') STOP 2 - co_str_k4_scal[1] = str_k1_scal + co_str_k4_scal[this_image()] = str_k1_scal if (co_str_k4_scal /= str_k4_scal // 4_' ') STOP 3 - co_str_k1_scal[1] = str_k4_scal + co_str_k1_scal[this_image()] = str_k4_scal if (co_str_k1_scal /= str_k1_scal // ' ') STOP 4 - co_str_k1_arr(:)[1] = str_k1_arr + co_str_k1_arr(:)[this_image()] = str_k1_arr if (any(co_str_k1_arr /= ['abc ', 'EFG ', 'klm ', 'NOP '])) STOP 5 - co_str_k4_arr(:)[1] = [4_'abc', 4_'EFG', 4_'klm', 4_'NOP']! str_k4_arr + co_str_k4_arr(:)[this_image()] = [4_'abc', 4_'EFG', 4_'klm', 4_'NOP']! str_k4_arr if (any(co_str_k4_arr /= [4_'abc ', 4_'EFG ', 4_'klm ', 4_'NOP '])) STOP 6 - co_str_k4_arr(:)[1] = str_k1_arr + co_str_k4_arr(:)[this_image()] = str_k1_arr if (any(co_str_k4_arr /= [ 4_'abc ', 4_'EFG ', 4_'klm ', 4_'NOP '])) STOP 7 - co_str_k1_arr(:)[1] = str_k4_arr + co_str_k1_arr(:)[this_image()] = str_k4_arr if (any(co_str_k1_arr /= ['abc ', 'EFG ', 'klm ', 'NOP '])) STOP 8 end program send_convert_char_array
[patch, fortran, committed] Set DECL_ARTIFICIAL on auxiliary variables
Hello world, committed to trunk as obvious and simple. Best regards Thomas Set DECL_ARTIFICIAL on gfortran internal variables. It seems we sometimes use DECL_ARTIFICIAL as choosing between different code paths. In order not to make -fdebug-aux-vars do different things, set DECL_ARTIFICIAL on the variables to avoid these different code paths (and the corresponding regressions). gcc/fortran/ChangeLog: * trans.c (create_var_debug_raw): Set DECL_ARTIFICIAL on variables. diff --git a/gcc/fortran/trans.c b/gcc/fortran/trans.c index fa7fd9d88aa..57a344a63c9 100644 --- a/gcc/fortran/trans.c +++ b/gcc/fortran/trans.c @@ -128,6 +128,9 @@ create_var_debug_raw (tree type, const char *prefix) t = build_decl (input_location, VAR_DECL, get_identifier (name_buf), type); + /* Not setting this causes some regressions. */ + DECL_ARTIFICIAL (t) = 1; + /* We want debug info for it. */ DECL_IGNORED_P (t) = 0; /* It should not be nameless. */
[patch, fortran] Fix memory leak on deallocation
Hello world, Our finalization handling is a mess. Really, we should get to try and get this fixed for gcc 11. In the meantime, here is a patch which fixes a regression I introduced when fixing a regression with a memory leak. The important thing here is to realize that we do not need to finalize (and deallocate) multiple times for the same expression and the same component in the same namespace. It might cause code size regressions, but better big code than wrong code... This is a regression all the way down to gcc 8. OK for all affected branches? Regards Thomas gcc/fortran/ChangeLog: PR fortran/94109 * class.c (finalize_component): Return early if finalization has already happened for expression and component within namespace. * gfortran.h (gfc_was_finalized): New type. (gfc_namespace): Add member was_finalzed. (gfc_expr): Remove finalized. * symbol.c (gfc_free_namespace): Free was_finalized. gcc/testsuite/ChangeLog: PR fortran/94109 * gfortran.dg/finalize_34.f90: Adjust free counts. * gfortran.dg/finalize_36.f90: New test. diff --git a/gcc/fortran/class.c b/gcc/fortran/class.c index 9aa3eb7282c..b5a1edae27f 100644 --- a/gcc/fortran/class.c +++ b/gcc/fortran/class.c @@ -911,7 +911,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp, if (!comp_is_finalizable (comp)) return; - if (comp->finalized) + if (expr->finalized) return; e = gfc_copy_expr (expr); @@ -1002,6 +1002,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp, } else (*code) = cond; + } else if (comp->ts.type == BT_DERIVED && comp->ts.u.derived->f2k_derived @@ -1041,7 +1042,7 @@ finalize_component (gfc_expr *expr, gfc_symbol *derived, gfc_component *comp, sub_ns); gfc_free_expr (e); } - comp->finalized = true; + expr->finalized = 1; } diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 7094791e871..5af44847f9b 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -1107,7 +1107,6 @@ typedef struct gfc_component struct gfc_typebound_proc *tb; /* When allocatable/pointer and in a coarray the associated token. */ tree caf_token; - bool finalized; } gfc_component; @@ -2218,6 +2217,9 @@ typedef struct gfc_expr /* Set this if the expression came from expanding an array constructor. */ unsigned int from_constructor : 1; + /* Set this if the expression has already been finalized. */ + unsigned int finalized : 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 diff --git a/gcc/testsuite/gfortran.dg/finalize_28.f90 b/gcc/testsuite/gfortran.dg/finalize_28.f90 index 597413b2dd3..f0c9665252f 100644 --- a/gcc/testsuite/gfortran.dg/finalize_28.f90 +++ b/gcc/testsuite/gfortran.dg/finalize_28.f90 @@ -21,4 +21,4 @@ contains integer, intent(out) :: edges(:,:) end subroutine coo_dump_edges end module coo_graphs -! { dg-final { scan-tree-dump-times "__builtin_free" 5 "original" } } +! { dg-final { scan-tree-dump-times "__builtin_free" 6 "original" } } diff --git a/gcc/testsuite/gfortran.dg/finalize_33.f90 b/gcc/testsuite/gfortran.dg/finalize_33.f90 index 2205f9eed7f..3857e4485ee 100644 --- a/gcc/testsuite/gfortran.dg/finalize_33.f90 +++ b/gcc/testsuite/gfortran.dg/finalize_33.f90 @@ -116,4 +116,4 @@ contains ! (iii) mci_template end program main_ut ! { dg-final { scan-tree-dump-times "__builtin_malloc" 17 "original" } } -! { dg-final { scan-tree-dump-times "__builtin_free" 19 "original" } } +! { dg-final { scan-tree-dump-times "__builtin_free" 20 "original" } } diff --git a/gcc/testsuite/gfortran.dg/finalize_34.f90 b/gcc/testsuite/gfortran.dg/finalize_34.f90 index e2f02a5c51c..fef7dac6d89 100644 --- a/gcc/testsuite/gfortran.dg/finalize_34.f90 +++ b/gcc/testsuite/gfortran.dg/finalize_34.f90 @@ -22,4 +22,4 @@ program main use testmodule type(evtlist_type), dimension(10) :: a end program main -! { dg-final { scan-tree-dump-times "__builtin_free" 8 "original" } } +! { dg-final { scan-tree-dump-times "__builtin_free" 12 "original" } } diff --git a/gcc/testsuite/gfortran.dg/finalize_35.f90 b/gcc/testsuite/gfortran.dg/finalize_35.f90 new file mode 100644 index 000..66435c43ecc --- /dev/null +++ b/gcc/testsuite/gfortran.dg/finalize_35.f90 @@ -0,0 +1,48 @@ +! { dg-do compile } +! { dg-additional-options "-fdump-tree-original" } +! PR 94361 - this left open some memory leaks. Original test case by +! Antony Lewis. + +module debug + private + + Type TypeWithFinal + contains + FINAL :: finalizer !No leak if this line is commented + end type TypeWithFinal + + Type Tester + real, dimension(:), allocatable :: Dat + Type(TypeWithFi
ping**(5./7.) [patch, fortran] Fix memory leak on deallocation
In the meantime, here is a patch which fixes a regression I introduced when fixing a regression with a memory leak. The important thing here is to realize that we do not need to finalize (and deallocate) multiple times for the same expression and the same component in the same namespace. It might cause code size regressions, but better big code than wrong code... This is a regression all the way down to gcc 8. OK for all affected branches? Ping? Unless there are any objections, I would commit this to master on Sunday. Regards Thomas
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Hi Tobias, I think you need to add at least VOLATILE to this list Yes, I'll do that. Any other comments? Regards Thomas
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Am 20.11.19 um 21:45 schrieb Janne Blomqvist: BTW, since this is done for the purpose of optimization, have you done testing on some suitable benchmark suite such as polyhedron, whether it a) generates any different code b) does it make it go faster? I haven't run any actual benchmarks. However, there is a simple example which shows its advantages. Consider subroutine foo(n,m) m = 0 do 100 i=1,100 call bar m = m + n 100 continue end (I used old-style DO loops just because :-) Without the optimization, the inner loop is translated to .L2: xorl%eax, %eax callbar_ movl(%r12), %eax addl%eax, 0(%rbp) subl$1, %ebx jne .L2 and with the optimization to .L2: xorl%eax, %eax callbar_ addl%r12d, 0(%rbp) subl$1, %ebx jne .L2 so the load of the address is missing. (Why do we zero %eax before each call? It should not be a variadic call right?) Of course, Fortran language rules specify that the call to bar cannot do anything to n, but apparently we do not tell the gcc middle end that this is the case, or maybe that there is no way to really specify this. (Actually, I just tried out subroutine foo(n,m) integer :: dummy_n, dummy_m dummy_n = n dummy_m = 0 do 100 i=1,100 call bar dummy_m = dummy_m + dummy_n 100 continue m = dummy_m end This is optimized even further: .L2: xorl%eax, %eax callbar_ subl$1, %ebx jne .L2 imull $100, %r12d, %r12d So, a copy in / copy out for variables where we can not be sure that no value is assigned? Does anybody see a downside for that?) Is there a risk of performance regressions due to higher register pressure? I don't think so. Either the compiler realizes that it can keep the variable in a register (then it makes no difference), or it has to load it fresh from its address (then there is one additional register needed). Regards Thomas
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Am 20.11.19 um 23:18 schrieb Janne Blomqvist: On Wed, Nov 20, 2019 at 11:35 PM Thomas König wrote: Am 20.11.19 um 21:45 schrieb Janne Blomqvist: BTW, since this is done for the purpose of optimization, have you done testing on some suitable benchmark suite such as polyhedron, whether it a) generates any different code b) does it make it go faster? I haven't run any actual benchmarks. However, there is a simple example which shows its advantages. Consider subroutine foo(n,m) m = 0 do 100 i=1,100 call bar m = m + n 100 continue end (I used old-style DO loops just because :-) Without the optimization, the inner loop is translated to .L2: xorl%eax, %eax callbar_ movl(%r12), %eax addl%eax, 0(%rbp) subl$1, %ebx jne .L2 and with the optimization to .L2: xorl%eax, %eax callbar_ addl%r12d, 0(%rbp) subl$1, %ebx jne .L2 so the load of the address is missing. (Why do we zero %eax before each call? It should not be a variadic call right?) Not sure. Maybe some belt and suspenders thing? I guess someone better versed in ABI minutiae knows better. It's not Fortran-specific though, the C frontend does the same when calling a void function. OK, so considering your other e-mail, this is a separate issue that we can fix another time. AFAIK on reasonably current OoO CPU's xor'ing a register with itself is handled by the renamer and doesn't consume an execute slot, so it's in effect a zero-cycle instruction. Still bloats the code slightly, though. Of course, Fortran language rules specify that the call to bar cannot do anything to n Hmm, does it? What about the following modification to your testcase: module nmod integer :: n end module nmod subroutine foo(n,m) m = 0 do 100 i=1,100 call bar m = m + n 100 continue end subroutine foo subroutine bar() use nmod n = 0 end subroutine bar program main use nmod implicit none integer :: m n = 1 m = 0 call foo(n, m) print *, m end program main That is not allowed: # 15.5.2.13 Restrictions on entities associated with dummy arguments [...] # (3) Action that affects the value of the entity or any subobject of it # shall be taken only through the dummy argument unless [none of the restrictions apply]. So, a copy in / copy out for variables where we can not be sure that no value is assigned? Does anybody see a downside for that?) In principle sounds good, unless my concerns above are real and affect this case too. So, how to proceed? Commit the patch with the maximum length for a mangled symbol, and then maybe try for the copy-out variant in a follow-up patch? I agree with Tobias that dealing with this in the middle end is probably the right thing to do in the long run (especially since we could also handle arrays and structs this way). Until we get around to doing this (gcc 11 at earliest), we could still profit somewhat from this optimization in the meantime. Regards Thomas
Re: Async I/O patch with compilation fix
Hi Cristophe, this is seriously weird - there is not even an I/O statement in that test case. One question: Is this real hardware or an emulator? Also, Could you try a few things? Run the test case manually. Do you still fail? Is there an error if the executable is run under valgrind? If you have two compilers, one with the patch and one without: Is there a difference in the generated files for -dump-tree-original, -fdump-tree-optimized and -S? Regards, Thomas
[patch, fortran] Fix PR 86837, wrong code regression in implied do loop in i/o
Hello world, this patch fixes a regression by correctly checking that the innner start, step or end values of an implied do loop do not depend on an outer loop variable. The check was actually done before, but gfc_check_dependency wasn't finding all relevant cases. Regression-tested. OK for trunk and 8.x? Regards Thomas 2018-08-23 Thomas Koenig PR fortran/86837 * frontend-passes.c (var_in_expr_callback): New function. (var_in_expr): New function. (traverse_io_block): Use var_in_expr instead of gfc_check_dependency for checking if the variable depends on the previous interators. 2018-08-23 Thomas Koenig PR fortran/86837 * gfortran.dg/implied_do_io_6.f90: New test. Index: frontend-passes.c === --- frontend-passes.c (Revision 263752) +++ frontend-passes.c (Arbeitskopie) @@ -1104,6 +1104,31 @@ convert_elseif (gfc_code **c, int *walk_subtrees A return 0; } +/* Callback function to var_in_expr - return true if expr1 and + expr2 are identical variables. */ +static int +var_in_expr_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + gfc_expr *expr1 = (gfc_expr *) data; + gfc_expr *expr2 = *e; + + if (expr2->expr_type != EXPR_VARIABLE) +return 0; + + return expr1->symtree->n.sym == expr2->symtree->n.sym; +} + +/* Return true if expr1 is found in expr2. */ + +static bool +var_in_expr (gfc_expr *expr1, gfc_expr *expr2) +{ + gcc_assert (expr1->expr_type == EXPR_VARIABLE); + + return gfc_expr_walker (&expr2, var_in_expr_callback, (void *) expr1); +} + struct do_stack { struct do_stack *prev; @@ -1256,9 +1281,9 @@ traverse_io_block (gfc_code *code, bool *has_reach for (int j = i - 1; j < i; j++) { if (iters[j] - && (gfc_check_dependency (var, iters[j]->start, true) - || gfc_check_dependency (var, iters[j]->end, true) - || gfc_check_dependency (var, iters[j]->step, true))) + && (var_in_expr (var, iters[j]->start) + || var_in_expr (var, iters[j]->end) + || var_in_expr (var, iters[j]->step))) return false; } } ! { dg-do run } ! { dg-options "-ffrontend-optimize" } ! PR 86837 - this was mis-optimized by trying to turn this into an ! array I/O statement. ! Original test case by "Pascal". Program read_loop implicit none integer :: i, j ! number of values per column integer, dimension(3) :: nvalues data nvalues / 1, 2, 4 / ! values in a 1D array real, dimension(7) :: one_d data one_d / 1, 11, 12, 21, 22, 23, 24 / ! where to store the data back real, dimension(4, 3) :: two_d ! 1 - write our 7 values in one block open(unit=10, file="loop.dta", form="unformatted") write(10) one_d close(unit=10) ! 2 - read them back in chosen cells of a 2D array two_d = -9 open(unit=10, file="loop.dta", form="unformatted", status='old') read(10) ((two_d(i,j), i=1,nvalues(j)), j=1,3) close(unit=10, status='delete') ! 4 - print the whole array, just in case if (any(reshape(two_d,[12]) /= [1.,-9.,-9.,-9.,11.,12.,-9.,-9.,21.,22.,23.,24.])) call abort end Program read_loop
Re: [PATCH] PR fortan/85779 -- Fix NULL pointer dereference
Hi Steve, > Am 24.05.2018 um 02:24 schrieb Steve Kargl > : > > Subject says it all. OK to commit? OK. Thanks for the patch! Thomas > > 2018-05-23 Steven G. Kargl > >PR fortran/85779 >*decl.c (gfc_match_derived_decl): Fix NULL point dereference. > > 2018-05-23 Steven G. Kargl > >PR fortran/85779 >* gfortran.dg/pr85779_1.f90: New test. >* gfortran.dg/pr85779_2.f90: Ditto. >* gfortran.dg/pr85779_3.f90: Ditto. > > -- > Steve >
Re: [PATCH] PR fortran/78571 -- Avoid ICE in invalid CHARACTER initialization
Hi Steve, Regression tested on x86_64-*-freebsd. OK to commit? OK, and thanks! Thomas
Re: [PATCH] PR fortran/65453 -- procedure statement vs subprogram name clash
Hi Steve, The attached patch fixes an ICE by detecting a name clash between a procedure statement and a contained subprogram. Regression tested on x86_64-*-freebsd. The patch is OK. Thanks! Regards Thomas (In case anybody is wondering about my e-mail address: I have a new one and am switching over lists etc to this one, so I can have a chance of switching my provider).
[patch, fortran] Fix type conversion in large array constructors with iterators
Hello world, the attached patch fixes the bug that Steve found; the patch itself simply makes sure to copy a constructor instead of only the value of the constructor when converting types. Regressoin-tested. OK for trunk? Maybe this is also a candidate for gcc-7, because of the silent wrong-code issue. Regards Thomas 2018-03-18 Thomas Koenig PR fortran/84931 * simplify.c (gfc_convert_constant): Correctly handle iterators for type conversion. 2018-03-18 Thomas Koenig PR fortran/84931 * gfortran.dg/array_constructor_52.f90: New test. Index: simplify.c === --- simplify.c (Revision 258501) +++ simplify.c (Arbeitskopie) @@ -8016,26 +8016,32 @@ gfc_convert_constant (gfc_expr *e, bt type, int ki { gfc_expr *tmp; if (c->iterator == NULL) - tmp = f (c->expr, kind); + { + tmp = f (c->expr, kind); + if (tmp == NULL) + { + gfc_free_expr (result); + return NULL; + } + + gfc_constructor_append_expr (&result->value.constructor, + tmp, &c->where); + } else { + gfc_constructor *n; g = gfc_convert_constant (c->expr, type, kind); - if (g == &gfc_bad_expr) + if (g == NULL || g == &gfc_bad_expr) { gfc_free_expr (result); return g; } - tmp = g; + n = gfc_constructor_get (); + n->expr = g; + n->iterator = gfc_copy_iterator (c->iterator); + n->where = c->where; + gfc_constructor_append (&result->value.constructor, n); } - - if (tmp == NULL) - { - gfc_free_expr (result); - return NULL; - } - - gfc_constructor_append_expr (&result->value.constructor, - tmp, &c->where); } break; ! { dg-do run } ! PR 84931 - long array constructors with type conversion were not ! handled correctly. program test implicit none integer, parameter :: n = 2**16 real, dimension(n) :: y integer :: i y = (/ (1, i=1, n) /) if (y(2) /= 1) stop 1 end program test
[patch, fortran] Simplify constants which come from parameter arrays
Hello world, the attached patch potentially saves some space in the object file by simplifying access to individual elements of a parameter array, which means that the original parameter may not be needed any more. Regression-tested. OK for trunk? Regards Thomas 2018-03-25 Thomas Koenig PR fortran/51260 * resolve.c (resolve_variable): Simplify cases where access to a parameter array results in a single constant. 2018-03-25 Thomas Koenig PR fortran/51260 * gfortran.dg/parameter_array_element_3.f90: New test. Index: resolve.c === --- resolve.c (Revision 258501) +++ resolve.c (Arbeitskopie) @@ -5577,6 +5577,11 @@ resolve_procedure: if (t && flag_coarray == GFC_FCOARRAY_LIB && gfc_is_coindexed (e)) add_caf_get_intrinsic (e); + /* Simplify cases where access to a parameter array results in a + single constant. */ + if (e->rank == 0 && sym->as && sym->attr.flavor == FL_PARAMETER) +gfc_simplify_expr (e, 1); + return t; } ! { dg-do compile } ! PR 51260 - an unneeded parameter found its way into the ! assembly code. Original test case by Tobias Burnus. module x contains subroutine foo(i) integer, intent(in) :: i end subroutine foo end module x program main use x integer, parameter:: unneeded_parameter (1)=(/(i,i=1,1)/) call foo(unneeded_parameter (1)) print *,unneeded_parameter (1) end program ! { dg-final { scan-assembler-times "unneeded_parameter" 0 } }
Re: [patch fortran] PR 84924 - Erroneous error in C_F_POINTER
Hi Dominique, The attached patch allows scalar noninteroperable scalar derived type with -std=f2003 and -std=f2008. Regstrapped on x86_64-apple-darwin17. OK for trunk? OK. Regards Thomas
Re: [Patch, fortran] PR84931 - Expansion of array constructor with constant implied-do-object goes sideways
Hi Paul, Bootstraps and regtests on FC27/x86_64 - OK for trunk, 7- and 6-branches? OK, and thanks for catching these extra cases! In general, I think we should not modifying original test case unless it is necessary, so I would prefer if you could add the extra tests to a separate test case. Regards Thomas
[patch, fortran, committed] Fix error caused by running front-end optimizatins after error
Hello world, I have just committed r258900 as obvious on trunk to fix an out-of-memory error in front-end optimization, which was caused by gfortran's AST being in an inconsistent state. I suspect that this will also fix other, as yet unknown errors. I will backport to the other affected branches, gcc-7 and gcc-6, over the next few days. Regards Thomas 2018-03-27 Thomas Koenig PR fortran/85084 * frontend-passes.c (gfc_run_passes): Do not run front-end optimizations if a previous error occurred. 2018-03-27 Thomas Koenig PR fortran/85084 * gfortran.dg/matmul_rank_1.f90: New test. ig25@flaemmli:~/Krempel/MMICE> ! { dg-do compile } ! { dg-additional-options "-ffrontend-optimize" } ! PR 85044 - used to die on allocating a negative amount of memory. ! Test case by Gerhard Steinmetz. program p real :: a(3,3) = 1.0 real :: b(33) b = matmul(a, a) ! { dg-error "Incompatible ranks" } end Index: frontend-passes.c === --- frontend-passes.c (revision 258845) +++ frontend-passes.c (working copy) @@ -156,6 +156,10 @@ gfc_run_passes (gfc_namespace *ns) check_locus (ns); #endif + gfc_get_errors (&w, &e); + if (e > 0) +return; + if (flag_frontend_optimize || flag_frontend_loop_interchange) optimize_namespace (ns); @@ -168,10 +172,6 @@ gfc_run_passes (gfc_namespace *ns) expr_array.release (); } - gfc_get_errors (&w, &e); - if (e > 0) - return; - if (flag_realloc_lhs) realloc_strings (ns); }
Re: [patch, fortran] Simplify constants which come from parameter arrays
Hi Dominique, If I am not mistaken, the patch causes: FAIL: gfortran.dg/pr71935.f90 -O (test for excess errors) FAIL: gfortran.dg/substr_6.f90 -O0 execution test FAIL: gfortran.dg/substr_6.f90 -O1 execution test FAIL: gfortran.dg/substr_6.f90 -O2 execution test FAIL: gfortran.dg/substr_6.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions execution test FAIL: gfortran.dg/substr_6.f90 -O3 -g execution test FAIL: gfortran.dg/substr_6.f90 -Os execution test These have been resolved now - I have removed the invalid code from substr_6.f90 (PR85130), and the error is now suppressed in the attached patch. Re-regression-tested. OK for trunk? Regards Thomas 2018-03-25 Thomas Koenig PR fortran/51260 * resolve.c (resolve_variable): Simplify cases where access to a parameter array results in a single constant. 2018-03-25 Thomas Koenig PR fortran/51260 * gfortran.dg/parameter_array_element_3.f90: New test. ! { dg-do compile } ! PR 51260 - an unneeded parameter found its way into the ! assembly code. Original test case by Tobias Burnus. module x contains subroutine foo(i) integer, intent(in) :: i end subroutine foo end module x program main use x integer, parameter:: unneeded_parameter (1)=(/(i,i=1,1)/) call foo(unneeded_parameter (1)) print *,unneeded_parameter (1) end program ! { dg-final { scan-assembler-times "unneeded_parameter" 0 } } Index: resolve.c === --- resolve.c (Revision 258845) +++ resolve.c (Arbeitskopie) @@ -5577,6 +5577,16 @@ resolve_procedure: if (t && flag_coarray == GFC_FCOARRAY_LIB && gfc_is_coindexed (e)) add_caf_get_intrinsic (e); + /* Simplify cases where access to a parameter array results in a + single constant. Suppress errors since those will have been + issued before, as warnings. */ + if (e->rank == 0 && sym->as && sym->attr.flavor == FL_PARAMETER) +{ + gfc_push_suppress_errors (); + gfc_simplify_expr (e, 1); + gfc_pop_suppress_errors (); +} + return t; }
[patch, fortran] Fix PR 85102, take 2
Hello world, here is the second version of the fix for PR 85102. This is a much more general approach, which actually uses simplification (while avoiding some "interesting" regressions when resolving, or simplifying, too soon...) Thanks to Dominique for the hint about the problem with my first patch. Regression-tested. OK for trunk? Regards Thomas 2018-04-02 Thomas Koenig PR fortran/85102 * decl.c (variable_decl): If upper or lower bounds simplify to a constant, use that. 2018-04-02 Thomas Koenig PR fortran/85102 * gfortran.dg/array_simplify_2.f90: New test. ! { dg-do run } ! PR 85102 - this used to ICE ! Original test case by Gerhard Steinmetz program p integer, parameter :: a((1+2)) = 1 integer, parameter :: b((1+1)+1) = 1 integer, parameter :: c = dot_product(a, a) integer, parameter :: d = dot_product(b,b) if (c /= 3) stop 1 if (d /= 3) stop 2 end program p Index: decl.c === --- decl.c (revision 258845) +++ decl.c (working copy) @@ -2424,6 +2424,29 @@ variable_decl (int elem) goto cleanup; } } + if (as->type == AS_EXPLICIT) + { + for (int i = 0; i < as->rank; i++) + { + gfc_expr *e, *n; + e = as->lower[i]; + if (e->expr_type != EXPR_CONSTANT) + { + n = gfc_copy_expr (e); + gfc_simplify_expr (n, 1); + if (n->expr_type == EXPR_CONSTANT) + gfc_replace_expr (e, n); + } + e = as->upper[i]; + if (e->expr_type != EXPR_CONSTANT) + { + n = gfc_copy_expr (e); + gfc_simplify_expr (n, 1); + if (n->expr_type == EXPR_CONSTANT) + gfc_replace_expr (e, n); + } + } + } } char_len = NULL;
Re: [patch, fortran] Fix PR 85102, take 2
Hi Steve, else gfc_free_expr (n); Don't you need the above changes to avoid leaking memory? Correct. OK with those changes? Regards Thomas
Re: [patch, fortran] Simplify constants which come from parameter arrays
Am 03.04.2018 um 17:21 schrieb Dominique d'Humières: Hi Dominique, The new patch regtest fine now. However as said on IRC this looks as a kludge made necessary by a questionable (invalid) test. What I want to avoid is to have first an error and then a warning for the same thing. If we say that doesn't matter, I am also fine with that. IMO it would be more general (better) to call gfc_simplify_expr (e, 1); only when there is no pending error (warning?). That makes less sense than the current approach. We do not want to grow the size of an executable because the code has, let's say, a conversion warning somewhere. I have also a question about "is out of bounds": it is a warning in > resolve.c, but an error in expr.c and simplify.c. Should not it be> an error everywhere? It is inconsistent now. We had this general discussion last year, when adding the -Wdo-subsript code, and I think we should fix this, but not in time for gcc 8. Suggested way forward: Apply the patch as is and open an PR to sort out the warning/error stuff for out-of-bounds access to be resolved consistently for gcc-9. OK? Regards Thomas
[patch, libfortran, committed] Implement stop_numeric for minimal targets
Hello world, the recent patch to make the gfortran and libgomp testsuites more standard conforming, by replacing CALL ABORT() with STOP N, led to numerous testsuite failures on nvptx because stop_numeric was not implemented in minimal.c. I have committed the patch below in r259072 as obvious after Tom de Vries had confirmed that it solves the problem. Regards Thomas 2018-04-04 Thomas Koenig PR libfortran/85166 * runtime/minimal.c (stop_numeric): Add new function in order to implement numeric stop on minimal targets. Index: runtime/minimal.c === --- runtime/minimal.c (Revision 259055) +++ runtime/minimal.c (Arbeitskopie) @@ -187,3 +187,17 @@ abort(); } + +/* A numeric STOP statement. */ + +extern _Noreturn void stop_numeric (int, bool); +export_proto(stop_numeric); + +void +stop_numeric (int code, bool quiet) +{ + if (!quiet) +printf ("STOP %d\n", code); + + exit (code); +}
[patch, libfortran] Fix PR 88235, buffer overrun in matmul
Hello world, the attached patch fixes a buffer overrun in matmul, an 8 regression. No test case since this was only detectable with the address sanitizer or with valgrind. Regression-tested on trunk. OK? Regards Thomas 2018-04-06 Thomas Koenig PR libfortran/85253 * m4/matmul_internal.m4: If ycount == 1, add one more row to the internal buffer. * generated/matmul_c10.c: Regenerated. * generated/matmul_c16.c: Regenerated. * generated/matmul_c4.c: Regenerated. * generated/matmul_c8.c: Regenerated. * generated/matmul_i1.c: Regenerated. * generated/matmul_i16.c: Regenerated. * generated/matmul_i2.c: Regenerated. * generated/matmul_i4.c: Regenerated. * generated/matmul_i8.c: Regenerated. * generated/matmul_r10.c: Regenerated. * generated/matmul_r16.c: Regenerated. * generated/matmul_r4.c: Regenerated. * generated/matmul_r8.c: Regenerated. * generated/matmulavx128_c10.c: Regenerated. * generated/matmulavx128_c16.c: Regenerated. * generated/matmulavx128_c4.c: Regenerated. * generated/matmulavx128_c8.c: Regenerated. * generated/matmulavx128_i1.c: Regenerated. * generated/matmulavx128_i16.c: Regenerated. * generated/matmulavx128_i2.c: Regenerated. * generated/matmulavx128_i4.c: Regenerated. * generated/matmulavx128_i8.c: Regenerated. * generated/matmulavx128_r10.c: Regenerated. * generated/matmulavx128_r16.c: Regenerated. * generated/matmulavx128_r4.c: Regenerated. * generated/matmulavx128_r8.c: Regenerated. Index: m4/matmul_internal.m4 === --- m4/matmul_internal.m4 (Revision 259152) +++ m4/matmul_internal.m4 (Arbeitskopie) @@ -234,7 +234,7 @@ sinclude(`matmul_asm_'rtype_code`.m4')dnl /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; Index: generated/matmul_c10.c === --- generated/matmul_c10.c (Revision 259152) +++ generated/matmul_c10.c (Arbeitskopie) @@ -318,7 +318,7 @@ matmul_c10_avx (gfc_array_c10 * const restrict ret /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; @@ -870,7 +870,7 @@ matmul_c10_avx2 (gfc_array_c10 * const restrict re /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; @@ -1422,7 +1422,7 @@ matmul_c10_avx512f (gfc_array_c10 * const restrict /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; @@ -1988,7 +1988,7 @@ matmul_c10_vanilla (gfc_array_c10 * const restrict /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; @@ -2614,7 +2614,7 @@ matmul_c10 (gfc_array_c10 * const restrict retarra /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; Index: generated/matmul_c16.c === --- generated/matmul_c16.c (Revision 259152) +++ generated/matmul_c16.c (Arbeitskopie) @@ -318,7 +318,7 @@ matmul_c16_avx (gfc_array_c16 * const restrict ret /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; @@ -870,7 +870,7 @@ matmul_c16_avx2 (gfc_array_c16 * const restrict re /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; @@ -1422,7 +1422,7 @@ matmul_c16_avx512f (gfc_array_c16 * const restrict /* Adjust size of t1 to what is needed. */ index_type t1_dim; - t1_dim = (a_dim1-1) * 256 + b_dim1; + t1_dim = (a_dim1 - (ycount > 1)) * 256 + b_dim1; if (t1_dim > 65536) t1_dim = 65536; @@ -1988,7 +1988,7 @@ matmul_c16_vanilla (gfc_array_c16 * const restrict /* Adjust size of t1 to what i
[patch, middle-end] Fix PR 82976, non-trivial conversion at assignment
Hello world, the attached patch fixes a middle-end bug, which caused a Fortran regression. The solution was given by Andrew on IRC and in the PR, I did the testing. Regression-tested with "configure --enable-languages=all" and "make -k check" on x86_64-pc-linux-gnu. OK for trunk? Regards Thomas 2018-04-07 Thomas Koenig Andrew Pinski PR middle-end/82976 * match.pd: Use constant_boolean_node of correct type instead of boolean_true_node or boolean_false_node for simplifying pointer comparisons to zero. 2018-04-07 Thomas Koenig PR middle-end/82976 * gfortran.dg/realloc_on_assign_16a.f90: New test. ! { dg-do compile } ! { dg-options "-Ofast -fno-tree-forwprop" } ! Test that PR 82976 is fixed, this used to ICE. ! ! Contributed by Stefan Mauerberger ! PROGRAM main !USE MPI TYPE :: test_typ REAL, ALLOCATABLE :: a(:) END TYPE TYPE(test_typ) :: xx, yy TYPE(test_typ), ALLOCATABLE :: conc(:) !CALL MPI_INIT(i) xx = test_typ( [1.0,2.0] ) yy = test_typ( [4.0,4.9] ) conc = [ xx, yy ] if (any (int (10.0*conc(1)%a) .ne. [10,20])) STOP 1 if (any (int (10.0*conc(2)%a) .ne. [40,49])) STOP 2 !CALL MPI_FINALIZE(i) END PROGRAM main Index: match.pd === --- match.pd (Revision 259152) +++ match.pd (Arbeitskopie) @@ -3700,7 +3700,7 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) (neeq @0 @1) (if (POINTER_TYPE_P (TREE_TYPE (@0)) && ptrs_compare_unequal (@0, @1)) - { neeq == EQ_EXPR ? boolean_false_node : boolean_true_node; }))) + { constant_boolean_node (neeq != EQ_EXPR, type); }))) /* PR70920: Transform (intptr_t)x eq/ne CST to x eq/ne (typeof x) CST. and (typeof ptr_cst) x eq/ne ptr_cst to x eq/ne (typeof x) CST.
[patch, fortran] Fix PR 85387
Hello world, the attached patch fixes the PR, an 8 regression caused by trying to convert a nested implied DO loop to an array for a case where this was not possible. Regression-tested. OK for trunk? Regards Thomas 2018-04-14 Thomas Koenig PR fortran/85387 * frontend-passes.c (traverse_io_block): Check for start, end or stride being defined by an outer implied DO loop. 2018-04-14 Thomas Koenig PR fortran/85387 * gfortran.dg/implied_do_io_5.f90: New test. Index: frontend-passes.c === --- frontend-passes.c (Revision 259222) +++ frontend-passes.c (Arbeitskopie) @@ -1237,6 +1237,23 @@ traverse_io_block (gfc_code *code, bool *has_reach } } + /* Check for cases like ((a(i, j), i=1, j), j=1, 2). */ + for (int i = 1; i < ref->u.ar.dimen; i++) +{ + if (iters[i]) + { + gfc_expr *var = iters[i]->var; + for (int j = i - 1; j < i; j++) + { + if (iters[j] + && (gfc_check_dependency (var, iters[j]->start, true) + || gfc_check_dependency (var, iters[j]->end, true) + || gfc_check_dependency (var, iters[j]->step, true))) + return false; + } + } +} + /* Create new expr. */ new_e = gfc_copy_expr (curr->expr1); new_e->expr_type = EXPR_VARIABLE; ! { dg-do run } ! { dg-additional-options "-ffrontend-optimize" } ! PR fortran/85387 - incorrect output ! Original test case by Vittorio Zecca program main real :: efg_pw(2,2) character (len=80) :: c1, c2 efg_pw(1,1)=1 efg_pw(2,1)=2 efg_pw(1,2)=3 efg_pw(2,2)=4 write (unit=c1,fmt='(3F12.5)') ((efg_pw(i, j), i=1, j), j=1, 2) write (unit=c2,fmt='(3F12.5)') 1.0, 3.0, 4.0 if (c1 /= c2) stop 1 end
Re: [patch, fortran] Fix PR 85387
Hi Andre, this looks good. Ok for trunk. Thanks for the patch. Committed as r259384. Thanks for the quick review! Looking at the serious regressions from the gcc home page, we are fast approaching the gcc 8 release. Serious regressions are below 100, and there are currently only So, if anybody has any bugs that should urgently be fixed for the release, please go ahead :-) Regards Thomas
[patch, rfc] Clobber scalar intent(out) variables on entry
Hi, the patch below tries to clobber scalar intent(out) arguments on procedure entry. Index: trans-decl.c === --- trans-decl.c(Revision 264423) +++ trans-decl.c(Arbeitskopie) @@ -4143,6 +4143,19 @@ init_intent_out_dt (gfc_symbol * proc_sym, gfc_wra gfc_add_expr_to_block (&init, tmp); } +else if (f->sym->attr.dummy && !f->sym->attr.dimension +&& f->sym->attr.intent == INTENT_OUT +&& !f->sym->attr.codimension && !f->sym->attr.allocatable +&& (f->sym->ts.type != BT_CLASS +|| (!CLASS_DATA (f->sym)->attr.dimension +&& !(CLASS_DATA (f->sym)->attr.codimension + && CLASS_DATA (f->sym)->attr.allocatable + { + tree t1, t2; + t1 = build_fold_indirect_ref_loc (input_location, f->sym->backend_decl); + t2 = build_clobber (TREE_TYPE (t1)); + gfc_add_modify (&init, t1, t2); + } gfc_add_init_cleanup (block, gfc_finish_block (&init), NULL_TREE); } With this patch, module x contains subroutine foo(a) real, intent(out) :: a a = 21. a = a + 22. end subroutine foo end module x generates, with -fdump-tree-original foo (real(kind=4) & restrict a) { *a = {CLOBBER}; *a = 2.1e+1; *a = *a + 2.2e+1; } Is this the right way to proceed? (The if statement is not yet correct, so this version causes regressions, that would have to be adjusted). Regards Thomas
[patch, fortran, committed] Another fallout from clobbering INTENT(OUT) variables
Hello world, committed as obvious after regression-testing. Instead of spending a lot of work trying to reducing the test case, I used all of it (retainging the copyright notice). Regards Thomas 2018-09-23 Thomas Koenig PR fortran/87397 * gfc_conv_procedure_call: Do not add clobber on INTENT(OUT) for variables having the dimension attribute. 2018-09-23 Thomas Koenig PR fortran/87397 * gfortran.dg/intent_out_11.f90: New test. Index: trans-expr.c === --- trans-expr.c (Revision 264512) +++ trans-expr.c (Arbeitskopie) @@ -5280,6 +5280,7 @@ gfc_conv_procedure_call (gfc_se * se, gfc_symbol * bool add_clobber; add_clobber = fsym && fsym->attr.intent == INTENT_OUT && !fsym->attr.allocatable && !fsym->attr.pointer + && !e->symtree->n.sym->attr.dimension && !e->symtree->n.sym->attr.pointer /* See PR 41453. */ && !e->symtree->n.sym->attr.dummy ! { dg-do compile } ! { dg-options "-cpp -fcoarray=lib" } ! PR 87397 - this used to generate an ICE. ! Coarray Distributed Transpose Test ! ! Copyright (c) 2012-2014, Sourcery, Inc. ! All rights reserved. ! ! Redistribution and use in source and binary forms, with or without ! modification, are permitted provided that the following conditions are met: ! * Redistributions of source code must retain the above copyright ! notice, this list of conditions and the following disclaimer. ! * Redistributions in binary form must reproduce the above copyright ! notice, this list of conditions and the following disclaimer in the ! documentation and/or other materials provided with the distribution. ! * Neither the name of the Sourcery, Inc., nor the ! names of its contributors may be used to endorse or promote products ! derived from this software without specific prior written permission. ! ! THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ! ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED ! WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE ! DISCLAIMED. IN NO EVENT SHALL SOURCERY, INC., BE LIABLE FOR ANY ! DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES ! (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; ! LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ! ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT ! (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS ! ! Robodoc header: !m* dist_transpose/run_size ! NAME ! run_size ! SYNOPSIS ! Encapsulate problem state, wall-clock timer interface, integer broadcasts, and a data copy. !** !== test transposes with integer x,y,z values === module run_size use iso_fortran_env implicit none integer(int64), codimension[*] :: nx, ny, nz integer(int64), codimension[*] :: my, mx, first_y, last_y, first_x, last_x integer(int64) :: my_node, num_nodes real(real64), codimension[*] :: tran_time contains !s* run_size/broadcast_int ! NAME ! broadcast_int ! SYNOPSIS ! Broadcast a scalar coarray integer from image 1 to all other images. !** subroutine broadcast_int( variable ) integer(int64), codimension[*] :: variable integer(int64) :: i if( my_node == 1 ) then do i = 2, num_nodes;variable[i] = variable; end do end if end subroutine broadcast_int subroutine copy3( A,B, n1, sA1, sB1, n2, sA2, sB2, n3, sA3, sB3 ) implicit none complex, intent(in) :: A(0:*) complex, intent(out) :: B(0:*) integer(int64), intent(in) :: n1, sA1, sB1 integer(int64), intent(in) :: n2, sA2, sB2 integer(int64), intent(in) :: n3, sA3, sB3 integer(int64) i,j,k do k=0,n3-1 do j=0,n2-1 do i=0,n1-1 B(i*sB1+j*sB2+k*sB3) = A(i*sA1+j*sA2+k*sA3) end do end do end do end subroutine copy3 end module run_size !e* dist_transpose/coarray_distributed_transpose ! NAME ! coarray_distributed_transpose ! SYNOPSIS ! This program tests the transpose routines used in Fourier-spectral simulations of homogeneous turbulence. ! The data is presented to the physics routines as groups of y-z or x-z planes distributed among the images. ! The (out-of-place) transpose routines do the x <--> y transposes required and consist of transposes within ! data blocks (intra-image) and a transpose of the distribution of these blocks among the images (inter-image). ! ! Two methods are tested here: ! RECEIVE: receive block from other image and transpose it ! SEND:transpose block and send it to other image ! ! This code is the coarray analog of mpi_distributed_transpose. !** program coarray_distributed_transpose !(***
Re: PR85463 '[nvptx] "exit" in offloaded region doesn't terminate process' (was: [patch, libfortran, committed] Implement stop_numeric for minimal targets)
> Mapping exit to abort is weird, For Fortran, this is mapping STOP with a numeric code to abort. The Fortran standard does not apply in this case. What does the OpenACC standard say about STOP in an offloaded region? Regards, Thomas
Re: PR85463 '[nvptx] "exit" in offloaded region doesn't terminate process'
Am 19.04.2018 um 13:59 schrieb Thomas Schwinge: The Fortran standard does not apply in this case. What does the OpenACC standard say about STOP in an offloaded region? Nothing explicitly, as far as I know. ;-/ Which means, that this either a) has to be forbidden, or b) some common sense implementation is called for. Well, implicitly it's meant such that "standard Fortran language usage" is supported inside such offloading regions. And, as code like: !$ACC PARALLEL [compute A] if (.not. [sanity check computation A]) then stop 1 end if [compute B, using A] !$ACC END PARALLEL [compute C, using A and B] ... certainly is a reasonable thing to support, option b) clearly is preferrable over option a). Well, if the Fortran standard may be relevant after all, let's look at what it has to say. J3/10-007 # 8.4 STOP and ERROR STOP statements # When an image is terminated by a STOP or ERROR STOP statement, its # stop code, if any, is made available in a processor-dependent manner. Well, an offloading region is not a Fortran image, but at least it is something vaguely similar. Also, it is unclear to whom this stop code is made available. Having said that, the spirit, if not the letter, of this sentence certainly supports that something other than silently ignoring the error should be done. Regards Thomas
[patch, fortran, doc, committed] Document BACK for MINLOC and MAXLOC
Hello world, I just commmitted the attached patch as obvious after checking that it passes "make info", "make dvi" and "make pdf". Regards Thomas 2018-05-10 Thomas Koenig PR fortran/54613 * intrinsic.texi: Document BACK for MINLOC and MAXLOC. Index: intrinsic.texi === --- intrinsic.texi (Revision 260022) +++ intrinsic.texi (Arbeitskopie) @@ -9991,8 +9991,10 @@ locations of the maximum element along each row of @var{DIM} direction. If @var{MASK} is present, only the elements for which @var{MASK} is @code{.TRUE.} are considered. If more than one element in the array has the maximum value, the location returned is -that of the first such element in array element order. If the array has -zero size, or all of the elements of @var{MASK} are @code{.FALSE.}, then +that of the first such element in array element order if the +@var{BACK} is not present, or if it false; otherwise, the location +returned is that of the first such element. If the array has zero +size, or all of the elements of @var{MASK} are @code{.FALSE.}, then the result is an array of zeroes. Similarly, if @var{DIM} is supplied and all of the elements of @var{MASK} along a given row are zero, the result value for that row is zero. @@ -1,6 +10002,7 @@ result value for that row is zero. @item @emph{Standard}: Fortran 95 and later; @var{ARRAY} of @code{CHARACTER} and the @var{KIND} argument are available in Fortran 2003 and later. +The @var{BACK} argument is available in Fortran 2008 and later. @item @emph{Class}: Transformational function @@ -10006,8 +10009,8 @@ Transformational function @item @emph{Syntax}: @multitable @columnfractions .80 -@item @code{RESULT = MAXLOC(ARRAY, DIM [, MASK] [,KIND])} -@item @code{RESULT = MAXLOC(ARRAY [, MASK] [,KIND])} +@item @code{RESULT = MAXLOC(ARRAY, DIM [, MASK] [,KIND] [,BACK])} +@item @code{RESULT = MAXLOC(ARRAY [, MASK] [,KIND] [,BACK])} @end multitable @item @emph{Arguments}: @@ -10021,6 +10024,7 @@ inclusive. It may not be an optional dummy argume and conformable with @var{ARRAY}. @item @var{KIND} @tab (Optional) An @code{INTEGER} initialization expression indicating the kind parameter of the result. +@item @var{BACK} @tab (Optional) A scalar of type @code{LOGICAL}. @end multitable @item @emph{Return value}: @@ -10343,7 +10347,9 @@ locations of the minimum element along each row of @var{DIM} direction. If @var{MASK} is present, only the elements for which @var{MASK} is @code{.TRUE.} are considered. If more than one element in the array has the minimum value, the location returned is -that of the first such element in array element order. If the array has +that of the first such element in array element order if the +@var{BACK} is not present, or if it false; otherwise, the location +returned is that of the first such element. If the array has zero size, or all of the elements of @var{MASK} are @code{.FALSE.}, then the result is an array of zeroes. Similarly, if @var{DIM} is supplied and all of the elements of @var{MASK} along a given row are zero, the @@ -10352,6 +10358,7 @@ result value for that row is zero. @item @emph{Standard}: Fortran 95 and later; @var{ARRAY} of @code{CHARACTER} and the @var{KIND} argument are available in Fortran 2003 and later. +The @var{BACK} argument is available in Fortran 2008 and later. @item @emph{Class}: Transformational function @@ -10358,8 +10365,8 @@ Transformational function @item @emph{Syntax}: @multitable @columnfractions .80 -@item @code{RESULT = MINLOC(ARRAY, DIM [, MASK] [,KIND])} -@item @code{RESULT = MINLOC(ARRAY [, MASK], [,KIND])} +@item @code{RESULT = MINLOC(ARRAY, DIM [, MASK] [,KIND] [,BACK])} +@item @code{RESULT = MINLOC(ARRAY [, MASK], [,KIND] [,BACK])} @end multitable @item @emph{Arguments}: @@ -10373,6 +10380,7 @@ inclusive. It may not be an optional dummy argume and conformable with @var{ARRAY}. @item @var{KIND} @tab (Optional) An @code{INTEGER} initialization expression indicating the kind parameter of the result. +@item @var{BACK} @tab (Optional) A scalar of type @code{LOGICAL}. @end multitable @item @emph{Return value}:
Re: Document PR 84073 change in /gcc-8/porting_to.html
Am 10.05.2018 um 14:20 schrieb Thomas Koenig: Am 10.05.2018 um 12:33 schrieb Jonathan Wakely: Should the fix for PR 84073 be documented, so that users whose code is now rejected understand why, and how to fix it? https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84073 https://gcc.gnu.org/gcc-8/porting_to.html#fortran Sounds like a good idea. Since I introduced this, I'll do it within a few days (unless, of course, somebody beats me to it, which I won't mind :-) OK, not a few days, but a few hours :-) Here is the diff. OK to commit to gcc-docs? Regards Thomas Index: porting_to.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-8/porting_to.html,v retrieving revision 1.6 diff -u -r1.6 porting_to.html --- porting_to.html 4 May 2018 06:35:39 - 1.6 +++ porting_to.html 10 May 2018 15:16:57 - @@ -196,7 +196,31 @@ void foo_ (char*, int*, fortran_charlen_t); + + Versions of gfortran prior to 8.1 wrongly accepted CHARACTER + variables with a length type parameter other than one as C + interoperable. For example, the code + + module mod +use iso_c_binding +type, bind(C) :: a + character(len=2,kind=c_char) :: b ! Wrong +end type a +character(len=2), bind(C) :: c ! Also wrong + end module mod + + was accepted. To achieve similar functionality, an array of + LEN=1 characters can be used, for example + + module mod +use iso_c_binding +type, bind(C) :: a + character(kind=c_char) :: b(2) +end type a +character(kind=c_char), bind(C) :: c(2) + end module mod + + Links -
[patch, fortran] Fix PR 90561
Hello world, this patch fixes a 9/10 regression by placing the variable used to hold a string length at function scope. I chose to implement this version of gfc_evaluate_now as a separate function because I have a sneaking suspicion this may not be the last time we are going to encounter something like that - better have the function there for future use. Regression-tested. OK for trunk and gcc-9? Regards Thomas Index: trans.h === --- trans.h (Revision 274370) +++ trans.h (Arbeitskopie) @@ -507,6 +507,7 @@ void gfc_conv_label_variable (gfc_se * se, gfc_exp /* If the value is not constant, Create a temporary and copy the value. */ tree gfc_evaluate_now_loc (location_t, tree, stmtblock_t *); tree gfc_evaluate_now (tree, stmtblock_t *); +tree gfc_evaluate_now_function_scope (tree, stmtblock_t *); /* Find the appropriate variant of a math intrinsic. */ tree gfc_builtin_decl_for_float_kind (enum built_in_function, int); Index: trans.c === --- trans.c (Revision 274370) +++ trans.c (Arbeitskopie) @@ -118,7 +118,20 @@ gfc_evaluate_now (tree expr, stmtblock_t * pblock) return gfc_evaluate_now_loc (input_location, expr, pblock); } +/* Like gfc_evaluate_now, but add the created variable to the + function scope. */ +tree +gfc_evaluate_now_function_scope (tree expr, stmtblock_t * pblock) +{ + tree var; + var = gfc_create_var_np (TREE_TYPE (expr), NULL); + gfc_add_decl_to_function (var); + gfc_add_modify (pblock, var, expr); + + return var; +} + /* Build a MODIFY_EXPR node and add it to a given statement block PBLOCK. A MODIFY_EXPR is an assignment: LHS <- RHS. */ Index: trans-expr.c === --- trans-expr.c (Revision 274370) +++ trans-expr.c (Arbeitskopie) @@ -10796,7 +10796,8 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr if (expr1->ts.deferred && gfc_expr_attr (expr1).allocatable && gfc_check_dependency (expr1, expr2, true)) - rse.string_length = gfc_evaluate_now (rse.string_length, &rse.pre); + rse.string_length = + gfc_evaluate_now_function_scope (rse.string_length, &rse.pre); string_length = rse.string_length; } else ! { dg-do run } ! PR fortran/90561 ! This used to ICE. ! Original test case by Gerhard Steinmetz. program p character(:), allocatable :: z(:) z = [character(2):: 'ab', 'xy'] z = z(2) if (any(z /= 'xy')) stop 1 end
[patch, fortran] Fix PR 91390 - treatment of extra parameter in a subroutine call
Hello world, here is the next installment of checking for mismatched calls, this time for mismatching CALLs. The solution is to build a separate namespace with procedure arguments determined from the actual arguments the first time a procedure is seen, and then compare it against that on subsequent calls. This has uncovered quite a few examples of non-conforming code in our testsuite, so no separate test case needed, IMHO. So, OK for trunk? (The -std=legacy question can be settled later). 2019-08-20 Thomas Koenig PR fortran/91390 * frontend-passes.c (check_externals_procedure): New function. If a procedure is not in the translation unit, create an "interface" for it, including its formal arguments. (check_externals_code): Use check_externals_procedure for common code with check_externals_expr. (check_externals_expr): Vice versa. * gfortran.h (gfc_get_formal_from_actual-arglist): New prototype. (gfc_compare_actual_formal): New prototype. * interface.c (compare_actual_formal): Rename to (gfc_compare_actual_forma): New function, make global. (gfc_get_formal_from_actual_arglist): Make global, and move here from * trans-types.c (get_formal_from_actual_arglist): Remove here. (gfc_get_function_type): Use gfc_get_formal_from_actual_arglist. 2019-08-20 Thomas Koenig PR fortran/91390 * gfortran.dg/bessel_3.f90: Add type mismatch errors. * gfortran.dg/coarray_7.f90: Rename subroutines to avoid additional errors. * gfortran.dg/g77/20010519-1.f: Add -std=legacy. Remove warnings for ASSIGN. Add warnings for type mismatch. * gfortran.dg/goacc/acc_on_device-1.f95: Add -std=legacy. Add cath-all warning. * gfortran.dg/internal_pack_9.f90: Rename subroutine to avoid type error. * gfortran.dg/internal_pack_9.f90: Add -std=legacy. Add warnings for type mismatch. * gfortran.dg/pr39937.f: Add -std=legacy and type warnings. Move here from * gfortran.fortran-torture/compile/pr39937.f: Move to gfortran.dg. Index: fortran/frontend-passes.c === --- fortran/frontend-passes.c (Revision 274623) +++ fortran/frontend-passes.c (Arbeitskopie) @@ -5369,25 +5369,22 @@ gfc_code_walker (gfc_code **c, walk_code_fn_t code 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. */ +/* Common tests for argument checking for both functions and subroutines. */ + static int -check_externals_expr (gfc_expr **ep, int *walk_subtrees ATTRIBUTE_UNUSED, - void *data ATTRIBUTE_UNUSED) +check_externals_procedure (gfc_symbol *sym, locus *loc, gfc_actual_arglist *actual) { - gfc_expr *e = *ep; - gfc_symbol *sym, *def_sym; gfc_gsymbol *gsym; + gfc_symbol *def_sym = NULL; - if (e->expr_type != EXPR_FUNCTION) + if (sym == NULL || sym->attr.is_bind_c) return 0; - sym = e->value.function.esym; - - if (sym == NULL || sym->attr.is_bind_c) + if (sym->attr.proc != PROC_EXTERNAL && sym->attr.proc != PROC_UNKNOWN) return 0; - if (sym->attr.proc != PROC_EXTERNAL && sym->attr.proc != PROC_UNKNOWN) + if (sym->attr.if_source == IFSRC_IFBODY || sym->attr.if_source == IFSRC_DECL) return 0; gsym = gfc_find_gsymbol (gfc_gsym_root, sym->name); @@ -5394,15 +5391,39 @@ static int if (gsym == NULL) return 0; - gfc_find_symbol (sym->name, gsym->ns, 0, &def_sym); + if (gsym->ns) +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); + if (def_sym) +{ + gfc_procedure_use (def_sym, &actual, loc); + return 0; +} + /* First time we have seen this procedure called. Let's create an + "interface" from the call and put it into a new namespace. */ + gfc_namespace *save_ns; + gfc_symbol *new_sym; + + gsym->where = *loc; + save_ns = gfc_current_ns; + gsym->ns = gfc_get_namespace (gfc_current_ns, 0); + gsym->ns->proc_name = sym; + + gfc_get_symbol (sym->name, gsym->ns, &new_sym); + gcc_assert (new_sym); + new_sym->attr = sym->attr; + new_sym->attr.if_source = IFSRC_DECL; + gfc_current_ns = gsym->ns; + + gfc_get_formal_from_actual_arglist (new_sym, actual); + gfc_current_ns = save_ns; + return 0; + } -/* Callback for external code. */ +/* Callback for calls of external routines. */ static int check_externals_code (gfc_code **c, int *walk_subtrees ATTRIBUTE_UNUSED, @@ -5409,32 +5430,43 @@ check_externals_code (gfc_code **c, int *walk_subt void *data ATTRIBUTE_UNUSED) { gfc_code *co = *c; - gfc_symbol *sym, *def_sym; - gfc_gsymbol *gsym; + gfc_symbol *sym; + locus *loc; + gfc_actual_arglist *actual; if (co->op != EXEC_CALL) return 0; sym = co-
Re: [patch, fortran] Fix PR 91390 - treatment of extra parameter in a subroutine call
Hi Steve, OK. Thanks for taking on this task. Committed (r274902). Thanks for the review! As to the open question about how to handle this check, I would create -fallow-argument-mismatch (or whatever option name you like). gfortran issues an error if a mismatch is detected. -fallow-... would reduce the error to warning, which can only be silenced with -w. Hopefully, this will encourage users to fix the code. Yes, that is what I will submit next. -fallow-argument-mismatch sounds like a good name, unless somebody else comes up with a better name. Regards Thomas
[patch, doc, fortran] Document FINDLOC
Hi, here is the promised documentation for FINDLOC. Tested with "make dvi" and "make pdf". OK for trunk? Regards Thomas 2018-10-28 Thomas Koenig PR fortran/54613 * gfortran.texi (File format of unformatted sequential files): Replace random comma with period. * intrinsic.texi (Intrinsic Procedures): Add FINDLOC to menu. (FINDLOC): Document. (MAXLOC): Add refrence to FINDLOC. (MINLOC): Likewise. Index: gfortran.texi === --- gfortran.texi (Revision 265569) +++ gfortran.texi (Arbeitskopie) @@ -1479,7 +1479,7 @@ contains a negative number, then there is a preced In the most simple case, with only one subrecord per logical record, both record markers contain the number of bytes of user data in the -record, +record. The format for unformatted sequential data can be duplicated using unformatted stream, as shown in the example program for an unformatted Index: intrinsic.texi === --- intrinsic.texi (Revision 265569) +++ intrinsic.texi (Arbeitskopie) @@ -148,6 +148,7 @@ Some basic guidelines for editing this document: * @code{FDATE}: FDATE, Subroutine (or function) to get the current time as a string * @code{FGET}: FGET, Read a single character in stream mode from stdin * @code{FGETC}: FGETC, Read a single character in stream mode +* @code{FINDLOC}: FINDLOC, Search an array for a value * @code{FLOOR}: FLOOR, Integer floor function * @code{FLUSH}: FLUSH, Flush I/O unit(s) * @code{FNUM}: FNUM, File number function @@ -6021,8 +6022,68 @@ END PROGRAM @ref{FGET}, @ref{FPUT}, @ref{FPUTC} @end table +@node FINDLOC +@section @code{FINDLOC} --- Search an array for a value +@fnindex FINDLOC +@cindex findloc +@table @asis +@item @emph{Description}: +Determines the location of the element in the array with the value +given in the @var{VALUE} argument, or, if the @var{DIM} argument is +supplied, determines the locations of the maximum element along each +row of the array in the @var{DIM} direction. If @var{MASK} is present, +only the elements for which @var{MASK} is @code{.TRUE.} are +considered. If more than one element in the array has the value +@var{VALUE}, the location returned is that of the first such element +in array element order if the @var{BACK} is not present, or if it +false; otherwise, the location returned is that of the first such +element. If the array has zero size, or all of the elements of +@var{MASK} are @code{.FALSE.}, then the result is an array of zeroes. +Similarly, if @var{DIM} is supplied and all of the elements of +@var{MASK} along a given row are zero, the result value for that row +is zero. +@item @emph{Standard}: +Fortran 2008 and later. + +@item @emph{Class}: +Transformational function + +@item @emph{Syntax}: +@multitable @columnfractions .80 +@item @code{RESULT = FINDLOC(ARRAY, VALUE, DIM [, MASK] [,KIND] [,BACK])} +@item @code{RESULT = FINDLOC(ARRAY, VALUE, [, MASK] [,KIND] [,BACK])} +@end multitable + +@item @emph{Arguments}: +@multitable @columnfractions .15 .70 +@item @var{ARRAY} @tab Shall be an array of intrinsic type. +@item @var{VALUE} @tab A scalar of intrinsic type which is in type +conformance with @var{ARRAY}. +@item @var{DIM} @tab (Optional) Shall be a scalar of type +@code{INTEGER}, with a value between one and the rank of @var{ARRAY}, +inclusive. It may not be an optional dummy argument. +@item @var{KIND} @tab (Optional) An @code{INTEGER} initialization +expression indicating the kind parameter of the result. +@item @var{BACK} @tab (Optional) A scalar of type @code{LOGICAL}. +@end multitable + +@item @emph{Return value}: +If @var{DIM} is absent, the result is a rank-one array with a length +equal to the rank of @var{ARRAY}. If @var{DIM} is present, the result +is an array with a rank one less than the rank of @var{ARRAY}, and a +size corresponding to the size of @var{ARRAY} with the @var{DIM} +dimension removed. If @var{DIM} is present and @var{ARRAY} has a rank +of one, the result is a scalar. If the optional argument @var{KIND} +is present, the result is an integer of kind @var{KIND}, otherwise it +is of default kind. + +@item @emph{See also}: +@ref{MAXLOC}, @ref{MINLOC} + +@end table + @node FLOOR @section @code{FLOOR} --- Integer floor function @fnindex FLOOR @@ -10039,7 +10100,7 @@ is present, the result is an integer of kind @var{ is of default kind. @item @emph{See also}: -@ref{MAX}, @ref{MAXVAL} +@ref{FINDLOC}, @ref{MAX}, @ref{MAXVAL} @end table @@ -10395,7 +10456,7 @@ is present, the result is an integer of kind @var{ is of default kind. @item @emph{See also}: -@ref{MIN}, @ref{MINVAL} +@ref{FINDLOC}, @ref{MIN}, @ref{MINVAL} @end table
Re: [patch, doc, fortran] Document FINDLOC
Am 29.10.18 um 12:31 schrieb Bernhard Reutner-Fischer: Hi! On Sun, 28 Oct 2018 15:13:49 +0100 Thomas König wrote: One question and some nits below. === --- intrinsic.texi (Revision 265569) +++ intrinsic.texi (Arbeitskopie) @@ -6021,8 +6022,68 @@ END PROGRAM @ref{FGET}, @ref{FPUT}, @ref{FPUTC} @end table +@node FINDLOC +@section @code{FINDLOC} --- Search an array for a value I think one should not put spaces around triple-dashes. We have it everywhere else. Should we change this in general? If so, that would probably make sense as a followup patch. DIM} direction. If @var{MASK} is present, dot-space-space at end of sentence: direction. If Done. +only the elements for which @var{MASK} is @code{.TRUE.} are +considered. If more than one element in the array has the value +@var{VALUE}, the location returned is that of the first such element +in array element order if the @var{BACK} is not present, or if it missing "is": ...not present or if it is @code{.FALSE.}. +false; otherwise, the location returned is that of the first such +element. If the array has zero size, or all of the elements of I think this should mention BACK=.TRUE. for clarity; and it should refer to the _last_ such element, not the first, doesn't it? Reordered, to make it more sensible. Excess spaces between @var{DIM} and @tab? Removed. Excess third space at the start of the sentence: dot-space-space Removed. Unless there are other comments, I'll commit in a couple of days. Regards Thomas Index: gfortran.texi === --- gfortran.texi (Revision 265569) +++ gfortran.texi (Arbeitskopie) @@ -1479,7 +1479,7 @@ contains a negative number, then there is a preced In the most simple case, with only one subrecord per logical record, both record markers contain the number of bytes of user data in the -record, +record. The format for unformatted sequential data can be duplicated using unformatted stream, as shown in the example program for an unformatted Index: intrinsic.texi === --- intrinsic.texi (Revision 265569) +++ intrinsic.texi (Arbeitskopie) @@ -148,6 +148,7 @@ Some basic guidelines for editing this document: * @code{FDATE}: FDATE, Subroutine (or function) to get the current time as a string * @code{FGET}: FGET, Read a single character in stream mode from stdin * @code{FGETC}: FGETC, Read a single character in stream mode +* @code{FINDLOC}: FINDLOC, Search an array for a value * @code{FLOOR}: FLOOR, Integer floor function * @code{FLUSH}: FLUSH, Flush I/O unit(s) * @code{FNUM}: FNUM, File number function @@ -6021,8 +6022,68 @@ END PROGRAM @ref{FGET}, @ref{FPUT}, @ref{FPUTC} @end table +@node FINDLOC +@section @code{FINDLOC} --- Search an array for a value +@fnindex FINDLOC +@cindex findloc +@table @asis +@item @emph{Description}: +Determines the location of the element in the array with the value +given in the @var{VALUE} argument, or, if the @var{DIM} argument is +supplied, determines the locations of the maximum element along each +row of the array in the @var{DIM} direction. If @var{MASK} is +present, only the elements for which @var{MASK} is @code{.TRUE.} are +considered. If more than one element in the array has the value +@var{VALUE}, the location returned is that of the first such element +in array element order if the @var{BACK} is not present or if it is +@code{.FALSE.}. If @var{BACK} is true, the location returned is that +of the last such element. If the array has zero size, or all of the +elements of @var{MASK} are @code{.FALSE.}, then the result is an array +of zeroes. Similarly, if @var{DIM} is supplied and all of the +elements of @var{MASK} along a given row are zero, the result value +for that row is zero. +@item @emph{Standard}: +Fortran 2008 and later. + +@item @emph{Class}: +Transformational function + +@item @emph{Syntax}: +@multitable @columnfractions .80 +@item @code{RESULT = FINDLOC(ARRAY, VALUE, DIM [, MASK] [,KIND] [,BACK])} +@item @code{RESULT = FINDLOC(ARRAY, VALUE, [, MASK] [,KIND] [,BACK])} +@end multitable + +@item @emph{Arguments}: +@multitable @columnfractions .15 .70 +@item @var{ARRAY} @tab Shall be an array of intrinsic type. +@item @var{VALUE} @tab A scalar of intrinsic type which is in type +conformance with @var{ARRAY}. +@item @var{DIM} @tab (Optional) Shall be a scalar of type +@code{INTEGER}, with a value between one and the rank of @var{ARRAY}, +inclusive. It may not be an optional dummy argument. +@item @var{KIND} @tab (Optional) An @code{INTEGER} initialization +expression indicating the kind parameter of the result. +@item @var{BACK} @tab (Optional) A scalar of type @code{LOGICAL}. +@end multitable + +@item @emph{Return valu
Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.
Hi Martin, > Thank you both useful comments! I installed that as r269035. Excellent work, thanks! Now a final step: Could you also add a short sentence to gcc-9/changes.html ? In German, we have a saying „Tue Gutes und rede darüber“, do good deeds and talk about it 😉 Regards, Thomas
Re: [PATCH][RFC] Extend locations where to seach for Fortran pre-include.
Hi Martin, I would change "among" to "between" in the sentence below. OK otherwise (not that I think an OK is really needed in this case). + The purpose of the directive is to provide an API among the GCC compiler and +the GNU C Library which would define vector implementations of math routines. Regards Thomas
[patch, fortran] Implement debug() for some Fortran data structures
Hello world, the attached patch implements a few versions of debug for Fortran data structures. It lets you type, for example (gdb) call debug (expr) and get some output which is readable, instead of having to look at the structures yourself. This has no user impact, because thsese functions are never called from the compiler itself. Comments, suggestions etc welcome. I will commit in a couple of days unless there are objections. Once that is done, I will also adjust the quickstart guide on the wiki. Oh yes, the code compiles :-) Regards Thomas
Re: [patch, fortran] Implement debug() for some Fortran data structures
Am 20.02.19 um 21:36 schrieb Steve Kargl: On Wed, Feb 20, 2019 at 09:10:51PM +0100, Thomas König wrote: Hello world, the attached patch implements a few versions of debug for Fortran data structures. It lets you type, for example (gdb) call debug (expr) and get some output which is readable, instead of having to look at the structures yourself. This has no user impact, because thsese functions are never called from the compiler itself. Comments, suggestions etc welcome. I will commit in a couple of days unless there are objections. Once that is done, I will also adjust the quickstart guide on the wiki. Oh yes, the code compiles :-) Did you mean to attach a patch? Well, yes :-) Index: dump-parse-tree.c === --- dump-parse-tree.c (Revision 268968) +++ dump-parse-tree.c (Arbeitskopie) @@ -48,11 +48,37 @@ static void show_expr (gfc_expr *p); static void show_code_node (int, gfc_code *); static void show_namespace (gfc_namespace *ns); static void show_code (int, gfc_code *); +static void show_symbol (gfc_symbol *); +static void show_typespec (gfc_typespec *); - /* Allow dumping of an expression in the debugger. */ void gfc_debug_expr (gfc_expr *); +void debug (gfc_expr *e) +{ + FILE *tmp = dumpfile; + dumpfile = stderr; + show_expr (e); + fputc (' ', dumpfile); + show_typespec (&e->ts); + fputc ('\n', dumpfile); + dumpfile = tmp; +} + +void debug (gfc_typespec *ts) +{ + FILE *tmp = dumpfile; + dumpfile = stderr; + show_typespec (ts); + fputc ('\n', dumpfile); + dumpfile = tmp; +} + +void debug (gfc_typespec ts) +{ + debug (&ts); +} + void gfc_debug_expr (gfc_expr *e) { @@ -76,6 +102,15 @@ gfc_debug_code (gfc_code *c) dumpfile = tmp; } +void debug (gfc_symbol *sym) +{ + FILE *tmp = dumpfile; + dumpfile = stderr; + show_symbol (sym); + fputc ('\n', dumpfile); + dumpfile = tmp; +} + /* Do indentation for a specific level. */ static inline void
Re: [patch, fortran] Fix PR 89174, segfault on allocate with MOLD
Hi Steve, I think this was introduced quite some time ago, not sure if it was ever documented anywhere. I guess we should do so. Probably want to document this in the testcase. I just checked and found 77 occurences in the test suite (most of them mine, to be sure). So, maybe an entry in the wiki would be more appropriate. I am just trying to think of who introduced this (or if this is even gfortran specific), but coming up blank. Does anybody remember? Regards Thomas
[patch, fortran] Fix PR 89496, error with alternate return
Hello world, the attached patch fixes a regression introduced by my recent patch for PR 87689, where the alternate return arguments were not handled correctly. Some experimentation resulted in a test case which actually segfaulted on a normal compiler, instead of just being visible on an instrumened one. The test case also checks that the alternate return runs correctly (and uses the "dg-do run" hack :-) I also checked this with -flto and by inspecting the output of -fdump-fortran-original. Thanks to Martin Liska for the bug report. Regression-tested. OK for trunk? Regards Thomas 2019-02-25 Thomas Koenig PR fortran/89496 * trans-types.c (get_formal_from_actual_arglist): If the actual arglist has no expression, the corresponding formal arglist is an alternate return. 2019-02-25 Thomas Koenig PR fortran/89496 * gfortran.dg/altreturn_9_0.f90: New file. * gfortran.dg/altreturn_9_1.f90: New file.
Re: [PATCH] Fix libgfortran/caf/single.c warnings (PR libgfortran/89593)
Hi Jakub, Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Yes (also could be considered obvious, I guess). Thanks for the patch! Regards Thomas
[patch, fortran] Fix the rest of PR 71203
Hello world, the attached patch fixes the rest of PR 71203 (i.e. the non-character parts). I have checked in gdb that the shape set with this patch does indeed not create a memory leak. Regression-tested. OK for trunk? (This makes three patches that are currently awaiting review, the other two are https://gcc.gnu.org/ml/fortran/2019-03/msg00022.html and https://gcc.gnu.org/ml/fortran/2019-02/msg00241.html. I'd appreciate somebody looking at these, I think I am close to the limit of concurrent patches in my tree :-) Regards Thomas 2019-03-09 Thomas König PR fortran/71203 * decl.c (add_init_expr_to_sym): Add shape if init has none. Add assert that it has to be an EXPR_ARRAY in this case. 2019-03-09 Thomas König PR fortran/71203 * gfortran.dg/array_simplify_3.f90: New test case. Index: decl.c === --- decl.c (Revision 269443) +++ decl.c (Arbeitskopie) @@ -1983,8 +1983,14 @@ add_init_expr_to_sym (const char *name, gfc_expr * return false; } - /* Shape should be present, we get an initialization expression. */ - gcc_assert (init->shape); + /* The shape may be NULL for EXPR_ARRAY, set it. */ + if (init->shape == NULL) + { + gcc_assert (init->expr_type == EXPR_ARRAY); + init->shape = gfc_get_shape (1); + if (!gfc_array_size (init, &init->shape[0])) + gfc_internal_error ("gfc_array_size failed"); + } for (dim = 0; dim < sym->as->rank; ++dim) { ! { dg-do run } ! PR 71203 - this used to ICE program p integer :: i integer, parameter :: x(2) = 0 integer, parameter :: y(*) = [(x(i:i), i=1,2)] if (size(y,1) /= 2) stop 1 if (any(y /= 0)) stop 2 end
[patch, fortran, committed] Add a few more debug functions
Hello world, I have just committed this patch as obvious to make debugging a bit easier. No user impact, the code compiles :-) Regards Thomas 2019-03-31 Thomas Koenig * dump-parse-tree.c (debug): Add for symbol_attribute *, symbol_attribute and gfc_ref * arguments. Index: dump-parse-tree.c === --- dump-parse-tree.c (Revision 269895) +++ dump-parse-tree.c (Arbeitskopie) @@ -50,10 +50,26 @@ static void show_code (int, gfc_code *); static void show_symbol (gfc_symbol *); static void show_typespec (gfc_typespec *); +static void show_ref (gfc_ref *); +static void show_attr (symbol_attribute *, const char *); /* Allow dumping of an expression in the debugger. */ void gfc_debug_expr (gfc_expr *); +void debug (symbol_attribute *attr) +{ + FILE *tmp = dumpfile; + dumpfile = stderr; + show_attr (attr, NULL); + fputc ('\n', dumpfile); + dumpfile = tmp; +} + +void debug (symbol_attribute attr) +{ + debug (&attr); +} + void debug (gfc_expr *e) { FILE *tmp = dumpfile; @@ -79,6 +95,15 @@ debug (&ts); } +void debug (gfc_ref *p) +{ + FILE *tmp = dumpfile; + dumpfile = stderr; + show_ref (p); + fputc ('\n', dumpfile); + dumpfile = tmp; +} + void gfc_debug_expr (gfc_expr *e) {
Re: [patch] Fix PR 84487, large rodata increase in tonto and other programs
Hi, thanks a lot for the extensive discussion :-) How should we now proceed, first for gcc 9, snd then for backporting? Use Richard‘s patch with the corresponding Fortran FE change? Regards Thomas
Re: [PATCH] fortran: C++ support for generating C prototypes
Hi Janne, I do not think we need to add header guards. The headers, as we emit them, contain prototypes only, so repeated inclusions Should be harmless. So. the potential disadvantage would be a teeny bit of compilation time vs the chance of header macro collision and resulting wrong code. Filenames can be duplicates, and eventually the responsibility should be the user‘s. Finally, I want people to read the comment about this not being recommended 😉
[patch, fortran] Fix PR 57048
Hello world, let the regression hunt continue! the attached patch fixes a long-time regression where a c_funptr from a module could not be found. The solution is a bit of a hack, but so is our whole implementation of the C interop stuff. Regression-tested. OK for trunk? Regards Thomas 2019-01-28 Thomas Koenig PR fortran/57048 * interface.c (gfc_compare_types): If a derived type and an integer both have a derived type, and they are identical, this is a C binding type and compares equal. 2019-01-28 Thomas Koenig PR fortran/57048 * gfortran.dg/c_funptr_1.f90: New file. * gfortran.dg/c_funptr_1_mod.f90: New file. Index: interface.c === --- interface.c (Revision 268104) +++ interface.c (Arbeitskopie) @@ -692,6 +692,15 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec if (ts1->type == BT_VOID || ts2->type == BT_VOID) return true; + /* Special case for our C interop types. There should be a better + way of doing this... */ + + if (((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED) + || (ts1->type == BT_DERIVED && ts2->type == BT_INTEGER)) + && ts1->u.derived && ts2->u.derived + && ts1->u.derived == ts2->u.derived) +return true; + /* The _data component is not always present, therefore check for its presence before assuming, that its derived->attr is available. When the _data component is not present, then nevertheless the ! { dg-do preprocess } ! { dg-additional-options "-cpp" } ! PR 57048 - this used not to compile. Original test case by Angelo ! Graziosi. Only works if compiled c_funptr_1_mod.f90, hence the ! do-nothing directive above. module procs implicit none private public WndProc contains function WndProc() integer :: WndProc WndProc = 0 end function WndProc end module procs function WinMain() use, intrinsic :: iso_c_binding, only: C_INT,c_sizeof,c_funloc use win32_types use procs implicit none integer :: WinMain type(WNDCLASSEX_T) :: WndClass WndClass%cbSize = int(c_sizeof(Wndclass),C_INT) WndClass%lpfnWndProc = c_funloc(WndProc) WinMain = 0 end function WinMain program main end ! { dg-do run } ! { dg-additional-sources c_funptr_1.f90 } ! Additional module to go with c_funptr_1.f90 module win32_types use, intrinsic :: iso_c_binding, only: C_INT,C_FUNPTR implicit none private public WNDCLASSEX_T type, bind(C) :: WNDCLASSEX_T integer(C_INT) :: cbSize type(C_FUNPTR) :: lpfnWndProc end type WNDCLASSEX_T end module win32_types
Re: [patch, fortran] Fix PR 57048
Hi Steve, PR fortran/57048 * interface.c (gfc_compare_types): If a derived type and an integer both have a derived type, and they are identical, this is a C binding type and compares equal. I don't understand this sentence. How can an INTEGER have a derived type? ISO C handling is a bit of a mess. Of course. The original tree has symtree: 'C_funptr'|| symbol: 'c_funptr' type spec : (DERIVED c_funptr C_INTEROP ISO_C) attributes: (DERIVED BIND(C) USE-ASSOC(__iso_c_binding)) components: (c_address (INTEGER 8 C_INTEROP) () PRIVATE) and we somehow later lose the derived type stuff and use a naked integer. The problem appears to be that the module is written out when that conversion has not yet happened. So, the C_funptr (capitalized because it is a derived type) is written to the module file, and later compared to c_funptr upon module reading. Hilarity ensues, and we cannot use a C function pointer from a module, the topic of the PR. 2019-01-28 Thomas Koenig PR fortran/57048 * gfortran.dg/c_funptr_1.f90: New file. * gfortran.dg/c_funptr_1_mod.f90: New file. Index: interface.c === --- interface.c (Revision 268104) +++ interface.c (Arbeitskopie) @@ -692,6 +692,15 @@ gfc_compare_types (gfc_typespec *ts1, gfc_typespec if (ts1->type == BT_VOID || ts2->type == BT_VOID) return true; + /* Special case for our C interop types. There should be a better + way of doing this... */ + + if (((ts1->type == BT_INTEGER && ts2->type == BT_DERIVED) + || (ts1->type == BT_DERIVED && ts2->type == BT_INTEGER)) + && ts1->u.derived && ts2->u.derived + && ts1->u.derived == ts2->u.derived) +return true; If the above is a test for ISO C binding, shouldn't the test also check (ts1->is_c_interop || ts1->is_iso_c) and similar for ts2? Unfortunately, these are both set to zero. Yep, this is a rather messy fix to a messy situation. I think the C interop stuff could do with a thorough redesign, but that's not going to happen for gcc 9. So, I think this is about the best we can do - at least it gets the bug fixed. If you want me to put in a FIXME, I'll gladly do this. Regards Thomas
Re: [patch, fortran] Fix PR 57048
Hi Steve, Thanks for the patch, and OK to commit. Committed to trunk (r268372). Thanks! I will backport to the affected branches later in the week. Regards Thomas
Re: [PATCH] Do not dereference NULL pointer in resolve_ref (PR fortran/89185).
Hi Martin, > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Ready to be installed? > Thanks, > Martin Ok. Thanks for the very quick fix! Regards Thomas
Re: [PR fortran/89348, patch] Fortran Command Options documentation fixes
Mark, > Patch and change log attached to PR. Could you please submit this the normal way, with the ChangeLog in the text and the patch ad attachment? Regards, Thomas