--enable-maintainer-mode currently broken, needs --disable-werror to complete bootstrap
Hi, this is a heads-up that configuring with --enable-maintainer-mode currently breaks bootstrap; see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86450 for details. Running configure with --enable-maintainer-mode --disable-werror allows bootstrap to proceed until the underlying issue is fixed. Regards Thomas
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
Hi Janus, The cleaner approach would certainly be to avoid short-circuiting of impure functions altogether. If we can all agree that this is a good idea, This is a fine example of logical short-circuiting - the condition you mention is false, therefore the rest need not be evaluated :-)
Re: [patch, fortran] Asynchronous I/O, take 3
Hi everybody, I am currently testing the patch at https://gcc.gnu.org/ml/fortran/2018-07/msg8.html so far, so good! IMO the tests should go to gfortran.dg (they pass my tests). I put the asycn_io_*.f90 tests into libgomp.fortran because, under Linux, gfortran.dg does not link in pthreads, so the tests would not be executed in parallel, and some of them would fail. So, here is the final version. I would really like to get this into trunk, and out of the way, so Nicolas and I can focus on other things. So, OK? Regards Thomas 2018-07-15 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-15 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-15 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-15 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. * testsuite/libgomp.fortran/async_io_5.f90: New test. * testsuite/libgomp.fortran/async_io_6.f90: New test. * testsuite/libgomp.fortran/async_io_7.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 specification and initialization expressions, including the support for intrinsics with real and complex arguments. -@item Support for the asynchronous input/output syntax; however, the -data transfer is currently always synchronously performed. +@
Re: [patch, fortran] Asynchronous I/O, take 3
Hi Rainer, I've now regtested the patch on i386-pc-solaris2.11 and sparc-sun-solaris2.11: no regressions and the new tests all PASS. Thanks, that is good news! However, I still don't understand why you insist on the hack with putting the async_io_*.f90 tests into the libgomp testsuite. Why not just make the pthread requirement explicit with { dg-require-effective-target pthread } { dg-additional-options "-pthread" } and put them in gfortran.dg where they belong? Because this does not appear to work with Linux. I, like most gfortran developers, work on Linux, and I would like to catch any failure during regression-testing on my own system, if possible. We have had this discussion with Jakub, and he advised us to put all the stuff requiring pthreads into libgomp. It is debatable if this is a good thing, or if we should at least make one round of tests with -pthread enabled. However, this is something for the future, and requires knowledge of dejagnu that I don't currently have :-) Regards Thomas
Re: [patch, fortran] Asynchronous I/O, take 3
Am 15.07.2018 um 19:47 schrieb Rainer Orth: Because this does not appear to work with Linux. I, like most gfortran developers, work on Linux, and I would like to catch any failure during regression-testing on my own system, if possible. huh, what doesn't work? I've just finished an x86_64-pc-linux-gnu bootstrap with your patch included, added the above to the async_io_?.f90 tests, linked them to gfortran.dg and ran the tests there (both 32 and 64-bit multilibs), all PASSed and I verified that they were linked with -lpthread. We have had this discussion with Jakub, and he advised us to put all the stuff requiring pthreads into libgomp. Do you have a pointer to that previous discussion? https://gcc.gnu.org/ml/fortran/2018-04/msg00048.html is what I based my recollection on. Regards Thomas
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
Hi Janus, I tested it on a fairly large code base and found no further false positives. Also it still regtests cleanly. Ok for trunk? while I still disagree with this on principle, I will not stand in the way. However, one point: I think that the warning should be under a separate warning, which should then be enabled by -Wextra. -Waggressive-function-elimination, could be reused for this, or something else Regards Thomas
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
Am 16.07.2018 um 10:06 schrieb Janus Weil: However, one point: I think that the warning should be under a separate warning, which should then be enabled by -Wextra. -Waggressive-function-elimination, could be reused for this, or something else I don't actually see such a flag in the manual. Ah, sorry, I misremembered the option, it is actually -Wfunction-elimination. What I would suggest is to enable -Wfunction-eliminiation with -Wextra and also use that for your new warning. (I would also suggest to enable -faggressive-function-elimination at least for -Ofast, but that is another matter). Regards Thomas
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
Hi Kyrill, The current implementation expands to: mvar = a1; if (a2 .op. mvar || isnan (mvar)) mvar = a2; if (a3 .op. mvar || isnan (mvar)) mvar = a3; ... return mvar; That is, if one of the operands is a NaN it will return the other argument. If both (all) are NaNs, it will return NaN. This is the same as the semantics of fmin/max as far as I can tell. I've looked at the F2008 standard, and, interestingly enough, the requirement on MIN and MAX do not mention NaNs at all. 13.7.106 has, for MAX, Result Value. The value of the result is that of the largest argument. plus some stuff about character variables (not relevant here). Similar for MIN. Also, the section on IEEE_ARITHMETIC (14.9) does not mention comparisons; also, "Complete conformance with IEC 60559:1989 is not required", what is required is the correct support for +,-, and *, plus support for / if IEEE_SUPPORT_DIVIDE is covered. So, the Fortran standard does not impose many requirements. I do think that a patch such as yours should not change the current behavior unless we know what it does and do think it is a good idea. Hmm... Having said that, I think we pretty much cover all the corner cases in nan_1.f90, so if that test passes without regression, then that aspect should be fine. Question: You have found an advantage on Aarm64. Do you have access to other architectures so see if there is also a speed advantage, or maybe a disadvantage? Regards Thomas
Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
Am 17.07.2018 um 19:19 schrieb Janus Weil: 2018-07-17 17:18 GMT+02:00 Fritz Reese : 2018-07-17 9:52 GMT+02:00 Janus Weil : In other words: Does it make sense to tone down -Wfunction-elimination, by only warning about impure functions? Here is an update of the patch which does that. Regtesting now ... Would not this break the testcase function_optimize_5.f90 : My regtest says so as well ;) The docs for -Wfunction-elimination would read: Warn if any calls to functions are eliminated by the optimizations enabled by the @option{-ffrontend-optimize} option. This option is implied by @option{-Wextra}. However, with your patch, it should probably read something like "warn if any calls to impure functions are eliminated..." Possibly with an explicit remark indicating that pure functions are not warned. Yes. However, the test case above seems to indicate that the function-elimination optimization is not applied to impure functions anyway (which is good IMHO). If you specify -faggressive-function-elimination, it is also done for impure (and non implicitly-pure) functions. Problem is that, in all probability, nobody uses this option at the moment. It that is true, then my modifications practically disable the old -Wfunction-elimination warnings completely :/ I do not think it would be a problem not to warn for removing calls to pure or implicitly pure fuctions. The test cases can easily be modified not to emit this warning, as you did. As the author of the original test cases, I may be able to say so with a certain amount of credibility. The actual elimination is checked for by counting the function names in the *.original dump file, which is done.
Re: [PATCH][Fortran] Use MIN/MAX_EXPR for intrinsics or __builtin_fmin/max when appropriate
Hi Kyrill, Because the expansion now emits straightline code rather than conditionals and branches it should be easier to optimise in general, so I'd expect this to be an improvement overall. That said, I have benchmarked it on SPEC2017 on aarch64. If you have any benchmarks of interest to you you (or somebody else) can run on a target that you care about I would be very grateful for any results. Well, most people currently use x86_64 for scientific computing, so I would be concerned most about this architecture. As for the test case, min / max performance clearly has an effect on 521.wrf, so this would be ideal. If you could run 521.wrf on x86_64, and find that it does not regress measureably (or even shows an improvement), the patch is OK. I'd be interested in the timings you get. Regards Thomas
Re: Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)
Hi Ulrich, The problem is that io/asynch.h unconditionally uses a couple of features that are not provided by gthr-simplex, in particular __gthread_cond_t and __gthread_equal / __gthread_self According to the documentation in gthr.h, the former is only available if __GTHREAD_HAS_COND is defined, and the latter are only available if __GTHREADS_CXX0X is defined. Neither is true for gthr-simple.h. Thanks for the analysis, and the pointer to the macros. Because the functionality depends on these features, it is best to remove them if it is not present. So, here is a patch which does just that. This was reg-tested on Linux, which showed that the functionality is still there. I tried bootstrapping on AIX on gcc119, but this failed due to an unrelated issue (problem with compiling the inline libraries). Would it be possible to check if this restores bootstrap in the next 10 hours or so? If so, I would like to commit this. Otherwise, Nicolas and I will not be able to fix this for a week or so, and it would be best to revert the async I/O patch :-( Regards Thomas 2018-07-25 Thomas Koenig * io/async.h: Test for feature macros for __gthread_cond_t and __gthread_equal. Define ASYNC_IO if both are present. (SIGNAL): Define as no-op if ASYNC_IO is not defined. (WAIT_SIGNAL_MUTEX): Likewise. (REVOLE_SIGNAL): Likewise. (transfer_args): Define as useless struct if ASYNC_IO is not defined. (adv_cond): Likewise. (async_unit): Likewise. * io/async.c (init_async_unit): If ASYNC_IO is not defined, define alternate function which does nothing. (enqueue_transfer): Likewise. (enqueue_done_id): Likewise. (enqueue_done): Likewise. (enqueue_close): Likewise. (enqueue_data_transfer_init): Likewise. (collect_async_errors): Likewise. (async_wait_id): Likewise. (async_wait): Likewise. (async_close): Likewise. Index: io/async.h === --- io/async.h (revision 262978) +++ io/async.h (working copy) @@ -25,6 +25,16 @@ #ifndef ASYNC_H #define ASYNC_H +/* Async I/O will not work on targets which do not support + __gthread_cond_t and __gthread_equal / __gthread_self. Check + this. */ + +#if defined(__GTHREAD_HAS_COND) && defined(__GTHREADS_CXX0X) +#define ASYNC_IO 1 +#else +#undef ASYNC_IO +#endif + /* Defining DEBUG_ASYNC will enable somewhat verbose debugging output for async I/O. */ @@ -217,6 +227,8 @@ #define INTERN_UNLOCK(mutex) T_ERROR (__gthread_mutex_unlock, mutex); +#if ASYNC_IO + #define SIGNAL(advcond) do{ \ INTERN_LOCK (&(advcond)->lock); \ (advcond)->pending = 1; \ @@ -257,6 +269,15 @@ INTERN_UNLOCK (&(advcond)->lock); \ } while (0) +#else + +#define SIGNAL(advcond) do{} while(0) +#define WAIT_SIGNAL_MUTEX(advcond, condition, mutex) do{} while(0) +#define REVOKE_SIGNAL(advcond) do{} while(0) + +#endif + +#if ASYNC_IO DEBUG_LINE (extern __thread const char *aio_prefix); DEBUG_LINE (typedef struct aio_lock_debug{ @@ -274,6 +295,7 @@ DEBUG_LINE (extern __gthread_mutex_t debug_queue_l error reporting. */ extern __thread gfc_unit *thread_unit; +#endif enum aio_do { AIO_INVALID = 0, @@ -285,6 +307,8 @@ enum aio_do { AIO_CLOSE }; +#if ASYNC_IO + typedef union transfer_args { struct @@ -342,6 +366,23 @@ typedef struct async_unit } async_unit; +#else +typedef union transfer_args +{ + int x; +}; + +struct adv_cond +{ + int x; +}; + +typedef struct async_unit +{ + int x; +}; +#endif + void init_async_unit (gfc_unit *); internal_proto (init_async_unit); Index: io/async.c === --- io/async.c (revision 262978) +++ io/async.c (working copy) @@ -36,6 +36,7 @@ #include #include "async.h" +#if ASYNC_IO DEBUG_LINE (__thread const char *aio_prefix = MPREFIX); @@ -481,3 +482,88 @@ async_close (async_unit *au) T_ERROR (__gthread_join, au->thread, NULL); free_async_unit (au); } + +#else + +/* Do-nothing function, which will not be called. */ + +void +init_async_unit (gfc_unit *u) +{ + u->au = NULL; + return; +} + +/* Do-nothing function, which will not be called. */ + +void +enqueue_transfer (async_unit *au, transfer_args *arg, enum aio_do type) +{ + return; +} + +/* Do-nothing function, which will not be called. */ + +int +enqueue_done_id (async_unit *au, enum aio_do type) +{ + return 0; +} + +/* Do-nothing function, which will not be called. */ + +void +enqueue_done (async_unit *au, enum aio_do type) +{ + return; +} + +/* Do-nothing function, which will not be called. */ + +void +enqueue_close (async_unit *au) +{ + return; +} + +/* Do-nothing function, which will not be called. */ + +void +enqueue_data_transfer_init (async_unit *au, st_parameter_dt *dt,
Re: Build fail on gthr-simple.h targets (Re: AsyncI/O patch committed)
Am 26.07.2018 um 22:54 schrieb Thomas Koenig: Hi Ulrich, The problem is that io/asynch.h unconditionally uses a couple of features that are not provided by gthr-simplex, in particular __gthread_cond_t and __gthread_equal / __gthread_self According to the documentation in gthr.h, the former is only available if __GTHREAD_HAS_COND is defined, and the latter are only available if __GTHREADS_CXX0X is defined. Neither is true for gthr-simple.h. Thanks for the analysis, and the pointer to the macros. Because the functionality depends on these features, it is best to remove them if it is not present. So, here is a patch which does just that. This was reg-tested on Linux, which showed that the functionality is still there. I tried bootstrapping on AIX on gcc119, but this failed due to an unrelated issue (problem with compiling the inline libraries). OK, this does not work. We have found a method of checking on Linux, and this does not work. We have also found a way of working in the next couple of days, so expect an update in one or two days. If that is too much time, feel free to revert the async patch in the meantime. Regards Thomas
Re: [PATCH v4] libgfortran: Replace mutex with rwlock
Hi Lipeng, Sure, as your comments, in the patch V6, I added 3 test cases with OpenMP to test different cases in concurrency respectively: 1. find and create unit very frequently to stress read lock and write lock. 2. only access the unit which exist in cache to stress read lock. 3. access the same unit in concurrency. For the third test case, it also help to find a bug: When unit can't be found in cache nor unit list in read phase, then threads will try to acquire write lock to insert the same unit, this will cause duplicate key error. To fix this bug, I get the unit from unit list once again before insert in write lock. More details you can refer the patch v6. Could you help to review this update? I really appreciate your assistance. Could you help to review this update? Any concern will be appreciated. Fortran parts are OK (I think I wrote that already), we need somebody for the non-Fortran parts. Jakub, could you maybe take a look? Best regards Thomas
Re: Argument-mismatch fallout
However, passing a scalar instead of an array/array element worked/works with (nearly?) all compilers. Hence, passing a scalar is seemingly common pattern. Thus, I wonder whether we should do something about this. Maybe we could mention -fallow-argument-mismatch in the error message. (Really, the changes needed are quite trivial, and the code is in fact invalid. I actually wrote the patch for LAPACK which removed all the fallout from their TESTING routines; it didn't take long). Regards Thomas
Re: [Patch][Fortran] PR 92208 don't use function-result dummy variable as actual argument
Hi Tobias, OK for the trunk and GCC 9? As far as I can see, this looks good. So, OK for trunk. If it later turns out that there are problems caused by this, I suspect we will hear about them soon enough :-) Thanks for taking this on! Regards Thomas
[patch, fortran] Fix PR 92113
Hello world, the attached patch fixes an 8/9/10 regression where, to fix PR 84487 by not putting the initializers and vtabs into the read-only section (for reasons of size, which could grow enormously) led to a regression on POWER9 and other non-x86 architectures, where the initializer was sometimes optimized away, depending on optimization levels. This was a strange beast to hunt down. This only showed up on the testresults for gcc 8, so I tried to find out what commit had fixed this on trunk, in order to backport. However, bisecting this I found that the test case actually segfaults all the way up to current trunk when run by hand. By running the testsuite, I didn't see it. This is strange, and raises some issues about the testsuite and the possibility of a latent issue, but I lack the knowledge to hunt this down. In the meantime, here is this patch, which puts the vtabs and the initializers where the user actually specified something into the read-only section again. Test case: Well, theoretically it is already there, so it makes little sense to add a new one. Regression-tested on powerpc64le-unknown-linux-gnu, also verified by hand that pr51434.f90 now passes with -O2 there. OK for trunk/9/8? Regards Thomas 2019-11-02 Thomas Koenig PR fortran/92133 * trans-decl.c (gfc_get_symbol_decl): If __def_init actually contains a value, put it into the read-only section. Index: trans-decl.c === --- trans-decl.c (Revision 277486) +++ trans-decl.c (Arbeitskopie) @@ -1911,14 +1911,19 @@ gfc_get_symbol_decl (gfc_symbol * sym) if (sym->attr.associate_var) GFC_DECL_ASSOCIATE_VAR_P (decl) = 1; - /* We no longer mark __def_init as read-only so it does not take up - space in the read-only section and dan go into the BSS instead, - see PR 84487. Marking this as artificial means that OpenMP will - treat this as predetermined shared. */ - if (sym->attr.vtab - || (sym->name[0] == '_' && gfc_str_startswith (sym->name, "__def_init"))) -DECL_ARTIFICIAL (decl) = 1; + /* We only mark __def_init as read-only if it actually has an + initializer so it does not needlessly take up space in the + read-only section and can go into the BSS instead, see PR 84487. + Marking this as artificial means that OpenMP will treat this as + predetermined shared. */ + if (sym->attr.vtab || gfc_str_startswith (sym->name, "__def_init")) +{ + DECL_ARTIFICIAL (decl) = 1; + if (sym->attr.vtab || sym->value) + TREE_READONLY (decl) = 1; +} + return decl; }
Re: [patch, fortran] Fix PR 92113
Hi Steve, OK for trunk/9/8? OK for all three. Thanks, committed to trunk as r277760. I'll be AFK for a few days, so I will have to wait before committing this to gcc-9. Given the convoluted history of this bug, this might not be a bad thing. > It is, as you have indicated, troublesome that a segfaulting > testcase isn't caught by the testsuite. It certainly is, but I have no solution for this at the moment. Thanks for the review! Regards Thomas
[patch, fortran, committed] Commit symbol for external BLAS routine when translating MATMUL to *GEMM.
Hi, I just committed the patch below as obvious to fix a 9/10 regression when directly calling BLAS routines for matmul. Will backport to gcc-9 soon. Regards Thomas Commit symbol for external BLAS routine when translating MATMUL to *GEMM. 2019-11-09 Thomas Koenig PR fortran/92321 * frontend-passes.c (call_external_blas): Commit symbol for external BLAS routine. 2019-11-09 Thomas Koenig PR fortran/92321 * gfortran.dg/matmul_blas_2.f90: New test. ! { dg-do compile } ! { dg-options "-O3 -fdump-tree-original -fexternal-blas" } ! PR fortran/92321 - this used to cause an ICE. Original test case ! by Nathan Wukie. module mod_badmatmul implicit none contains subroutine test(c) real, intent(inout) :: c(3,3) real :: a(3,3), b(3,3) c = matmul(a, b) end subroutine test end module mod_badmatmul program main use mod_badmatmul, only: test implicit none real :: a(3,3) call test(a) end program main Index: frontend-passes.c === --- frontend-passes.c (Revision 277999) +++ frontend-passes.c (Arbeitskopie) @@ -4635,6 +4635,7 @@ call_external_blas (gfc_code **c, int *walk_subtre call->symtree->n.sym->attr.procedure = 1; call->symtree->n.sym->attr.flavor = FL_PROCEDURE; call->resolved_sym = call->symtree->n.sym; + gfc_commit_symbol (call->resolved_sym); /* Argument TRANSA. */ next = gfc_get_actual_arglist ();
Re: [PATCH] Bump minimum MPFR version to 3.1.0
Hi Janne, Bump the minimum MPFR version to 3.1.0, released 2011-10-03. With this requirement one can still build GCC with the operating system provided MPFR on old but still supported operating systems like SLES 12 (MPFR 3.1.2) or RHEL/CentOS 7.x (MPFR 3.1.1). OK for trunk. Can you also make a note in https://gcc.gnu.org/gcc-10/changes.html ? Regards Thomas
Re: [patch, fortran] Load scalar intent-in variables at the beginning of procedures
Am 11.11.19 um 22:55 schrieb Thomas König: Regression-tested. OK for trunk? Of course, better with a ChangeLog entry. 2019-11-11 Thomas Koenig PR fortran/67202 * dump-parse-tree.c (debug): Add for gfc_namespace. (show_code_node): Add INIT_ on dumping EXEC_INIT_ASSIGN. * frontent-passes.c (replace_intent_in): Add prototype. New function. (optimize_namespace): Call it. (sym_replacement): New struct. (replace_symbol_in_expr): New function. 2019-11-11 Thomas Koenig PR fortran/67202 * gfortran.dg/intent_optimize_3.f90: New test.
Re: [patch, fortran] Fix PR 65428, ICE on nested empty array constructors
Hi Tobias, I am not completely happy about the introduction of yet another two global variables, but I also do not see an easy way out. Hence: OK. Actually, I wasn't too happy myself, but, like you, I didn't find anything better. I was playing around with the following test case – you might consider to add them as well. (I would exclude the error item, however.) I have also added this as a test case. Committed as r280063 (without the spaces at the end of the lines that you pointed out in a separate mail). Thanks for the review! Regards Thomas
[patch, fortran] Fix PR 44960, accepts invalid reference on function
Hello world, here is my first patch made from the git world. It certainly took enough time to work out how to to this, but I think I have it figured out now... Anyway, the fix is rather straightforward. We cannot have a reference on a function. If this slipped through on parsing, let's issue the error during resolution. Regression-tested. OK for trunk? Regards Thomas 2020-01-16 Thomas König PR fortran/44960 * resolve.c (resolve_function): Issue error when a function call contains a reference. 2020-01-16 Thomas König PR fortran/44960 * gfortran.dg/function_reference_1.f90: New test. diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index 6f2a4c4d65a..1525c00ea4c 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -3129,6 +3129,13 @@ resolve_function (gfc_expr *expr) || sym->intmod_sym_id == GFC_ISYM_CAF_SEND)) return true; + if (expr->ref) +{ + gfc_error ("Function call can not contain a reference at %L", + &expr->where); + return false; +} + if (sym && sym->attr.intrinsic && !gfc_resolve_intrinsic (sym, &expr->where)) return false; diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90 new file mode 100644 index 000..78c92a6f20d --- /dev/null +++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR 44960 - this was erroneusly accepted. +! Original test case by Daniel Franke. + +type t + integer :: a +end type t +type(t) :: foo +print *, foo(1)%a ! { dg-error "Function call can not contain a reference" } +end + ! { dg-do compile } ! PR 44960 - this was erroneusly accepted. ! Original test case by Daniel Franke. type t integer :: a end type t type(t) :: foo print *, foo(1)%a ! { dg-error "Function call can not contain a reference" } end
Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
Am 17.01.20 um 15:42 schrieb Steve Kargl: Gfortran probably should not try to guess what the user thought s/he wanted. The generic "Syntax error" would seem to apply here. To me, foo(1)%a looks much more like an array reference rather than a function reference. OK, so here's a patch which does just that. The error message low looks like function_reference_1.f90:9:8: 9 | print *, foo(1)%a ! { dg-error "Syntax error" } |1 Error: Syntax error in expression at (1) The location information is a bit off, but in the absence of location information for the reference (which we do not collect), I think this is the best I can do. So, OK for trunk (with the old ChangeLog)? Regards Thomas diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index 6f2a4c4d65a..a846677b770 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -3129,6 +3129,12 @@ resolve_function (gfc_expr *expr) || sym->intmod_sym_id == GFC_ISYM_CAF_SEND)) return true; + if (expr->ref) +{ + gfc_error ("Syntax error in expression at %L", &expr->where); + return false; +} + if (sym && sym->attr.intrinsic && !gfc_resolve_intrinsic (sym, &expr->where)) return false; diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90 new file mode 100644 index 000..1b7f4809c5c --- /dev/null +++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR 44960 - this was erroneusly accepted. +! Original test case by Daniel Franke. + +type t + integer :: a +end type t +type(t) :: foo +print *, foo(1)%a ! { dg-error "Syntax error" } +end + ! { dg-do compile } ! PR 44960 - this was erroneusly accepted. ! Original test case by Daniel Franke. type t integer :: a end type t type(t) :: foo print *, foo(1)%a ! { dg-error "Syntax error" } end
Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
Am 18.01.20 um 12:44 schrieb Tobias Burnus: function_reference_1.f90:9:8: 9 | print *, foo(1)%a ! { dg-error "Syntax error" } | 1 Error: Syntax error in expression at (1) The error message is not helpful at all. Well, yes. It is no better than what we currently have for the slightly different test case type t integer :: a end type t type(t) :: foo external foo print *, foo(1)%a end which is a.f90:6:16: 6 | print *, foo(1)%a |1 Error: Syntax error in PRINT statement at (1) (but at least that points to the right place). I can see if I can also replace this with something more useful (have to find the place where this is issued first, though). Regards Thomas
Re: [patch, fortran] Fix PR 44960, accepts invalid reference on function
Hello world, I found an ommission in primary.c which prevented issuing a more specific error instead of "syntax error" for the case when a function was declared in an EXTERNAL statement, and I have now gone for the "Unexpected junk after foo" variant. Regression-tested. OK for trunk? Regards Thomas 2020-01-18 Thomas König PR fortran/44960 * primary.c (gfc_match_rvalue): Break after setting MATCH_ERROR. * resolve.c (resolve_function): Issue error when a function call contains a reference. 2020-01-18 Thomas König PR fortran/44960 * gfortran.dg/function_reference_1.f90: New test. diff --git a/gcc/fortran/primary.c b/gcc/fortran/primary.c index 07b8ac08ba2..bd50827bb15 100644 --- a/gcc/fortran/primary.c +++ b/gcc/fortran/primary.c @@ -3661,6 +3661,7 @@ gfc_match_rvalue (gfc_expr **result) gfc_error ("The leftmost part-ref in a data-ref cannot be a " "function reference at %C"); m = MATCH_ERROR; + break; } m = MATCH_YES; diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c index 6f2a4c4d65a..697afadb378 100644 --- a/gcc/fortran/resolve.c +++ b/gcc/fortran/resolve.c @@ -3129,6 +3129,13 @@ resolve_function (gfc_expr *expr) || sym->intmod_sym_id == GFC_ISYM_CAF_SEND)) return true; + if (expr->ref) +{ + gfc_error ("Unexpected junk after %qs at %L", expr->symtree->n.sym->name, + &expr->where); + return false; +} + if (sym && sym->attr.intrinsic && !gfc_resolve_intrinsic (sym, &expr->where)) return false; diff --git a/gcc/testsuite/gfortran.dg/function_reference_1.f90 b/gcc/testsuite/gfortran.dg/function_reference_1.f90 new file mode 100644 index 000..be634c9dd4b --- /dev/null +++ b/gcc/testsuite/gfortran.dg/function_reference_1.f90 @@ -0,0 +1,11 @@ +! { dg-do compile } +! PR 44960 - this was erroneusly accepted. +! Original test case by Daniel Franke. + +type t + integer :: a +end type t +type(t) :: foo +print *, foo(1)%a ! { dg-error "Unexpected junk" } +end + diff --git a/gcc/testsuite/gfortran.dg/function_reference_2.f90 b/gcc/testsuite/gfortran.dg/function_reference_2.f90 new file mode 100644 index 000..375c58bb6d2 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/function_reference_2.f90 @@ -0,0 +1,10 @@ +! { dg-do compile } +! PR 44960 - improve the error message +program main + type t + integer :: a +end type t +type(t) :: foo +external foo +i = foo(1)%1 ! { dg-error "leftmost part-ref in a data-ref cannot be a function reference" } +end ! { dg-do compile } ! PR 44960 - improve the error message program main type t integer :: a end type t type(t) :: foo external foo i = foo(1)%1 ! { dg-error "leftmost part-ref in a data-ref cannot be a function reference" } end ! { dg-do compile } ! PR 44960 - this was erroneusly accepted. ! Original test case by Daniel Franke. type t integer :: a end type t type(t) :: foo print *, foo(1)%a ! { dg-error "Unexpected junk" } end
Re: [PATCH] libgfortran: Fix and simplify IO locking [PR92836]
Hi Janne, Simplify IO locking in libgfortran. The new IO implementation avoids accessing units without locks, as seen in PR 92836. It also avoids lock inversion (except for a corner case wrt namelist query when reading from stdin and outputting to stdout), making it easier to verify correctness with tools like valgrind or threadsanitizer. It is also simplified as the waiting and closed variables are not needed anymore, making it easier to understand and analyze. Regtested on x86_64-pc-linux-gnu, Ok for master? I'll look into it, this might take a bit of time. What are you planning to use as a test case? You can put multighreading programs into libgomp/testsuite/libgomp.fortran where they will be executed. Regards Thomas
Re: [Patch] [OpenMP] Add missing parameters to omp_lib documentation (PR fortran/93541)
Hi Tobias, OK? – And for which branches? OK for all branches (assuming you checked with "make pdf" and "make dvi"). Thanks for doing this - our documentation is not always quite up to date... Regards Thomas
[patch, fortran] Introduce -finline-pack
Hello world, the attached patch introduces a new option, -finline-pack. Since the fix for PR88821, we now do inline packing of arguments (if required) via the scalarizer, instead of using _gfortran_internal_[un]pack when optimizing, but not when optimizing for size. This introduces (really) large performance gains for some test cases because now the middle end can see through the packing. On the other hand, for test cases which do a _lot_ of this, compile time and code size can increase by quite a bit. So, this patch introduces an option to control that behavior, so that people can turn it off on a by-file basis if they don't want it. OK for trunk? Regards Thomas Introduce -finline-pack. 2019-12-07 Thomas Koenig PR middle-end/91512 PR fortran/92738 * invoke.texi: Document -finline-pack. * lang.opt: Add -finline-pack. * options.c (gfc_post_options): Handle -finline-pack. * trans-array.c (gfc_conv_array_parameter): Use flag_inline_pack instead of checking for optimize and optimize_size. 2019-12-07 Thomas Koenig PR middle-end/91512 PR fortran/92738 * gfortran.dg/inline_pack_25.f90: New test. Index: invoke.texi === --- invoke.texi (Revision 279064) +++ invoke.texi (Arbeitskopie) @@ -192,8 +192,9 @@ and warnings}. -ffrontend-loop-interchange -ffrontend-optimize @gol -finit-character=@var{n} -finit-integer=@var{n} -finit-local-zero @gol -finit-derived -finit-logical=@var{} @gol --finit-real=@var{} @gol --finline-matmul-limit=@var{n} -fmax-array-constructor=@var{n} @gol +-finit-real=@var{} +-finline-matmul-limit=@var{n} @gol +-finline-pack -fmax-array-constructor=@var{n} @gol -fmax-stack-var-size=@var{n} -fno-align-commons -fno-automatic @gol -fno-protect-parens -fno-underscoring -fsecond-underscore @gol -fpack-derived -frealloc-lhs -frecursive -frepack-arrays @gol @@ -1779,6 +1780,34 @@ compiled with the @option{-fshort-enums} option. GNU Fortran choose the smallest @code{INTEGER} kind a given enumerator set will fit in, and give all its enumerators this kind. +@item -finline-pack +@opindex @code{finline-pack} +When passing an assumed-shape argument of a procedure as actual +argument to an assumed-size or explicit size or as argument to a +procedure that does not have an explicit interface, the argument may +have to be packed, that is put into contiguous memory. An example is +the call to @code{foo} in +@smallexample + subroutine foo(a) + real, dimension(*) :: a + end subroutine foo + subroutine bar(b) + real, dimension(:) :: b + call foo(b) + end subroutine bar +@end smallexample + +When @option{-finline-pack} is in effect, this packing will be +performed by inline code. This allows for more optimization while +increasing code size. + +@option{-finlie-pack} is implied by any of the @option{-O} options +except when optimizing for size via @option{-Os}. If the code +contains a very large number of argument that have to be packed, code +size and also compilation time may become excessive. If that is the +case, it may be better to disable this option. Instances of packing +can be found by using by using @option{-Warray-temporaries}. + @item -fexternal-blas @opindex @code{fexternal-blas} This option will make @command{gfortran} generate calls to BLAS functions Index: lang.opt === --- lang.opt (Revision 279064) +++ lang.opt (Arbeitskopie) @@ -647,6 +647,10 @@ Enum(gfc_init_local_real) String(inf) Value(GFC_IN EnumValue Enum(gfc_init_local_real) String(-inf) Value(GFC_INIT_REAL_NEG_INF) +finline-pack +Fortran Var(flag_inline_pack) Init(-1) +-finline-pack Perform argument packing inline + finline-matmul-limit= Fortran RejectNegative Joined UInteger Var(flag_inline_matmul_limit) Init(-1) -finline-matmul-limit= Specify the size of the largest matrix for which matmul will be inlined. Index: options.c === --- options.c (Revision 279064) +++ options.c (Arbeitskopie) @@ -467,6 +467,11 @@ gfc_post_options (const char **pfilename) if (flag_frontend_loop_interchange == -1) flag_frontend_loop_interchange = optimize; + /* Do inline packing by default if optimizing, but not if + optimizing for size. */ + if (flag_inline_pack == -1) +flag_inline_pack = optimize && !optimize_size; + if (flag_max_array_constructor < 65535) flag_max_array_constructor = 65535; Index: trans-array.c === --- trans-array.c (Revision 279064) +++ trans-array.c (Arbeitskopie) @@ -8139,7 +8139,7 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * making the packing and unpacking operation visible to the optimizers. */ - if (g77 && optimize && !optimize_size && expr-&g
[patch, fortran] Fix PR 92755
Hello world, In the fix for PR 91783, I had not considered the case that the _data ref could also be somewhere inside. The solution was obvious and simple: Move the check into the loop over the refs. Committed as r279086 after regression-testing. Regards Thomas 2019-12-08 Thomas Koenig PR fortran/92755 * dependency.c (gfc_dep_resolver): Move skipping of _data ref into the loop. 2019-12-08 Thomas Koenig PR fortran/92755 * gfortran.dg/dependency_57.f90: New test. Index: dependency.c === --- dependency.c (Revision 279064) +++ dependency.c (Arbeitskopie) @@ -2098,18 +2098,6 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf gfc_dependency this_dep; bool same_component = false; - /* The refs might come in mixed, one with a _data component and one - without. Look at their next reference in order to avoid an - ICE. */ - - if (lref && lref->type == REF_COMPONENT && lref->u.c.component - && strcmp (lref->u.c.component->name, "_data") == 0) -lref = lref->next; - - if (rref && rref->type == REF_COMPONENT && rref->u.c.component - && strcmp (rref->u.c.component->name, "_data") == 0) -rref = rref->next; - this_dep = GFC_DEP_ERROR; fin_dep = GFC_DEP_ERROR; /* Dependencies due to pointers should already have been identified. @@ -2117,6 +2105,18 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf while (lref && rref) { + /* The refs might come in mixed, one with a _data component and one + without. Look at their next reference in order to avoid an + ICE. */ + + if (lref && lref->type == REF_COMPONENT && lref->u.c.component + && strcmp (lref->u.c.component->name, "_data") == 0) + lref = lref->next; + + if (rref && rref->type == REF_COMPONENT && rref->u.c.component + && strcmp (rref->u.c.component->name, "_data") == 0) + rref = rref->next; + /* We're resolving from the same base symbol, so both refs should be the same type. We traverse the reference chain until we find ranges that are not equal. */ ! { dg-do compile } ! PR 92755 - this used to cause an ICE. ! Original test case by Gerhard Steinmetz program p type t end type type t2 class(t), allocatable :: a(:) end type type(t2) :: z z%a = [z%a] end
[patch, fortran] Add NULL check before checking interfaces
Hello world, the attached patch fixes an ICE where a NULL check was missing. Committed as obvious and simple after regression-testing as r279087. Since this is an ICE after an error has already been emitted, I don't see any particular need to backport. Regards Thomas 2018-12-08 Thomas Koenig PR fortran/92764 * interface.c (gfc_procedure_use): Check for existence of derived component before using (twice). 2018-12-08 Thomas Koenig PR fortran/92764 * gfortran.dg/interface_44.f90: New test. Index: interface.c === --- interface.c (Revision 279064) +++ interface.c (Arbeitskopie) @@ -3888,6 +3888,7 @@ gfc_procedure_use (gfc_symbol *sym, gfc_actual_arg /* F2008, C1303 and C1304. */ if (a->expr && (a->expr->ts.type == BT_DERIVED || a->expr->ts.type == BT_CLASS) + && a->expr->ts.u.derived && ((a->expr->ts.u.derived->from_intmod == INTMOD_ISO_FORTRAN_ENV && a->expr->ts.u.derived->intmod_sym_id == ISOFORTRAN_LOCK_TYPE) || gfc_expr_attr (a->expr).lock_comp)) @@ -3901,6 +3902,7 @@ gfc_procedure_use (gfc_symbol *sym, gfc_actual_arg if (a->expr && (a->expr->ts.type == BT_DERIVED || a->expr->ts.type == BT_CLASS) + && a->expr->ts.u.derived && ((a->expr->ts.u.derived->from_intmod == INTMOD_ISO_FORTRAN_ENV && a->expr->ts.u.derived->intmod_sym_id == ISOFORTRAN_EVENT_TYPE) ! { dg-do compile } ! PR 92964 - this used to ICE. ! Original test case by Arseny Solokha type(e6) function dn() ! { dg-error "The type for function" } call sub(dn) end function dn
Re: [patch, fortran] Introduce -finline-pack
Hi Richard, Just as a suggestion, maybe we'd want to extend this to other intrinsics in future so a -fno-inline-intrinsic=pack[,...] is more future proof? (I'd inline all intrinsics by default thus only provide the negative form). You can avoid the extra option parsing complexity by only literally adding -fno-inline-intrinsic=pack for now. I agree that such an option would make sense, I think this is something we should consider for gcc 11. In this instance, your reply shows that the option is poorly named, because it is actually not about the PACK intrinsic, but the internal packing that happens for arguments. Maybe -finline-repack would be a better name? -finline-internal-pack? Regards Thomas
[patch, fortran, committed] Fix PR 92863, 'ICE in gfc_typename
Hello world, I have just committed the attached patch as obvious and simple as r279180. The ICE for the test case occurred because a previous error had left the derived field of the typespec NULL. Just returning "invalid type" or "invalid class" in such a case is enough to cure the ICE. Regards Thomas 2019-12-10 Thomas Koenig PR fortran/92863 * misc.c (gfc_typename): If derived component is NULL for derived or class, return "invalid type" or "invalid class", respectively. 2019-12-10 Thomas Koenig PR fortran/92863 * gfortran.dg/interface_45.f90: New test. Index: misc.c === --- misc.c (Revision 279064) +++ misc.c (Arbeitskopie) @@ -164,9 +164,19 @@ gfc_typename (gfc_typespec *ts) sprintf (buffer, "UNION(%s)", ts->u.derived->name); break; case BT_DERIVED: + if (ts->u.derived == NULL) + { + sprintf (buffer, "invalid type"); + break; + } sprintf (buffer, "TYPE(%s)", ts->u.derived->name); break; case BT_CLASS: + if (ts->u.derived == NULL) + { + sprintf (buffer, "invalid class"); + break; + } ts1 = ts->u.derived->components ? &ts->u.derived->components->ts : NULL; if (ts1 && ts1->u.derived && ts1->u.derived->attr.unlimited_polymorphic) sprintf (buffer, "CLASS(*)"); ! { dg-do compile } ! PR 92863 - this used to ICE ! Test case by Arseny Solokha. type(l1) function mp() ! { dg-error "type for function" } call sub(mp) ! { dg-error "Type mismatch" } end function mp function bi(ry) call sub(ry) ! { dg-error "Type mismatch" } end function bi
Re: [patch, fortran] Introduce -finline-pack
Am 09.12.19 um 17:30 schrieb Thomas Koenig: Maybe -finline-repack would be a better name? -finline-internal-pack? Steve made an excellent suggestion: -finline-arg-packing . So, OK with that change? Regards Thomas
[patch, fortran, committed] Fix PR 91643, repacking of assumed rank argument
Hello world, this fixes a regression introduced by my inline repacking patch. With the test case, it is simple and obvious enough - do not repack an assumed rank argument (which makes no sense). Committed as obvious and simple as r279203 after regression-testing. 2019-12-10 Thomas Koenig PR fortran/91643 * trans-array.c (gfc_conv_array_parameter): Do not repack an assume dummy argument. 2019-12-10 Thomas Koenig PR fortran/91643 * gfortran.dg/assumed_rank_18.f90: New test. Index: trans-array.c === --- trans-array.c (Revision 279064) +++ trans-array.c (Arbeitskopie) @@ -8141,6 +8141,8 @@ gfc_conv_array_parameter (gfc_se * se, gfc_expr * if (g77 && optimize && !optimize_size && expr->expr_type == EXPR_VARIABLE && !is_pointer (expr) && ! gfc_has_dimen_vector_ref (expr) + && !(expr->symtree->n.sym->as + && expr->symtree->n.sym->as->type == AS_ASSUMED_RANK) && (fsym == NULL || fsym->ts.type != BT_ASSUMED)) { gfc_conv_subref_array_arg (se, expr, g77, ! { dg-do run } ! PR 91643 - this used to cause an ICE. ! Original test case by Gerhard Steinmetz. program p real :: z(3) = [1.0, 2.0, 3.0] call g(z) contains subroutine g(x) real :: x(..) call h(x) end subroutine h(x) real :: x(*) if (x(1) /= 1.0) stop 1 end end
Re: [Patch, Fortran] PR92898 - [9/10 Regression] ICE in gfc_check_is_contiguous, at fortran/check.c:7157
Hello Harald, Index: gcc/fortran/check.c === --- gcc/fortran/check.c (Revision 279183) +++ gcc/fortran/check.c (Arbeitskopie) @@ -7154,7 +7154,9 @@ bool gfc_check_is_contiguous (gfc_expr *array) { if (array->expr_type == EXPR_NULL - && array->symtree->n.sym->attr.pointer == 1) + && (!array->symtree || + (array->symtree->n.sym && + array->symtree->n.sym->attr.pointer == 1))) I have to admit I do not understand the original code here, nor do I quite understand your fix. Is there any circumstance where array->expr_type == EXPR_NULL, but is_contiguous is valid? What would go wrong if the other tests were removed? Index: gcc/testsuite/gfortran.dg/pr91641.f90 === --- gcc/testsuite/gfortran.dg/pr91641.f90 (Revision 279183) +++ gcc/testsuite/gfortran.dg/pr91641.f90 (Arbeitskopie) @@ -1,7 +1,9 @@ ! { dg-do compile } ! PR fortran/91641 -! Code conyributed by Gerhard Steinmetz +! PR fortran/92898 +! Code contributed by Gerhard Steinmetz program p real, pointer :: z(:) print *, is_contiguous (null(z))! { dg-error "shall be an associated" } + print *, is_contiguous (null()) ! { dg-error "shall be an associated" } end Sometimes, it is necessary to change test cases, when error messages change. If this is not the case, it is better to add new tests to new test cases - this makes regression hunting much easier. Regards Thomas
*Ping* Introduce -finline-arg-packing
Am 10.12.19 um 22:22 schrieb Thomas Koenig: Steve made an excellent suggestion: -finline-arg-packing . So, OK with that change? In other words, is https://gcc.gnu.org/ml/gcc-patches/2019-12/msg00485.html OK with renaming the option to -finline-arg-packing ? Regards Thomas
Re: [Patch, fortran] PR92753 - [9/10 Regression] ICE in gfc_trans_call, at fortran/trans-stmt.c:392
Hi Paul, Regtested on FC31/x86_64 - OK for 9- and 10- branches? OK. Thanks for the patch! Regards Thomas
[patch, fortran] Fix PR 91541, ICE on valid for INDEX
Hello world, the attached patch fixes an ICE on valid for INDEX (see test case). The problem was that the KIND argument was still present during scalarization, which caused the ICE. The fix is to remove the KIND argument, and the best place to do this is in resolution. I did try to do this in gfc_conv_intrinsic_index_scan_verify, but it is too late by then. Removing the KIND argument required changing the call signature of gfc_resolve_index_func, which in turn required the rest of the changes (including the one in trans-decl.c - I am not convinced that what we are doing there is right, but for this bug fix, I left the functionality as is). Regression-tested. OK for trunk? Regards Thomas 2019-12-19 Thomas Koenig PR fortran/91541 * intrinsic.c (add_sym_4ind): New function. (add_functions): Use it for INDEX. (resolve_intrinsic): Also call f1m for INDEX. * intrinsic.h (gfc_resolve_index_func): Adjust prototype to take a gfc_arglist instead of individual arguments. * iresolve.c (gfc_resolve_index_func): Adjust arguments. Remove KIND argument if present, and make sure this is not done twice. * trans-decl.c: Include "intrinsic.h". (gfc_get_extern_function_decl): Special case for resolving INDEX. 2019-12-19 Thomas Koenig PR fortran/91541 * gfortran.dg/index_3.f90: New test. Index: intrinsic.c === --- intrinsic.c (Revision 279405) +++ intrinsic.c (Arbeitskopie) @@ -851,7 +851,40 @@ add_sym_4 (const char *name, gfc_isym_id id, enum (void *) 0); } +/* Add a symbol to the function list where the function takes 4 + arguments and resolution may need to change the number or + arrangement of arguments. This is the case for INDEX, which needs + its KIND argument removed. */ +static void +add_sym_4ind (const char *name, gfc_isym_id id, enum klass cl, int actual_ok, + bt type, int kind, int standard, + bool (*check) (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *), + gfc_expr *(*simplify) (gfc_expr *, gfc_expr *, gfc_expr *, + gfc_expr *), + void (*resolve) (gfc_expr *, gfc_actual_arglist *), + const char *a1, bt type1, int kind1, int optional1, + const char *a2, bt type2, int kind2, int optional2, + const char *a3, bt type3, int kind3, int optional3, + const char *a4, bt type4, int kind4, int optional4 ) +{ + gfc_check_f cf; + gfc_simplify_f sf; + gfc_resolve_f rf; + + cf.f4 = check; + sf.f4 = simplify; + rf.f1m = resolve; + + add_sym (name, id, cl, actual_ok, type, kind, standard, cf, sf, rf, + a1, type1, kind1, optional1, INTENT_IN, + a2, type2, kind2, optional2, INTENT_IN, + a3, type3, kind3, optional3, INTENT_IN, + a4, type4, kind4, optional4, INTENT_IN, + (void *) 0); +} + + /* Add a symbol to the subroutine list where the subroutine takes 4 arguments. */ @@ -2153,11 +2186,11 @@ add_functions (void) /* The resolution function for INDEX is called gfc_resolve_index_func because the name gfc_resolve_index is already used in resolve.c. */ - add_sym_4 ("index", GFC_ISYM_INDEX, CLASS_ELEMENTAL, ACTUAL_YES, - BT_INTEGER, di, GFC_STD_F77, - gfc_check_index, gfc_simplify_index, gfc_resolve_index_func, - stg, BT_CHARACTER, dc, REQUIRED, ssg, BT_CHARACTER, dc, REQUIRED, - bck, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL); + add_sym_4ind ("index", GFC_ISYM_INDEX, CLASS_ELEMENTAL, ACTUAL_YES, + BT_INTEGER, di, GFC_STD_F77, + gfc_check_index, gfc_simplify_index, gfc_resolve_index_func, + stg, BT_CHARACTER, dc, REQUIRED, ssg, BT_CHARACTER, dc, REQUIRED, + bck, BT_LOGICAL, dl, OPTIONAL, kind, BT_INTEGER, di, OPTIONAL); make_generic ("index", GFC_ISYM_INDEX, GFC_STD_F77); @@ -4434,9 +4467,10 @@ resolve_intrinsic (gfc_intrinsic_sym *specific, gf arg = e->value.function.actual; - /* Special case hacks for MIN and MAX. */ + /* Special case hacks for MIN, MAX and INDEX. */ if (specific->resolve.f1m == gfc_resolve_max - || specific->resolve.f1m == gfc_resolve_min) + || specific->resolve.f1m == gfc_resolve_min + || specific->resolve.f1m == gfc_resolve_index_func) { (*specific->resolve.f1m) (e, arg); return; Index: intrinsic.h === --- intrinsic.h (Revision 279405) +++ intrinsic.h (Arbeitskopie) @@ -517,8 +517,7 @@ void gfc_resolve_ibits (gfc_expr *, gfc_expr *, gf void gfc_resolve_ibset (gfc_expr *, gfc_expr *, gfc_expr *); void gfc_resolve_image_index (gfc_expr *, gfc_expr *, gfc_expr *); void gfc_resolve_image_status (gfc_expr *, gfc_expr *, gfc_expr *); -void gfc_resolve_index_func (gfc_expr *, gfc_expr *, gfc_expr *, gfc_expr *, - gfc_expr *); +void gfc_resolve_index_func (gfc_expr *, gfc_actual_arglist *); vo
Re: [Patch, Fortran] PR 92996 – fix rank resolution EXPR_ARRAY
Hi Tobias, One has the choice between (a) Using the location where the expression was defined (in the scoping unit) – currently done (i.e. replacing expr->where by expr->symtree->n.sym->where) (b) using the location where the parameter is used, i.e. keeping expr->where despite simplification. Or you could use both. With the nice colorization of error messages that David introduced gcc10, this is quite readable. Hm... this would require an additional member in the gfc_expr structure. While this may be a nice addition, it is certainly not necessary to get this patch in. So, OK for trunk, and thanks for the patch! Regards Thomas
[pach, fortran] Fix PR 92961, ICE on division by zero error in array bounds
Hello world, the attached patch fixes an ICE in an array declaration where the specified size came from 0/0. This is an 8/9/10 regression. The actual ICE fix was the NULL check in simplify_intrinsic_op, which in turn led to strange error messages, which are then corrected by the rest of the patch. Rather than try to transport state across I do not know how many levels of calls, I chose a global variable. Regression-tested. OK for all affected branches? Regards Thomas 2019-12-20 Thomas Koenig PR fortran/92961 * gfortran.h (gfc_seen_div0): Add declaration. * arith.h (gfc_seen_div0): Add definition. (eval_intrinsic): For integer division by zero, issue gfc_error_now and set gfc_seen_div0. * decl.c (variable_decl): If a division by zero error has been seen previously, do not issue an addtional error. * expr.c (simplify_intrinsic_op): Return NULL if op1 is NULL. 2019-12-20 Thomas Koenig PR fortran/92961 * gfortran.dg/arith_divide_2.f90: New test. Index: gfortran.h === --- gfortran.h (Revision 279639) +++ gfortran.h (Arbeitskopie) @@ -2995,6 +2995,8 @@ void gfc_arith_done_1 (void); arith gfc_check_integer_range (mpz_t p, int kind); bool gfc_check_character_range (gfc_char_t, int); +extern bool gfc_seen_div0; + /* trans-types.c */ bool gfc_check_any_c_kind (gfc_typespec *); int gfc_validate_kind (bt, int, bool); Index: arith.c === --- arith.c (Revision 279639) +++ arith.c (Arbeitskopie) @@ -32,6 +32,8 @@ along with GCC; see the file COPYING3. If not see #include "target-memory.h" #include "constructor.h" +bool gfc_seen_div0; + /* MPFR does not have a direct replacement for mpz_set_f() from GMP. It's easily implemented with a few calls though. */ @@ -1617,7 +1619,15 @@ eval_intrinsic (gfc_intrinsic_op op, if (rc != ARITH_OK) { - gfc_error (gfc_arith_error (rc), &op1->where); + if (rc == ARITH_DIV0 && op2->ts.type == BT_INTEGER) + { + gfc_error_now (gfc_arith_error (rc), &op2->where); + gfc_seen_div0 = true; + return NULL; + } + else + gfc_error (gfc_arith_error (rc), &op1->where); + if (rc == ARITH_OVERFLOW) goto done; return NULL; Index: decl.c === --- decl.c (Revision 279639) +++ decl.c (Arbeitskopie) @@ -2535,6 +2535,10 @@ variable_decl (int elem) goto cleanup; } + /* eval_intrinsic may signal a division by zero. */ + + gfc_seen_div0 = false; + /* F2018:C830 (R816) An explicit-shape-spec whose bounds are not constant expressions shall appear only in a subprogram, derived type definition, BLOCK construct, or interface body. */ @@ -2573,7 +2577,15 @@ variable_decl (int elem) if (not_constant) { - gfc_error ("Explicit shaped array with nonconstant bounds at %C"); + /* If there is a division by zero error, it will have been reported + previously using gfc_error_now in eval_intrinsic. */ + + if (!gfc_seen_div0) + gfc_error ("Explicit shaped array with nonconstant bounds at " + "%C"); + + gfc_seen_div0 = false; + m = MATCH_ERROR; goto cleanup; } Index: expr.c === --- expr.c (Revision 279639) +++ expr.c (Arbeitskopie) @@ -1153,6 +1153,9 @@ simplify_intrinsic_op (gfc_expr *p, int type) op2 = p->value.op.op2; op = p->value.op.op; + if (op1 == NULL) +return false; + if (!gfc_simplify_expr (op1, type)) return false; if (!gfc_simplify_expr (op2, type)) ! { dg-do compile } ! This used to ICE. Original test case by Gerhard Steinmetz. program p integer :: a((0)/0)! { dg-error "Division by zero" } integer :: b(0/(0))! { dg-error "Division by zero" } integer :: c((0)/(0)) ! { dg-error "Division by zero" } integer :: d(0/0) ! { dg-error "Division by zero" } end
OpenACC regression and development pace
Hi, I just saw this: FAIL: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times gimple "(?n)#pragma omp target oacc_enter_exit_data map\\(delete:MEM\\[\\(c_char \\*\\)[^\\]]+\\] \\[len: [^\\]]+\\]\\) map\\(to:del_f_p \\[pointer set, len: [0-9]+\\]\\) map\\(alloc:del_f_p\\.data \\[pointer assign, bias: [^\\]]+\\]\\) finalize$" 1 FAIL: gfortran.dg/goacc/finalize-1.f -O scan-tree-dump-times gimple "(?n)#pragma omp target oacc_enter_exit_data map\\(force_from:MEM\\[\\(c_char \\*\\)[^\\]]+\\] \\[len: [^\\]]+\\]\\) map\\(to:cpo_f_p \\[pointer set, len: [0-9]+\\]\\) map\\(alloc:cpo_f_p\\.data \\[pointer assign, bias: [^\\]]+\\]\\) finalize$" 1 Regarding what is currently going on with OpenACC: I do not claim to understand this area of the compiler, but it certainly seems that the current development is too hasty - too many patches flying around, too many regressions occurring. It might be better to slow this down somewhat, and to conduct a more throrough review process before committing. Regards Thomas
Re: [pach, fortran] Fix PR 92961, ICE on division by zero error in array bounds
the attached patch fixes an ICE in an array declaration where the specified size came from 0/0. This is an 8/9/10 regression. Actually, it does not fix all testcases in the PR, so some more tweaking is still needed. Regards Thomas
Re: *Ping* Introduce -finline-arg-packing
Hi Jakub, This patch broke: +FAIL: compiler driver --help=fortran option(s): "^ +-.*[^:.]\$" absent from output: " -finline-arg-packingPerform argument packing inline" That test verifies that the help texts of all options are terminated with dot or semicolon. Thanks, and sorry for the breakage. Note to self: Try not to forget that dot. This was not caught with "make check-fortran" from the gcc build directory. Maybe it would be a good idea to add that test to check-fortran too. Does anybody have an idea how to do that? Regards Thomas
Re: [Patch] PR92990 - fix error message for invalid argument of NULLIFY
Am 20.12.19 um 22:33 schrieb Harald Anlauf: The fix for PR70853 changed an ICE-on-invalid for NULLIFY into a misleading error message. The patch below rectifies that. OK for trunk? OK. Thanks for the patch! Regards Thomas
[patch, fortran] Updated fix PR 92961, ICE on division by zero error in array bounds
Hello world, here is an update for the fix for PR 92961, which also takes care of the second test case in the PR (included in the first one). The patch itself should be clear enough - make sure that there is a MATCH_ERROR on matching an array spec which contains 0/(0). Rather than pass around information several calls deep, I chose to use a global variable. Regression-tested. OK for trunk? (Only a few bugs to fix to be at least below 900 bugs at the end of the year, by the way - we are at 389 submitted bugs vs. 461 closed, which is not bad). Regards Thomas 2019-12-22 Thomas Koenig PR fortran/92961 * gfortran.h (gfc_seen_div0): Add declaration. * arith.h (gfc_seen_div0): Add definition. (eval_intrinsic): For integer division by zero, set gfc_seen_div0. * decl.c (variable_decl): If resolution resp. simplification fails for array spec and a division of zero error has been seen, return MATCH_ERROR. 2019-12-22 Thomas Koenig PR fortran/92961 * gfortran.dg/arith_divide_2.f90: New test. Index: gfortran.h === --- gfortran.h (Revision 279639) +++ gfortran.h (Arbeitskopie) @@ -2995,6 +2995,8 @@ void gfc_arith_done_1 (void); arith gfc_check_integer_range (mpz_t p, int kind); bool gfc_check_character_range (gfc_char_t, int); +extern bool gfc_seen_div0; + /* trans-types.c */ bool gfc_check_any_c_kind (gfc_typespec *); int gfc_validate_kind (bt, int, bool); Index: arith.c === --- arith.c (Revision 279639) +++ arith.c (Arbeitskopie) @@ -32,6 +32,8 @@ along with GCC; see the file COPYING3. If not see #include "target-memory.h" #include "constructor.h" +bool gfc_seen_div0; + /* MPFR does not have a direct replacement for mpz_set_f() from GMP. It's easily implemented with a few calls though. */ @@ -1620,6 +1622,10 @@ eval_intrinsic (gfc_intrinsic_op op, gfc_error (gfc_arith_error (rc), &op1->where); if (rc == ARITH_OVERFLOW) goto done; + + if (rc == ARITH_DIV0 && op2->ts.type == BT_INTEGER) + gfc_seen_div0 = true; + return NULL; } Index: decl.c === --- decl.c (Revision 279639) +++ decl.c (Arbeitskopie) @@ -2551,7 +2551,12 @@ variable_decl (int elem) for (int i = 0; i < as->rank; i++) { e = gfc_copy_expr (as->lower[i]); - gfc_resolve_expr (e); + if (!gfc_resolve_expr (e) && gfc_seen_div0) + { + m = MATCH_ERROR; + goto cleanup; + } + gfc_simplify_expr (e, 0); if (e && (e->expr_type != EXPR_CONSTANT)) { @@ -2561,7 +2566,12 @@ variable_decl (int elem) gfc_free_expr (e); e = gfc_copy_expr (as->upper[i]); - gfc_resolve_expr (e); + if (!gfc_resolve_expr (e) && gfc_seen_div0) + { + m = MATCH_ERROR; + goto cleanup; + } + gfc_simplify_expr (e, 0); if (e && (e->expr_type != EXPR_CONSTANT)) { @@ -2587,7 +2597,12 @@ variable_decl (int elem) if (e->expr_type != EXPR_CONSTANT) { n = gfc_copy_expr (e); - gfc_simplify_expr (n, 1); + if (!gfc_simplify_expr (n, 1) && gfc_seen_div0) + { + m = MATCH_ERROR; + goto cleanup; + } + if (n->expr_type == EXPR_CONSTANT) gfc_replace_expr (e, n); else @@ -2597,7 +2612,12 @@ variable_decl (int elem) if (e->expr_type != EXPR_CONSTANT) { n = gfc_copy_expr (e); - gfc_simplify_expr (n, 1); + if (!gfc_simplify_expr (n, 1) && gfc_seen_div0) + { + m = MATCH_ERROR; + goto cleanup; + } + if (n->expr_type == EXPR_CONSTANT) gfc_replace_expr (e, n); else @@ -2934,6 +2954,7 @@ variable_decl (int elem) cleanup: /* Free stuff up and return. */ + gfc_seen_div0 = false; gfc_free_expr (initializer); gfc_free_array_spec (as); ! { dg-do compile } ! This used to ICE. Original test case by Gerhard Steinmetz. program p integer :: a((0)/0)! { dg-error "Division by zero" } integer :: b(0/(0))! { dg-error "Division by zero" } integer :: c((0)/(0)) ! { dg-error "Division by zero" } integer :: d(0/0) ! { dg-error "Division by zero" } integer :: x = ubound(a,1) ! { dg-error "must be an array" } end
Re: [Patch] OpenACC – support "if" + "if_present" clauses with "host_data"
Hi Tobias, Build on x86-64-gnu-linux without offloading and with nvptx offloading. OK for the trunk? I can't really say a lot about OpenACC, but the changes do look reasonable. So, OK for trunk. Regards Thomas
*ping* [patch, fortran] Updated fix PR 92961, ICE on division by zero error in array bounds
Am 22.12.19 um 16:28 schrieb Thomas Koenig: here is an update for the fix for PR 92961, which also takes care of the second test case in the PR (included in the first one). The patch itself should be clear enough - make sure that there is a MATCH_ERROR on matching an array spec which contains 0/(0). Rather than pass around information several calls deep, I chose to use a global variable. Regression-tested. OK for trunk? Ping? I'd like to get the bug count at least go to 902 in the old year (if 900 cannot be achieved :-) Regards Thomas
*ping*[patch, fortran] Fix PR 91541, ICE on valid for INDEX
Am 19.12.19 um 08:23 schrieb Thomas Koenig: Regression-tested. OK for trunk? Ping?
Re: [patch, libfortran] Fortran 2018: Support d0.d, e0.d, es0.d, en0.d, g0.d and ew.d e0 edit descriptors
Hi Jerry, OK for trunk? Looks good. I also think that your approach that DEC stuff should be checked later is good. If it passes the testsuite, that's good enough for a commit. Thanks for the patch! Regards Thomas
[patch, fortran, committed] Fix dependency for %re and %im
Hello world, New year, new bug, new patch :-) I have just committed as obvious and simple the attached patch as r279821, where we failed to account for %re and %im in dependency checking. This is a 10 regression, gcc 9 works. Regards Thomas Handle REF_INQUIRY for dependency checking. 2020-01-01 Thomas Koenig PR fortran/93113 * dependency.c (gfc_dep_resolver): Handle REF_INQUIRY in switch for ref types. 2020-01-01 Thomas Koenig PR fortran/93113 * gfortran.dg/dependency_58.f90: New test. Index: dependency.c === --- dependency.c (Revision 279765) +++ dependency.c (Arbeitskopie) @@ -2286,6 +2286,12 @@ gfc_dep_resolver (gfc_ref *lref, gfc_ref *rref, gf subsequent references also overlap. */ break; + case REF_INQUIRY: + if (lref->u.i != rref->u.i) + return 0; + + break; + default: gcc_unreachable (); } ! { dg-do run } ! { dg-additional-options "-ffrontend-optimize -Warray-temporaries" } ! PR 93113 - this used to ICE, and should not generate a temporary. program main integer, parameter :: n = 10 complex, dimension(n,n) :: a, b, c real, dimension(n,n) :: r call random_number (r) c%re = r call random_number (r) c%im = r a = c b = c b%re = a%re - 0.5 b%im = a%im - 0.5 a%re = a%re - 0.5 a%im = a%im - 0.5 if (any (a /= b)) stop 1 b%im = a%re a%im = a%re if (any (a /= b)) stop 2 a = c b = c b(2:n,:)%re = a(1:n-1,:)%re a(2:n,:)%re = a(1:n-1,:)%re if (any (a /= b)) stop 3 a = c b = c b(1:n-1,:)%im = a(2:,:)%im a(1:n-1,:)%im = a(2:,:)%im if (any (a /= b)) stop 3 end program main
[patch, fortran] Fix PR 65428, ICE on nested empty array constructors
Hello world, the attached patch fixes an ICE where an array constructor containing an empty array constructor with a type didn't get the type from the inner constructor. The solution is to stash the type away in yet another variable and only use it if the constructor turns out to be empty, and the type has not been set some other way. Regression-tested. OK for trunk? Regards Thomas Save typespec for empty array constructor. 2020-01-02 Thomas Koenig PR fortran/65428 * array.c (empty_constructor): New variable. (empty_ts): New variable. (expand_constructor): Save typespec in empty_ts. Unset empty_constructor if there is an element. (gfc_expand_constructor): Initialize empty_constructor and empty_ts. If there was no explicit constructor type and the constructor is empty, take the type from empty_ts. 2020-01-02 Thomas Koenig PR fortran/65428 * gfortran.dg/zero_sized_11.f90: New test. Index: array.c === --- array.c (Revision 279821) +++ array.c (Arbeitskopie) @@ -1759,7 +1759,12 @@ cleanup: return t; } +/* Variables for noticing if all constructors are empty, and + if any of them had a type. */ +static bool empty_constructor; +static gfc_typespec empty_ts; + /* Expand a constructor into constant constructors without any iterators, calling the work function for each of the expanded expressions. The work function needs to either save or free the @@ -1782,6 +1787,9 @@ expand_constructor (gfc_constructor_base base) e = c->expr; + if (empty_constructor) + empty_ts = e->ts; + if (e->expr_type == EXPR_ARRAY) { if (!expand_constructor (e->value.constructor)) @@ -1790,6 +1798,7 @@ expand_constructor (gfc_constructor_base base) continue; } + empty_constructor = false; e = gfc_copy_expr (e); if (!gfc_simplify_expr (e, 1)) { @@ -1873,6 +1882,8 @@ gfc_expand_constructor (gfc_expr *e, bool fatal) iter_stack = NULL; + empty_constructor = true; + gfc_clear_ts (&empty_ts); current_expand.expand_work_function = expand; if (!expand_constructor (e->value.constructor)) @@ -1882,6 +1893,13 @@ gfc_expand_constructor (gfc_expr *e, bool fatal) goto done; } + /* If we don't have an explicit constructor type, and there + were only empty constructors, then take the type from + them. */ + + if (constructor_ts.type == BT_UNKNOWN && empty_constructor) +e->ts = empty_ts; + gfc_constructor_free (e->value.constructor); e->value.constructor = current_expand.base; ! { dg-do compile } ! PR 65428 - this used to ICE. Original test case by FX Coudert. program p integer :: i print *, [shape(1)] print *, [[ integer :: ]] print *, (/ (/ (i, i=1,0) /) /) end
Re: [Patch, Fortran] PR 92994 – add more ASSOCIATE checks
Hi Tobias, Build on x86-64-gnu-linux. OK for the trunk? Looks good. Thanks for the patch! Regards Thomas
Fortran patches to be reviewed (was: [Patch, Fortran] PR91640 – Fix call to contiguous dummy)
Hi Tobias, PS: I lost a bit the overview. Is there any patch pending review or otherwise pending? From my side, there is the patch for PR 65428, https://gcc.gnu.org/ml/gcc-patches/2020-01/msg00040.html Apart from that, I don't see any outstanding patches. Regards Thomas
*ping* [patch, fortran] Fix PR 65428, ICE on nested empty array constructors
Am 02.01.20 um 23:35 schrieb Thomas Koenig: Hello world, the attached patch fixes an ICE where an array constructor containing an empty array constructor with a type didn't get the type from the inner constructor. The solution is to stash the type away in yet another variable and only use it if the constructor turns out to be empty, and the type has not been set some other way. Regression-tested. OK for trunk? Ping?
Re: Fortran: Use non conflicting file extensions for intermediates [PR81615]
Hi Rimvydas, Documentation part. The makeinfo gcc/fortran/gfortran.texi does not seem to have any new warnings. Thanks for your work on this! I think this also desevers a mention in changes.html. Here is something that I came up with. OK? Or does anybody have suggestions for a better wording? diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html index 4b83037a..b3b67dda 100644 --- a/htdocs/gcc-14/changes.html +++ b/htdocs/gcc-14/changes.html @@ -282,8 +282,13 @@ a work-in-progress. - - +Fortran + + With the -save-temps option, .fii and +.fi files are now generated from .F90 +and .F files, respectively. + +
Re: Fortran: Use non conflicting file extensions for intermediates [PR81615]
Replying to myself... I think this also desevers a mention in changes.html. Here is something that I came up with. OK? Or does anybody have suggestions for a better wording? Or maybe this is better: diff --git a/htdocs/gcc-14/changes.html b/htdocs/gcc-14/changes.html index 4b83037a..d232f631 100644 --- a/htdocs/gcc-14/changes.html +++ b/htdocs/gcc-14/changes.html @@ -282,8 +282,14 @@ a work-in-progress. - - +Fortran + + With the -save-temps option, preprocessed files +with the .fii extension will be generated for +free-form source files such as .F90 and +.fi for fixed-form files such as .F. + +
Re: [PATCH 0/8] fortran: Inline MINLOC/MAXLOC without DIM argument [PR90608]
Am 08.08.24 um 11:09 schrieb Mikael Morin: As we are not planning to remove the library implementation (-Os!), this is also the best way to compare library to inline code. This makes perfect sense, but why reuse the -ffrontend-optimize option? The manual describes it as: This option performs front-end optimization, based on manipulating parts the Fortran parse tree These patches are about inlining, there is no manipulation of the parse tree. So I would rather use a separate option (-finline-intrinsics?). I concur, -ffrontend-optimize should remain specific to manipulating the parse tree. Having a -finline-intrinsic options, which would be set at any optimization level except -Os, may be the right way forward. Or something else :-) Best regards Thomas
Re: [Fortran, Patch, PR116292, v1] Fix 15-regression in move_alloc
Hi Andre, attached patch fixes a regression introduced by my previous patch on handling _vptr's more consistently. The patch gets the _vptr only of a derived or class type now and not of every type. Regression tested ok on x86_64-pc-linux-gnu / Fedora 39. Ok for mainline? OK (and looks obvious, too). Best regards Thomas
Re: New version of unsiged patch
Steve, On Sun, Aug 18, 2024 at 12:10:18PM +0200, Thomas Koenig wrote: this version of the patch includes DOT_PRODUCT, MATMUL and quite a few improvements for simplification. Thomas, Your updated patch applied cleanly on top-of-tree gcc. Bootstrap and regression testing on amd64-*-freebsd completed without any oddities. I'll read through the patch over the next few days. Any comments so far? I know the patch is very big :-) but I can also incorporate comments you make before you have read the whole thing. Best regards Thomas
Re: [PATCH] fortran: Support optional dummy as BACK argument of MINLOC/MAXLOC.
Hi Mikael, + gcc_assert (backexpr->expr_type == EXPR_VARIABLE); drop it, downgrade to checking, or is it worth? Whether it is worth it, I don't know; it's protecting the access to backexpr->symtree a few lines down, idependently of the implementation of maybe_absent_optional_variable. I expect the compiler to optimize it away, so I can surely make it a checking-only assert. I would also lean towards checking only. OK with that change (or, if you really prefer, as submitted is also fine). Thanks for the patch! It's good to see so much progress... Best regards Thomas
Re: [PATCH 0/8] fortran: Inline MINLOC/MAXLOC without DIM argument [PR90608]
Hi Mikael and Harald, - inline expansion is inhibited at -Os. But wouldn't it be good if we make this expansion also dependent on -ffrontend-optimize? (This was the case for rank-1 before your patch). The original idea was to have -ffrontend-optimize as a check if anything went wrong with front-end optimization in particular - if the bug went away with -fno-frontend-optimize, we knew where to look (and I knew I had to look). So, probably better to not do this at -Os. One thought: Should we also do the inlining without optimization? Very nice set of patches! Best regards Thomas
Re: New version of unsiged patch
Ping (a little bit)? With another weekend coming up, I would have some time to work on incorporating any feedback, or on putting in more intrinsics. Best regards Thomas
Re: New version of unsiged patch
Am 06.09.24 um 20:15 schrieb Steve Kargl: +Generally, unsigned integers are only permitted as data in intrinsics. Does the word 'intrinsics' apply to 'intrinsic operators' or 'intrinsic subprograms' or both? This might benefit from a big of wordiness. You're right, that wasn't clear at all. I have now replaced it with In intrinsic procedures, unsigned arguments are typically permitted for arguments for the data to be processed, analogous to the use of @code{REAL} arguments. Unsigned values are prohibited as index variables in @code{DO} loops and as array indices. which is clearer, I think. Best regards Thomas
Re: New version of unsiged patch
Am 06.09.24 um 20:58 schrieb Steve Kargl: Your testcases are all free source form. In fixed form, gfortran would need to deal with 'u = 1 2 3 4 u _8' I don't have the patch in my tree at the moment, it isn't clear to me if the matcher for an unsigned constant is prepared to deal with space between 4 an u in the above. The match_digits likely leaves the current locus at u, but I'll need to go check that. I have added a new test case ! { dg-do compile } ! { dg-options "-funsigned" } program memain print *,12u_8 print *,1 2u_8 print *,12 u_8 print *,12u _8 print *,12u_ 8 end as unsigned_24.f, which passes, so that should be OK. Thanks for the review! Best regards Thomas
[patch, fortran] Matmul and dot_product for unsigned
Hello world, like the subject says. The patch is gzipped because it is large; it contains multiple MATMUL library implementations. OK for trunk? Implement MATMUL and DOT_PRODUCT for unsgigned. gcc/fortran/ChangeLog: * arith.cc (gfc_arith_uminus): Fix warning. (gfc_arith_minus): Correctly truncate unsigneds. * check.cc (gfc_check_dot_product): Handle unsigned arguments. (gfc_check_matmul): Likewise. * expr.cc (gfc_get_unsigned_expr): New function. * gfortran.h (gfc_get_unsigned_expr): Add prototype. * gfortran.texi: Document MATMUL and DOT_PRODUCT for unsigned. * simplify.cc (compute_dot_product): Handle unsigneds. libgfortran/ChangeLog: * Makefile.am: Add unsigned MATMUL versions. * Makefile.in: Regenerate. * gfortran.map: Add unsigned MATMUL versions.o * libgfortran.h: Add array types for unsigned. * m4/iparm.m4: Add UNSIGNED if type is m. * generated/matmul_m1.c: New file. * generated/matmul_m16.c: New file. * generated/matmul_m2.c: New file. * generated/matmul_m4.c: New file. * generated/matmul_m8.c: New file. * generated/matmulavx128_m1.c: New file. * generated/matmulavx128_m16.c: New file. * generated/matmulavx128_m2.c: New file. * generated/matmulavx128_m4.c: New file. * generated/matmulavx128_m8.c: New file. gcc/testsuite/ChangeLog: * gfortran.dg/unsigned_25.f90: New test. * gfortran.dg/unsigned_26.f90: New test. 0001-Implement-MATMUL-and-DOT_PRODUCT-for-unsgigned.patch.gz Description: application/gzip
Re: [patch, fortran] Matmul and dot_product for unsigned
Am 09.09.24 um 09:19 schrieb Richard Biener: Is the library implementation in any way different from the signed one? Iff only multiplication and addition/subtraction are involved the unsigned implementation could implement both variants (the signed one would eventually cause undefinedness with respect to overflow unless built with -fwrapv). That would save code size in libgfortran and eventually icache if mixing uses of both. The versions for signed and unsigned matmul are generated from the same source, so that should work. But it will require some ugly m4 hackery... I'll take a look if I can make it work. Best regards Thomas
Re: [patch, fortran] Matmul and dot_product for unsigned
Am 09.09.24 um 20:01 schrieb Richard Biener: But it will require some ugly m4 hackery... I'll take a look if I can make it work. > I meant you shouldn’t need new library entry points for unsigned > but simply call the signed ones (and switch the signed implementation > to use unsigned arithmetic due to the overflow issue). That's clear. The evil m4 hackery comes from the way that the files are generated. The file name is passed to m4, which is then parsed with the macros in libgfortran/m4/iparm.m4 , where (among other things) 'i' in the name of the file to be generated is translated to GFC_INTEGER_... in the macros rtype, rtype_name and rtype_code (if that exists). So, I will need to either persudade iparm.m4 to check for the matmul name and redefine the get_typename macro accordingly, or (probably the easier option) to redefine rtype and friends if rtype_name happens to match ^GFC_INTEGER, in matmul.m4. Best regards Thomas
[patch, fortran, committed] module support for UNSIGNED
Hello world, I just pushed Steve's patch for module support to trunk as obvious, as https://gcc.gnu.org/g:2847a541c1f19b67ae84be8d0f6dc8e1f9371d16 . Best regards Thomas gcc/fortran/ChangeLog: * module.cc (bt_types): Add BT_UNSIGNED. gcc/testsuite/ChangeLog: * gfortran.dg/unsigned_kiss.f90: New test. diff --git a/gcc/fortran/module.cc b/gcc/fortran/module.cc index c565b84d61b..8cf58ff5142 100644 --- a/gcc/fortran/module.cc +++ b/gcc/fortran/module.cc @@ -2781,6 +2781,7 @@ static const mstring bt_types[] = { minit ("UNKNOWN", BT_UNKNOWN), minit ("VOID", BT_VOID), minit ("ASSUMED", BT_ASSUMED), +minit ("UNSIGNED", BT_UNSIGNED), minit (NULL, -1) }; diff --git a/gcc/testsuite/gfortran.dg/unsigned_kiss.f90 b/gcc/testsuite/gfortran.dg/unsigned_kiss.f90 new file mode 100644 index 000..46ee86ccd26 --- /dev/null +++ b/gcc/testsuite/gfortran.dg/unsigned_kiss.f90 @@ -0,0 +1,100 @@ +! +! { dg-do run } +! { dg-options "-funsigned" } +! +! Modern Fortran rewrite of Marsaglia's 64-bit KISS PRNG. +! https://www.thecodingforums.com/threads/64-bit-kiss-rngs.673657/ +! +module kissm + + implicit none + private + public uk, kseed, kiss + + integer, parameter :: uk = kind(1u_8) ! Check kind() works. + + ! Default seeds. Checks unsigned with parameter attribute. + unsigned(uk), parameter :: seed(4) = [ & + & 1234567890987654321u_uk, 362436362436362436u_uk, & + & 1066149217761810u_uk, 123456123456123456u_uk ] + + ! Seeds used during generation + unsigned(uk), save :: sd(4) = seed + + contains + + ! Tests unsigned in an internal function. + function s(x) + unsigned(uk) s + unsigned(uk), intent(in) :: x + s = ishft(x, -63)! Tests ishft + end function + + ! Poor seeding routine. Need to check v for entropy! + ! Tests intent(in) and optional attributes. + ! Tests ishftc() and array constructors. + subroutine kseed(v) + unsigned(uk), intent(in), optional :: v + if (present(v)) then +sd = seed + [ishftc(v,1), ishftc(v,15), ishftc(v,31), ishftc(v,44)] + else +sd = seed + end if + end subroutine kseed + + function kiss() + unsigned(uk) kiss + unsigned(uk) m, t + integer k + + ! Test unsigned in a statement function + m(t, k) = ieor(t, ishft(t, k)) + + t = ishft(sd(1), 58) + sd(4) + if (s(sd(1)) == s(t)) then +sd(4) = ishft(sd(1), -6) + s(sd(1)) + else +sd(4) = ishft(sd(1), -6) + 1u_uk - s(sd(1) + t) + endif + + sd(1) = t + sd(1) + sd(2) = m(m(m(sd(2), 13), -17), 43) + sd(3) = 6906969069u_uk * sd(3) + 1234567u_uk + kiss = sd(1) + sd(2) + sd(3) + end function kiss + +end module kissm + +program testkiss + use kissm + integer, parameter :: n = 4 + unsigned(uk) prn(4) + + ! Default sequence + unsigned(uk), parameter :: a(4) = [8932985056925012148u_uk, & + & 5710300428094272059u_uk, 18342510866933518593u_uk, & + & 14303636270573868250u_uk] + + ! Sequence with the seed 123412341234u_uk + unsigned(uk), parameter :: b(4) = [4002508872477953753u_uk, & + & 18025327658415290923u_uk, 16058856976144281263u_uk, & + & 11842224026193909403u_uk] + + do i = 1, n + prn(i) = kiss() + end do + if (any(prn /= a)) stop 1 + + call kseed(123412341234u_uk) + do i = 1, n + prn(i) = kiss() + end do + if (any(prn /= b)) stop 2 + + call kseed() + do i = 1, n + prn(i) = kiss() + end do + if (any(prn /= a)) stop 3 + +end program testkiss
[patch, teststuite, Fortran, committed] Fix endianness issue on test case
I just committed the fix for PR 116653 as obvious. Unfortunately, I left out the description in the ChangeLog, I hope it is clear enough. Best regards Thomas https://gcc.gnu.org/g:5d9486c29938d79beb798dce1a5509da54fe8c9f commit r15-3619-g5d9486c29938d79beb798dce1a5509da54fe8c9f Author: Thomas Koenig Date: Fri Sep 13 07:47:24 2024 +0200 Fix endianness issue on unsigned_21.f90. gcc/testsuite/ChangeLog: PR fortran/116653 * gfortran.dg/unsigned_21.f90: * gfortran.dg/unsigned_21_be.f90: New test. diff --git a/gcc/testsuite/gfortran.dg/unsigned_21.f90 b/gcc/testsuite/gfortran.dg/unsigned_21.f90 index 23302c7eabe..c3f65a469dc 100644 --- a/gcc/testsuite/gfortran.dg/unsigned_21.f90 +++ b/gcc/testsuite/gfortran.dg/unsigned_21.f90 @@ -1,5 +1,6 @@ ! { dg-do run } ! { dg-options "-funsigned" } +! { dg-require-effective-target le } program main integer :: i integer(2) :: j diff --git a/gcc/testsuite/gfortran.dg/unsigned_21_be.f90 b/gcc/testsuite/gfortran.dg/unsigned_21_be.f90 new file mode 100644 index 000..64fecd9cd4a --- /dev/null +++ b/gcc/testsuite/gfortran.dg/unsigned_21_be.f90 @@ -0,0 +1,14 @@ +! { dg-do run } +! { dg-options "-funsigned" } +! { dg-require-effective-target be } +program main + integer :: i + integer(2) :: j + unsigned :: u + i = -1 + u = transfer(i,u) + if (u /= huge(u)) error stop 1 + u = 4278058235u + j = transfer(u,j) + if (j /= -259) error stop 2 +end program main
[Patch, Fortran] Implement Unsigned for SUM and PRODUCT
Hello world, this implements SUM and PRODUCT for UNSIGNED. This is actually missing from 24-116.txt, but that is due to an oversight. This patch is on top of https://gcc.gnu.org/pipermail/fortran/2024-September/060975.html and uses the same method suggested by Richard: Use unsigned for the data in SUM and PRODUCT in the integer library version, and call the integer library version for unsigned. Regression-tested. OK for trunk? Best regards Thomas gcc/fortran/ChangeLog: * gfortran.texi: Document SUM and PRODUCT. * iresolve.cc (resolve_transformational): New argument, use_integer, to translate calls to unsigned to calls to integer. (gfc_resolve_product): Use it (gfc_resolve_sum): Use it. * simplify.cc (init_result_expr): Handle BT_UNSIGNED. libgfortran/ChangeLog: * generated/product_c10.c: Regenerated. * generated/product_c16.c: Regenerated. * generated/product_c17.c: Regenerated. * generated/product_c4.c: Regenerated. * generated/product_c8.c: Regenerated. * generated/product_i1.c: Regenerated. * generated/product_i16.c: Regenerated. * generated/product_i2.c: Regenerated. * generated/product_i4.c: Regenerated. * generated/product_i8.c: Regenarated. * generated/product_r10.c: Regenerated. * generated/product_r16.c: Regenerated. * generated/product_r17.c: Regenerated. * generated/product_r4.c: Regenerated. * generated/product_r8.c: Regenarated. * generated/sum_c10.c: Regenerated. * generated/sum_c16.c: Regenerated. * generated/sum_c17.c: Regenerated. * generated/sum_c4.c: Regenerated. * generated/sum_c8.c: Regenerated. * generated/sum_i1.c: Regenerated. * generated/sum_i16.c: Regenerated. * generated/sum_i2.c: Regenerated. * generated/sum_i4.c: Regenerated. * generated/sum_i8.c: Regenerated. * generated/sum_r10.c: Regenerated. * generated/sum_r16.c: Regenerated. * generated/sum_r17.c: Regenerated. * generated/sum_r4.c: Regenerated. * generated/sum_r8.c: Regenerated. * m4/ifunction.m4: Whitespace fix. * m4/product.m4: If type is integer, change to unsigned. * m4/sum.m4: Likewise.
Re: [Patch, Fortran] Implement Unsigned for SUM and PRODUCT
As Jerry wrote (thanks!), this was missing the attached patch. Best regards Thomasdiff --git a/gcc/fortran/gfortran.texi b/gcc/fortran/gfortran.texi index 829ab00c665..e5ffe67 100644 --- a/gcc/fortran/gfortran.texi +++ b/gcc/fortran/gfortran.texi @@ -2788,7 +2788,7 @@ As of now, the following intrinsics take unsigned arguments: @item @code{MVBITS} @item @code{RANGE} @item @code{TRANSFER} -@item @code{MATMUL} and @code{DOT_PRODUCT} +@item @code{SUM}, @code{PRODUCT}, @code{MATMUL} and @code{DOT_PRODUCT} @end itemize This list will grow in the near future. @c - diff --git a/gcc/fortran/iresolve.cc b/gcc/fortran/iresolve.cc index 32b31432e58..92a591cf6d7 100644 --- a/gcc/fortran/iresolve.cc +++ b/gcc/fortran/iresolve.cc @@ -175,9 +175,11 @@ resolve_bound (gfc_expr *f, gfc_expr *array, gfc_expr *dim, gfc_expr *kind, static void resolve_transformational (const char *name, gfc_expr *f, gfc_expr *array, - gfc_expr *dim, gfc_expr *mask) + gfc_expr *dim, gfc_expr *mask, + bool use_integer = false) { const char *prefix; + bt type; f->ts = array->ts; @@ -200,9 +202,18 @@ resolve_transformational (const char *name, gfc_expr *f, gfc_expr *array, gfc_resolve_dim_arg (dim); } + /* For those intrinsic like SUM where we the integer version + actually uses unsigned, but we call it as the integer + version. */ + + if (use_integer && array->ts.type == BT_UNSIGNED) +type = BT_INTEGER; + else +type = array->ts.type; + f->value.function.name = gfc_get_string (PREFIX ("%s%s_%c%d"), prefix, name, - gfc_type_letter (array->ts.type), + gfc_type_letter (type), gfc_type_abi_kind (&array->ts)); } @@ -2333,7 +2344,7 @@ void gfc_resolve_product (gfc_expr *f, gfc_expr *array, gfc_expr *dim, gfc_expr *mask) { - resolve_transformational ("product", f, array, dim, mask); + resolve_transformational ("product", f, array, dim, mask, true); } @@ -2881,7 +2892,7 @@ gfc_resolve_storage_size (gfc_expr *f, gfc_expr *a ATTRIBUTE_UNUSED, void gfc_resolve_sum (gfc_expr *f, gfc_expr *array, gfc_expr *dim, gfc_expr *mask) { - resolve_transformational ("sum", f, array, dim, mask); + resolve_transformational ("sum", f, array, dim, mask, true); } diff --git a/gcc/fortran/simplify.cc b/gcc/fortran/simplify.cc index 83d0fdc9ea9..e5681c42a48 100644 --- a/gcc/fortran/simplify.cc +++ b/gcc/fortran/simplify.cc @@ -359,7 +359,16 @@ init_result_expr (gfc_expr *e, int init, gfc_expr *array) mpz_set_si (e->value.integer, init); break; - case BT_REAL: + case BT_UNSIGNED: + if (init == INT_MIN) + mpz_set_ui (e->value.integer, 0); + else if (init == INT_MAX) + mpz_set (e->value.integer, gfc_unsigned_kinds[i].huge); + else + mpz_set_ui (e->value.integer, init); + break; + + case BT_REAL: if (init == INT_MIN) { mpfr_set (e->value.real, gfc_real_kinds[i].huge, GFC_RND_MODE); diff --git a/libgfortran/generated/product_c10.c b/libgfortran/generated/product_c10.c index 9438f5d7316..c57f1642b65 100644 --- a/libgfortran/generated/product_c10.c +++ b/libgfortran/generated/product_c10.c @@ -29,13 +29,13 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see #if defined (HAVE_GFC_COMPLEX_10) && defined (HAVE_GFC_COMPLEX_10) -extern void product_c10 (gfc_array_c10 * const restrict, +extern void product_c10 (gfc_array_c10 * const restrict, gfc_array_c10 * const restrict, const index_type * const restrict); export_proto(product_c10); void -product_c10 (gfc_array_c10 * const restrict retarray, - gfc_array_c10 * const restrict array, +product_c10 (gfc_array_c10 * const restrict retarray, + gfc_array_c10 * const restrict array, const index_type * const restrict pdim) { index_type count[GFC_MAX_DIMENSIONS]; @@ -188,15 +188,15 @@ product_c10 (gfc_array_c10 * const restrict retarray, } -extern void mproduct_c10 (gfc_array_c10 * const restrict, +extern void mproduct_c10 (gfc_array_c10 * const restrict, gfc_array_c10 * const restrict, const index_type * const restrict, gfc_array_l1 * const restrict); export_proto(mproduct_c10); void -mproduct_c10 (gfc_array_c10 * const restrict retarray, - gfc_array_c10 * const restrict array, - const index_type * const restrict pdim, +mproduct_c10 (gfc_array_c10 * const restrict retarray, + gfc_array_c10 * const restrict array, + const index_type * const restrict pdim, gfc_array_l1 * const restrict mask) { index_type count[GFC_MAX_DIMENSIONS]; @@ -378,15 +378,15 @@ mproduct_c10 (gfc_array_c10 * const restrict retarray, } -extern void sproduct_c10 (gfc_array_c10 * const restrict, +extern void sproduct_c10 (gfc_array_c10 * const restrict, gfc_array_c10 * const restrict, const index_type * const restrict, GFC_LOGICAL_4 *); export_proto(sproduct_c10); void -sproduct_c10 (gfc_array_c10 * const restrict retarray,
Re: [Patch, fortran] PR57798 uninitialized loop bound with sum and array-returning function.
Hi Mikael, > Regression tested on x86_64-unknown-linux-gnu. OK for trunk/4.8? OK for both. Thanks for the patch! Thomas
[Patch, fortran] PR 52243 - avoid reallocation checks on assignment when possible
Hello world, the attached patch avoids checks for reallocation on assignment when the same variable is on both sides of the assignment. Regression-tested. OK for trunk? Thomas 2013-08-29 Thomas Koenig PR fortran/52243 * trans-expr.c (is_runtime_conformable): New function. * gfc_trans_assignment_1: Use it. 2013-08-29 Thomas Koenig PR fortran/52243 * gfortran.dg/realloc_on_assign_14.f90: Remove warning made obsolete by patch. * gfortran.dg/realloc_on_assign_19.f90: New test. Index: fortran/trans-expr.c === --- fortran/trans-expr.c (Revision 201996) +++ fortran/trans-expr.c (Arbeitskopie) @@ -7738,7 +7738,103 @@ alloc_scalar_allocatable_for_assignment (stmtblock } } +/* Check for assignments of the type + a = a + 4 + + to make sure we do not check for reallocation unneccessarily. */ + + +static bool +is_runtime_conformable (gfc_expr *expr1, gfc_expr *expr2) +{ + gfc_actual_arglist *a; + gfc_expr *e1, *e2; + + switch (expr2->expr_type) +{ +case EXPR_VARIABLE: + return gfc_dep_compare_expr (expr1, expr2) == 0; + +case EXPR_FUNCTION: + if (expr2->value.function.esym + && expr2->value.function.esym->attr.elemental) + { + for (a = expr2->value.function.actual; a != NULL; a = a->next) + { + e1 = a->expr; + if (e1->rank > 0 && !is_runtime_conformable (expr1, e1)) + return false; + } + return true; + } + else if (expr2->value.function.isym + && expr2->value.function.isym->elemental) + { + for (a = expr2->value.function.actual; a != NULL; a = a->next) + { + e1 = a->expr; + if (e1->rank > 0 && !is_runtime_conformable (expr1, e1)) + return false; + } + return true; + } + + break; + +case EXPR_OP: + switch (expr2->value.op.op) + { + case INTRINSIC_NOT: + case INTRINSIC_UPLUS: + case INTRINSIC_UMINUS: + case INTRINSIC_PARENTHESES: + return is_runtime_conformable (expr1, expr2->value.op.op1); + + case INTRINSIC_PLUS: + case INTRINSIC_MINUS: + case INTRINSIC_TIMES: + case INTRINSIC_DIVIDE: + case INTRINSIC_POWER: + case INTRINSIC_AND: + case INTRINSIC_OR: + case INTRINSIC_EQV: + case INTRINSIC_NEQV: + case INTRINSIC_EQ: + case INTRINSIC_NE: + case INTRINSIC_GT: + case INTRINSIC_GE: + case INTRINSIC_LT: + case INTRINSIC_LE: + case INTRINSIC_EQ_OS: + case INTRINSIC_NE_OS: + case INTRINSIC_GT_OS: + case INTRINSIC_GE_OS: + case INTRINSIC_LT_OS: + case INTRINSIC_LE_OS: + + e1 = expr2->value.op.op1; + e2 = expr2->value.op.op2; + + if (e1->rank == 0 && e2->rank > 0) + return is_runtime_conformable (expr1, e2); + else if (e1->rank > 0 && e2->rank == 0) + return is_runtime_conformable (expr1, e1); + else if (e1->rank > 0 && e2->rank > 0) + return is_runtime_conformable (expr1, e1) + && is_runtime_conformable (expr1, e2); + break; + + default: + break; + + } +default: + break; +} + return false; +} + /* Subroutine of gfc_trans_assignment that actually scalarizes the assignment. EXPR1 is the destination/LHS and EXPR2 is the source/RHS. init_flag indicates initialization expressions and dealloc that no @@ -7935,7 +8031,8 @@ gfc_trans_assignment_1 (gfc_expr * expr1, gfc_expr && gfc_is_reallocatable_lhs (expr1) && !gfc_expr_attr (expr1).codimension && !gfc_is_coindexed (expr1) - && expr2->rank) + && expr2->rank + && !is_runtime_conformable (expr1, expr2)) { realloc_lhs_warning (expr1->ts.type, true, &expr1->where); ompws_flags &= ~OMPWS_SCALARIZER_WS; Index: testsuite/gfortran.dg/realloc_on_assign_14.f90 === --- testsuite/gfortran.dg/realloc_on_assign_14.f90 (Revision 201996) +++ testsuite/gfortran.dg/realloc_on_assign_14.f90 (Arbeitskopie) @@ -23,7 +23,7 @@ str = 'abc'! { dg-warning "Code for reallocati astr = 'abc' ! no realloc astr = ['abc'] ! { dg-warning "Code for reallocating the allocatable array" } a = reshape(a,shape(a)) ! { dg-warning "Code for reallocating the allocatable array" } -r = sin(r) ! { dg-warning "Code for reallocating the allocatable array" } +r = sin(r) r = sin(r(1)) ! no realloc b = sin(r(1)) ! { dg-warning "Code for reallocating the allocatable variable" } ! { dg-do compile } ! { dg-options "-fdump-tree-original" } ! PR 52243 - avoid check for reallocation when doing simple ! assignments with the same variable on both sides. module foo contains elemental function ele(a) real, intent(in) :: a real :: ele ele = 1./(2+a) end function ele subroutine bar(a) real, dimension(:), allocatable :: a a = a * 2.0 a = sin(a-0.3) a = ele(a) end subroutine bar end module foo ! { dg-final { scan-tree-dump-times "alloc" 0 "original" } } ! { dg-final { cleanup-tree-dump "original" } }
[patch, fortran] PR 56519: Reject impure intrinsic subroutines in DO CONCURRENT
Hello world, the attached patch rejects impure subroutines, such as RANDOM_NUMBER, within DO CONCURRENT. Regression-tested. OK for trunk? Regards Thomas 2013-08-29 Thomas Koenig PR fortran/PR56519 * gfortran.h: Declare gfc_do_concurrent_flag as extern. * resolve.c: Rename do_concurrent_flag to gfc_do_concurrent_flag. and make non-static. (resolve_function): Use gfc_do_concurrent_flag instead of do_concurrent_flag. (pure_subroutine): Likewise. (resolve_code): Likewise. (resolve_types): Likewise. * intrinsic.c (gfc_intrinsic_sub_interface): Raise error for non-pure intrinsic subroutines within DO CONCURRENT. 2013-08-29 Thomas Koenig PR fortran/PR56519 * gfortran.dg/do_concurrent_3.f90: New test case. Index: gfortran.h === --- gfortran.h (Revision 201996) +++ gfortran.h (Arbeitskopie) @@ -2846,6 +2846,7 @@ gfc_expr *gfc_expr_to_initialize (gfc_expr *); bool gfc_type_is_extensible (gfc_symbol *); bool gfc_resolve_intrinsic (gfc_symbol *, locus *); bool gfc_explicit_interface_required (gfc_symbol *, char *, int); +extern int gfc_do_concurrent_flag; /* array.c */ Index: intrinsic.c === --- intrinsic.c (Revision 201996) +++ intrinsic.c (Arbeitskopie) @@ -4397,6 +4397,13 @@ gfc_intrinsic_sub_interface (gfc_code *c, int erro c->resolved_sym->attr.elemental = isym->elemental; } + if (gfc_do_concurrent_flag && !isym->pure) +{ + gfc_error ("Subroutine call to intrinsic '%s' in DO CONCURRENT " + "block at %L is not PURE", name, &c->loc); + return MATCH_ERROR; +} + if (gfc_pure (NULL) && !isym->pure) { gfc_error ("Subroutine call to intrinsic '%s' at %L is not PURE", name, Index: resolve.c === --- resolve.c (Revision 201996) +++ resolve.c (Arbeitskopie) @@ -60,7 +60,7 @@ static code_stack *cs_base = NULL; /* Nonzero if we're inside a FORALL or DO CONCURRENT block. */ static int forall_flag; -static int do_concurrent_flag; +int gfc_do_concurrent_flag; /* True when we are resolving an expression that is an actual argument to a procedure. */ @@ -2986,11 +2986,11 @@ resolve_function (gfc_expr *expr) forall_flag == 2 ? "mask" : "block"); t = false; } - else if (do_concurrent_flag) + else if (gfc_do_concurrent_flag) { gfc_error ("Reference to non-PURE function '%s' at %L inside a " "DO CONCURRENT %s", name, &expr->where, - do_concurrent_flag == 2 ? "mask" : "block"); + gfc_do_concurrent_flag == 2 ? "mask" : "block"); t = false; } else if (gfc_pure (NULL)) @@ -3059,7 +3059,7 @@ pure_subroutine (gfc_code *c, gfc_symbol *sym) if (forall_flag) gfc_error ("Subroutine call to '%s' in FORALL block at %L is not PURE", sym->name, &c->loc); - else if (do_concurrent_flag) + else if (gfc_do_concurrent_flag) gfc_error ("Subroutine call to '%s' in DO CONCURRENT block at %L is not " "PURE", sym->name, &c->loc); else if (gfc_pure (NULL)) @@ -9629,7 +9629,7 @@ resolve_code (gfc_code *code, gfc_namespace *ns) { frame.current = code; forall_save = forall_flag; - do_concurrent_save = do_concurrent_flag; + do_concurrent_save = gfc_do_concurrent_flag; if (code->op == EXEC_FORALL) { @@ -9663,9 +9663,9 @@ resolve_code (gfc_code *code, gfc_namespace *ns) to transform the SELECT TYPE into ASSOCIATE first. */ break; case EXEC_DO_CONCURRENT: - do_concurrent_flag = 1; + gfc_do_concurrent_flag = 1; gfc_resolve_blocks (code->block, ns); - do_concurrent_flag = 2; + gfc_do_concurrent_flag = 2; break; case EXEC_OMP_WORKSHARE: omp_workshare_save = omp_workshare_flag; @@ -9684,7 +9684,7 @@ resolve_code (gfc_code *code, gfc_namespace *ns) if (code->op != EXEC_COMPCALL && code->op != EXEC_CALL_PPC) t = gfc_resolve_expr (code->expr1); forall_flag = forall_save; - do_concurrent_flag = do_concurrent_save; + gfc_do_concurrent_flag = do_concurrent_save; if (!gfc_resolve_expr (code->expr2)) t = false; @@ -14404,7 +14404,7 @@ resolve_types (gfc_namespace *ns) } forall_flag = 0; - do_concurrent_flag = 0; + gfc_do_concurrent_flag = 0; gfc_check_interfaces (ns); gfc_traverse_ns (ns, resolve_values); ! { dg-do compile } ! PR 56519 - flag impure intrinsic subroutine calls ! within DO CONCURRENT program main implicit none integer :: i real :: array(123), val do concurrent (i = 1:123) call random_number (val) ! { dg-error "is not PURE" } array(i) = val end do end program main
[patch, fortran, docs] Unformatted sequential and special files
Hello world, the attached patch documents the format for unformatted sequential files and what is, and is not, supported with special files. Tested with "make dvi" and "make info". OK for trunk? Thomas 2013-08-30 Thomas Koenig PR fortran/30162 * gfortran.texi: Document unformatted sequential file format and I/O with special files. Index: gfortran.texi === --- gfortran.texi (Revision 201996) +++ gfortran.texi (Arbeitskopie) @@ -1121,6 +1121,8 @@ * Internal representation of LOGICAL variables:: * Thread-safety of the runtime library:: * Data consistency and durability:: +* Unformatted sequential file format:: +* I/O with special files:: @end menu @@ -1291,7 +1293,75 @@ releasing @code{fcntl} file locks, if the server supports them, will also force cache validation and flushing dirty data and metadata. +@node Unformatted sequential file format +@section Unformatted sequential file format +@cindex unformatted sequential files +@cindex record marker +@cindex subrecord +Unformatted sequential files are stored using record markers. Each +full record consists of a leading record marker, the data written +by the user program, and a trailing record marker. The record markers +are four-byte integers by default, and eight-byte integers if the +@option{-fmax-subrecord-length=8} option is in effect. Each record +marker contains the number of bytes of data in the record. + +The maximum number of bytes of user data in a record is 2147483639 for +a four-byte record marker. If this is exceeded, a record is split into +subrecords. Each subrecord also has a leading and a trailing record +marker. If the leading record marker contains a negative number, the +number of user data bytes in the subrecord equals the absolute value +of this number, and another subrecord follows the current one. If the +trailing record marker contains a negative number, then the number of +bytes of user data equals the absolute value of that number, and there +is a preceding subrecord. + +The format for unformatted sequential data can be duplicated using +unformatted stream, as shown in this example program: + +@smallexample +program main + implicit none + integer :: i + real, dimension(10) :: a, b + call random_number(a) + open (10,file='test.dat',form='unformatted',access='stream') + inquire (iolength=i) a + write (10) i, a, i + close (10) + open (10,file='test.dat',form='unformatted') + read (10) b + if (all (a == b)) print *,'success!' +end program main +@end smallexample + +@node I/O with special files +@section I/O with special files +@cindex special files +@cindex pipes +@cindex FIFO +@cindex terminal devices +@cindex BACKSPACE + +Special files such as pipes, FIFOs or terminal devices are supported +only for the following types of file access: + +@itemize + +@item Formatted sequential + +@item Formatted stream + +@item Unformatted stream + +@end itemize + +Unformatted sequential file access is not supported for +special files. If necessary, it can be simulated using +unformatted stream, see @ref{Unformatted sequential file format}. + +@code{BACKSPACE} is not supported for special files. + @c - @c Extensions @c -
Re: [patch, fortran, docs] Unformatted sequential and special files
Hi Janne, I have tried to answer your points in the attached patch. > The unformatted sequential format part looks Ok, but the special files > section is lacking. E.g. what about > > - The REWIND statement? Not supported. > - The ENDFILE statement? Not supported. > - ACCESS='stream' and the POS= specifier? Only for inpquire. > - ACCESS='direct'? (I suspect this should work for > pipes/FIFO's/terminals in case REC= numbers are sequential, but is > that a guarantee we want to make?) I have not listed this as supported in the patch. > - Special files which are special in other ways. E.g. block special > files tend to allow seeking, but IO must be block aligned. Also listed as not supported; I suspect buffering could cause grief there. I have updated an attached patch. OK? Thomas 2013-08-30 Thomas Koenig PR fortran/30162 * gfortran.texi: Document unformatted sequential file format and I/O with special files. ig25@linux-fd1f:~/Krempel/Unformatt Index: gfortran.texi === --- gfortran.texi (Revision 201996) +++ gfortran.texi (Arbeitskopie) @@ -1121,6 +1121,8 @@ * Internal representation of LOGICAL variables:: * Thread-safety of the runtime library:: * Data consistency and durability:: +* Unformatted sequential file format:: +* I/O with special files:: @end menu @@ -1291,7 +1293,85 @@ releasing @code{fcntl} file locks, if the server supports them, will also force cache validation and flushing dirty data and metadata. +@node Unformatted sequential file format +@section Unformatted sequential file format +@cindex unformatted sequential files +@cindex record marker +@cindex subrecord +Unformatted sequential files are stored using record markers. Each +full record consists of a leading record marker, the data written +by the user program, and a trailing record marker. The record markers +are four-byte integers by default, and eight-byte integers if the +@option{-fmax-subrecord-length=8} option is in effect. Each record +marker contains the number of bytes of data in the record. + +The maximum number of bytes of user data in a record is 2147483639 for +a four-byte record marker. If this is exceeded, a record is split into +subrecords. Each subrecord also has a leading and a trailing record +marker. If the leading record marker contains a negative number, the +number of user data bytes in the subrecord equals the absolute value +of this number, and another subrecord follows the current one. If the +trailing record marker contains a negative number, then the number of +bytes of user data equals the absolute value of that number, and there +is a preceding subrecord. + +The format for unformatted sequential data can be duplicated using +unformatted stream, as shown in this example program: + +@smallexample +program main + implicit none + integer :: i + real, dimension(10) :: a, b + call random_number(a) + open (10,file='test.dat',form='unformatted',access='stream') + inquire (iolength=i) a + write (10) i, a, i + close (10) + open (10,file='test.dat',form='unformatted') + read (10) b + if (all (a == b)) print *,'success!' +end program main +@end smallexample + +@node I/O with special files +@section I/O with special files +@cindex special files +@cindex pipes +@cindex FIFO +@cindex terminal devices +@cindex block devices +@cindex sockets +@cindex BACKSPACE +@cindex REWIND +@cindex ENDFILE + +Special character-oriented files such as pipes, FIFOs or terminal +devices are supported only for the following types of file access: + +@itemize + +@item Formatted sequential + +@item Formatted stream + +@item Unformatted stream + +@end itemize + +For special files, the @code{POS=} specifier for stream I/O can only +be used in @code{INQUIRE} statements. + +Unformatted sequential file access is @emph{not} supported for special +files. If necessary, it can be simulated using unformatted stream, +see @ref{Unformatted sequential file format}. + +I/O to and from block devices are also not supported. + +@code{BACKSPACE}, @code{REWIND} and @code{ENDFILE} are not supported +for special files. + @c - @c Extensions @c -
*ping* [patch, fortran] PR 56519: Reject impure intrinsic subroutines in DO CONCURRENT
Ping**0.5714 ? > the attached patch rejects impure subroutines, such as RANDOM_NUMBER, > within DO CONCURRENT. > > Regression-tested. OK for trunk?
Re: [patch, fortran, docs] Unformatted sequential and special files
Hello world, here is a rewrite, which I hope make things more clear. Unformatted sequential files are now made up of subrecords, where a logical record may only have one. Regarding block devices: I don't know anybody who ever used them from gfortran, so I tried to be as vague as possible. Any more suggestions? OK for trunk otherwise? Thomas Index: gfortran.texi === --- gfortran.texi (Revision 201996) +++ gfortran.texi (Arbeitskopie) @@ -1121,6 +1121,8 @@ * Internal representation of LOGICAL variables:: * Thread-safety of the runtime library:: * Data consistency and durability:: +* Unformatted sequential file format:: +* I/O with special files:: @end menu @@ -1291,7 +1293,109 @@ releasing @code{fcntl} file locks, if the server supports them, will also force cache validation and flushing dirty data and metadata. +@node Unformatted sequential file format +@section Unformatted sequential file format +@cindex unformatted sequential +@cindex sequential, unformatted +@cindex record marker +@cindex subrecord +Unformatted sequential files are stored as logical records using +record markers. Each logical record consists of one of more subrecords. + +Each subrecord consists of a leading record marker, the data written +by the user program, and a trailing record marker. The record markers +are four-byte integers by default, and eight-byte integers if the +@option{-fmax-subrecord-length=8} option (which exists for backwards +compability reasons only) is in effect. + +The maximum number of bytes of user data in a subrecord is 2147483639 +(2 GiB - 9) for a four-byte record marker. If a logical record +contains more data, the data is distributed among several subrecords. + +The absolute of the number stored in the record markers is the number +of bytes of user data in the corresponding subrecord. If the leading +record marker of a subrecord contains a negative number, another +subrecord follows the current one. If the trailing record marker +contains a negative number, then there is a preceding subrecord. + +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, + +The format for unformatted sequential data can be duplicated using +unformatted stream, as shown in the example program for a single +subrecord only: + +@smallexample +program main + use iso_fortran_env, only: int32 + implicit none + integer(int32) :: i + real, dimension(10) :: a, b + call random_number(a) + open (10,file='test.dat',form='unformatted',access='stream') + inquire (iolength=i) a + write (10) i, a, i + close (10) + open (10,file='test.dat',form='unformatted') + read (10) b + if (all (a == b)) print *,'success!' +end program main +@end smallexample + +@node I/O with special files +@section I/O with special files +@cindex special files +@cindex character devices +@cindex devices, character +@cindex block devices +@cindex devices, block +@cindex sockets +@cindex special files, character +@cindex BACKSPACE +@cindex REWIND +@cindex ENDFILE +@cindex pipes +@cindex FIFO +@cindex terminal devices +@cindex unformatted sequential +@cindex sequential, unformatted +@cindex unformatted stream +@cindex stream, unformatted +@cindex formatted stream +@cindex stream, formatted +@cindex direct access + +Special character files such as pipes, FIFOs, sockets or terminal +devices are supported only for the following types of file access: + +@itemize + +@item Formatted sequential + +@item Formatted stream + +@item Unformatted stream + +@end itemize + +For special character files, the @code{POS=} specifier for stream I/O +can only be used in @code{INQUIRE} statements. + +@code{BACKSPACE}, @code{REWIND} and @code{ENDFILE} are not supported +for character special files. + +Unformatted sequential and direct file access are @emph{not} supported +for character special files. If necessary, unformatted sequential +access can be simulated using unformatted stream, see @ref{Unformatted +sequential file format}. + +Block devices have not been tested. It is likely that only +unformatted stream and direct access will work for those. Some +restrictions specific to the operating system regarding sizes and +alignment of data may apply. + @c - @c Extensions @c -
Re: [Patch, Fortran] PR57697 - Fix an issue with defined assignment
Hi Tobias, the patch is OK, also for 4.8. Thanks a lot for fixing this. Just a couple of nits: - You may want to remove the output from the test case. - The two consecutive ifs in > > if (left != 0B) > { > if (_F.DA0 != 0B) goto L.2; > _F.DA0 = (struct parent *) __builtin_malloc (4); > L.2:; > *_F.DA0 = *left; > } > L.1:; > if (left != 0B) goto L.4; are a little bit inelegant. It is not really important, because they will be merged on optimization, but if you find an easy way to do this in the FE code, you might want to consider doing so. I would advise against spending a lot of work on this, though :-) Thomas
Re: [Patch, Fortran] PR57697 - Fix an issue with defined assignment
Hi Tobias, > As testing showed, it didn't fix the real-world code: ForTrilinos's > ForTrilinos_ADT_3D_Burgers_6th_Pade did still fail as it has: > > *_F.DA65 = matrix_diff_x (&parm.621); >_F.DA66 = ax->epetra_rowmatrix.universal; // Deref of "ax"! > > Build and regtested on x86-64-gnu-linux. > OK? The patch is OK, also for 4.8. Please add a test case which also checks for the ForTrilinos failure. Thomas
Re: *PING* Re: [Patch, Fortran] PR57697/58469 - Fix another defined-assignment issue
Hi Tobias, > * PING * http://gcc.gnu.org/ml/fortran/2013-09/msg00039.html > > Additionally pinging for: > http://gcc.gnu.org/ml/fortran/2013-09/msg00031.html Both are OK. Thanks a lot for the patches! Thomas
*ping* [patch, fortran] Handle -Wextra, -fcompare-reals is implied with -Wextra
I wrote: the attatched patch (this time for real!) implements -Wextra for the Fortran front end, and adds -fcompare-reals to -Wextra. Ping?
Re: [patch, fortran] Fix PR 52724, internal list-directed read/write with kind=4
Hi Janus, I'm not much of a libgfortran or I/O expert, but after all this looks ok to me. Thanks for the patch! Committed, thanks. What to people think about backporting to 4.7? It is a wrong-code issue, if a rather obscure corner. If there's no feedback within a few days, I think I'll close the PR. Thomas
[patch, libfortran] Fix PR 54736, memory corruption with GFORTRAN_CONVERT_UNIT
Hello world, the attached patch fixes the PR. The logic for processing GFORTRAN_CONVERT_UNIT had been quite wrong. I have checked that the original test case, plus a few more, no longer cause assertion failures or memory corruption (also checked with valgrind). No test case because it is not possible to set environment variables from the testsuite. Regression-tested. OK for trunk? I would also like to backport this to 4.7, and maybe 4.6. What do you think? Thomas 2012-09-29 Thomas König PR libfortran/54736 * runtime/environ.c (search_unit): Correct logic for binary search. (mark_single): Fix index errors. Index: runtime/environ.c === --- runtime/environ.c (Revision 191649) +++ runtime/environ.c (Arbeitskopie) @@ -459,21 +459,34 @@ search_unit (int unit, int *ip) { int low, high, mid; - low = -1; - high = n_elist; + if (n_elist == 0) +{ + *ip = 0; + return 0; +} + + low = 0; + high = n_elist - 1; while (high - low > 1) { mid = (low + high) / 2; - if (unit <= elist[mid].unit) + if (unit == elist[mid].unit) + { + *ip = mid; + return 1; + } + else if (unit > elist[mid].unit) + low = mid; + else high = mid; - else - low = mid; } - *ip = high; - if (elist[high].unit == unit) -return 1; + + if (unit > elist[high].unit) +*ip = high; else -return 0; +*ip = low; + + return 0; } /* This matches a keyword. If it is found, return the token supplied, @@ -588,13 +601,13 @@ mark_single (int unit) } if (search_unit (unit, &i)) { - elist[unit].conv = endian; + elist[i].conv = endian; } else { - for (j=n_elist; j>=i; j--) + for (j=n_elist-1; j>=i; j--) elist[j+1] = elist[j]; - + n_elist += 1; elist[i].unit = unit; elist[i].conv = endian;
Re: [Patch, Fortran, OOP] PR 54667: gimplification failure with c_f_pointer
Hi Janus, Regtested on x86_64-unknown-linux-gnu. Ok for trunk? This looks all right to me (although I'm not really an expert :-) OK, and thanks for the patch! Thomas
Re: [patch, libfortran] Fix PR 54736, memory corruption with GFORTRAN_CONVERT_UNIT
Hello world, the previous version of the patch has an issue that Shane pointed out in the PR. This version should work; at least it survived all the test cases I could come up with. Regression-tested (again). OK for trunk? Also for 4.6 and 4.7? Thomas 2012-10-01 Thomas König PR libfortran/54736 * runtime/environ.c (search_unit): Correct logic for binary search. (mark_single): Fix index errors. Index: runtime/environ.c === --- runtime/environ.c (Revision 191857) +++ runtime/environ.c (Arbeitskopie) @@ -459,21 +459,35 @@ search_unit (int unit, int *ip) { int low, high, mid; - low = -1; - high = n_elist; - while (high - low > 1) + if (n_elist == 0) { + *ip = 0; + return 0; +} + + low = 0; + high = n_elist - 1; + + do +{ mid = (low + high) / 2; - if (unit <= elist[mid].unit) - high = mid; + if (unit == elist[mid].unit) + { + *ip = mid; + return 1; + } + else if (unit > elist[mid].unit) + low = mid + 1; else - low = mid; -} - *ip = high; - if (elist[high].unit == unit) -return 1; + high = mid - 1; +} while (low <= high); + + if (unit > elist[mid].unit) +*ip = mid + 1; else -return 0; +*ip = mid; + + return 0; } /* This matches a keyword. If it is found, return the token supplied, @@ -588,13 +602,13 @@ mark_single (int unit) } if (search_unit (unit, &i)) { - elist[unit].conv = endian; + elist[i].conv = endian; } else { - for (j=n_elist; j>=i; j--) + for (j=n_elist-1; j>=i; j--) elist[j+1] = elist[j]; - + n_elist += 1; elist[i].unit = unit; elist[i].conv = endian;
[patch, fortran] PR 54833 Don't wrap calls to free(a) in if (a != NULL)
Hello world, the attached patch removes wrapping calls to free(a) by if (a != NULL) for some cases. It is not complete, because automatic deallocation of allocatable structure components is not yet covered. OK for trunk? Thomas 2012-10-06 Thomas König PR fortran/54833 * trans.c (gfc_call_free): Do not wrap the call to __builtin_free in check for NULL. (gfc_deallocate_with_status): For automatic deallocation without status, don't wrap call to __builtin_free in check for NULL. 2012-10-06 Thomas König PR fortran/54833 * gfortran.dg/auto_dealloc_3.f90: New test. Index: trans.c === --- trans.c (Revision 191857) +++ trans.c (Arbeitskopie) @@ -814,26 +814,23 @@ gfc_allocate_allocatable (stmtblock_t * block, tre } -/* Free a given variable, if it's not NULL. */ +/* Free a given variable. If it is NULL, free takes care of this + automatically. */ tree gfc_call_free (tree var) { stmtblock_t block; - tree tmp, cond, call; + tree call; if (TREE_TYPE (var) != TREE_TYPE (pvoid_type_node)) var = fold_convert (pvoid_type_node, var); gfc_start_block (&block); var = gfc_evaluate_now (var, &block); - cond = fold_build2_loc (input_location, NE_EXPR, boolean_type_node, var, - build_int_cst (pvoid_type_node, 0)); call = build_call_expr_loc (input_location, builtin_decl_explicit (BUILT_IN_FREE), 1, var); - tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, cond, call, - build_empty_stmt (input_location)); - gfc_add_expr_to_block (&block, tmp); + gfc_add_expr_to_block (&block, call); return gfc_finish_block (&block); } @@ -861,11 +858,10 @@ gfc_call_free (tree var) } } - In this front-end version, status doesn't have to be GFC_INTEGER_4. - Moreover, if CAN_FAIL is true, then we will not emit a runtime error, - even when no status variable is passed to us (this is used for - unconditional deallocation generated by the front-end at end of - each procedure). + In this front-end version, status doesn't have to be GFC_INTEGER_4. If + CAN_FAIL is true and no status variable is passed, we will simply call + free(). This is used for unconditional deallocation generated by the + front-end at end of each procedure. If a runtime-message is possible, `expr' must point to the original expression being deallocated for its locus and variable name. @@ -890,6 +886,14 @@ gfc_deallocate_with_status (tree pointer, tree sta STRIP_NOPS (pointer); } + if (can_fail && status == NULL_TREE) +{ + tmp = build_call_expr_loc (input_location, + builtin_decl_explicit (BUILT_IN_FREE), 1, + fold_convert (pvoid_type_node, pointer)); + return tmp; +} + cond = fold_build2_loc (input_location, EQ_EXPR, boolean_type_node, pointer, build_int_cst (TREE_TYPE (pointer), 0)); ! { dg-do compile } ! { dg-options "-fdump-tree-original" } ! PR fortran/54833 ! Make sure we don't wrap a free() in an if(a.data != NULL) by ! counting the ifs. subroutine foo real, allocatable :: a(:) allocate(a(10)) end subroutine foo ! { dg-final { scan-tree-dump-times "if" 3 "original" } } ! { dg-final { cleanup-tree-dump "original" } }
Re: [patch, fortran] PR 54833 Don't wrap calls to free(a) in if (a != NULL)
I wrote: the attached patch removes wrapping calls to free(a) by if (a != NULL) for some cases. It is not complete, because automatic deallocation of allocatable structure components is not yet covered. I accidentally posted an old version, which had a bug in coarrays (basically was just missing an "else"). Regression-tested. OK for trunk? Thomas 2012-10-06 Thomas König PR fortran/54833 * trans.c (gfc_call_free): Do not wrap the call to __builtin_free in check for NULL. (gfc_deallocate_with_status): For automatic deallocation without status for non-coarrays, don't wrap call to __builtin_free in check for NULL. 2012-10-06 Thomas König PR fortran/54833 * gfortran.dg/auto_dealloc_3.f90: New test. Index: trans.c === --- trans.c (Revision 191857) +++ trans.c (Arbeitskopie) @@ -814,26 +814,23 @@ gfc_allocate_allocatable (stmtblock_t * block, tre } -/* Free a given variable, if it's not NULL. */ +/* Free a given variable. If it is NULL, free takes care of this + automatically. */ tree gfc_call_free (tree var) { stmtblock_t block; - tree tmp, cond, call; + tree call; if (TREE_TYPE (var) != TREE_TYPE (pvoid_type_node)) var = fold_convert (pvoid_type_node, var); gfc_start_block (&block); var = gfc_evaluate_now (var, &block); - cond = fold_build2_loc (input_location, NE_EXPR, boolean_type_node, var, - build_int_cst (pvoid_type_node, 0)); call = build_call_expr_loc (input_location, builtin_decl_explicit (BUILT_IN_FREE), 1, var); - tmp = fold_build3_loc (input_location, COND_EXPR, void_type_node, cond, call, - build_empty_stmt (input_location)); - gfc_add_expr_to_block (&block, tmp); + gfc_add_expr_to_block (&block, call); return gfc_finish_block (&block); } @@ -861,11 +858,10 @@ gfc_call_free (tree var) } } - In this front-end version, status doesn't have to be GFC_INTEGER_4. - Moreover, if CAN_FAIL is true, then we will not emit a runtime error, - even when no status variable is passed to us (this is used for - unconditional deallocation generated by the front-end at end of - each procedure). + In this front-end version, status doesn't have to be GFC_INTEGER_4. If + CAN_FAIL is true, no status variable is passed and we are not dealing with + a coarray, we will simply call free(). This is used for unconditional + deallocation generated by the front-end at end of each procedure. If a runtime-message is possible, `expr' must point to the original expression being deallocated for its locus and variable name. @@ -890,6 +886,14 @@ gfc_deallocate_with_status (tree pointer, tree sta STRIP_NOPS (pointer); } + else if (can_fail && status == NULL_TREE) +{ + tmp = build_call_expr_loc (input_location, + builtin_decl_explicit (BUILT_IN_FREE), 1, + fold_convert (pvoid_type_node, pointer)); + return tmp; +} + cond = fold_build2_loc (input_location, EQ_EXPR, boolean_type_node, pointer, build_int_cst (TREE_TYPE (pointer), 0)); ! { dg-do compile } ! { dg-options "-fdump-tree-original" } ! PR fortran/54833 ! Make sure we don't wrap a free() in an if(a.data != NULL) by ! counting the ifs. subroutine foo real, allocatable :: a(:) allocate(a(10)) end subroutine foo ! { dg-final { scan-tree-dump-times "if" 3 "original" } } ! { dg-final { cleanup-tree-dump "original" } }
*ping* [patch, libfortran] Fix PR 54736, memory corruption with GFORTRAN_CONVERT_UNIT
Am 01.10.2012 20:34, schrieb Thomas Koenig: Hello world, the previous version of the patch has an issue that Shane pointed out in the PR. This version should work; at least it survived all the test cases I could come up with. Regression-tested (again). OK for trunk? Also for 4.6 and 4.7? Ping? I would like to start committing patches so I don't have too many of them in my tree at the same time :-) Thomas
Re: *ping* [patch, libfortran] Fix PR 54736, memory corruption with GFORTRAN_CONVERT_UNIT
Hi Steven, Ping? I would like to start committing patches so I don't have too many of them in my tree at the same time :-) This looks OK to me. Committed as rev. 192158. I'll commit to 4.7 and after that to 4.6 in a few days. Thanks a lot for the review! Thomas
Re: [Patch, Fortran] PR 40453: Enhanced (recursive) argument checking
Hi Janus, ping! 2012/10/7 Janus Weil : Hi all, here is a rather straightforward patch, which does 'recursive' checking of dummy procedures. Regtested on x86_64-unknown-linux-gnu. Ok for trunk? This is OK. Thanks for the patch! Thomas
Re: *ping* [patch, fortran] Handle -Wextra, -fcompare-reals is implied with -Wextra
Hi Janus, In the docu of -Wall, there is a list of which switches are included, but -Wc-binding-type is missing. Maybe you can add it? And how about also adding a short docu paragraph for Wextra? I've added both items to the documentation as you suggested, and committed as rev. 192649. Here is the patch as I committed it. Thanks a lot for your review! Now we can start thinking if we want to add other flags to -Wextra. Regards Thomas 012-10-21 Thomas Koenig PR fortran/54465 * lang.opt (Wextra): Add. * invoke.texi: Document that -Wc-binding-type, -Wconversion and -Wline-truncation are implied by -Wall. Document that -Wcompare-reals is implied by -Wextra. Document -Wextra. * options.c (set_Wextra): New function. (gfc_handle_option): Handle -Wextra. 2012-10-21 Thomas Koenig PR fortran/54465 * gfortran.dg/wextra_1.f: New test. Index: lang.opt === --- lang.opt (revision 192638) +++ lang.opt (working copy) @@ -230,6 +230,10 @@ Wconversion-extra Fortran Warning Warn about most implicit conversions +Wextra +Fortran Warning +Print extra (possibly unwanted) warnings + Wfunction-elimination Fortran Warning Warn about function call elimination Index: invoke.texi === --- invoke.texi (revision 192638) +++ invoke.texi (working copy) @@ -727,7 +727,7 @@ warnings. Enables commonly used warning options pertaining to usage that we recommend avoiding and that we believe are easy to avoid. This currently includes @option{-Waliasing}, @option{-Wampersand}, -@option{-Wconversion}, @option{-Wsurprising}, +@option{-Wconversion}, @option{-Wsurprising}, @option{-Wc-binding-type}, @option{-Wintrinsics-std}, @option{-Wno-tabs}, @option{-Wintrinsic-shadow}, @option{-Wline-truncation}, @option{-Wtarget-lifetime}, @option{-Wreal-q-constant} and @option{-Wunused}. @@ -778,7 +778,8 @@ avoid such temporaries. Warn if the a variable might not be C interoperable. In particular, warn if the variable has been declared using an intrinsic type with default kind instead of using a kind parameter defined for C interoperability in the -intrinsic @code{ISO_C_Binding} module. +intrinsic @code{ISO_C_Binding} module. This option is implied by +@option{-Wall}. @item -Wcharacter-truncation @opindex @code{Wcharacter-truncation} @@ -788,7 +789,8 @@ Warn when a character assignment will truncate the @item -Wline-truncation @opindex @code{Wline-truncation} @cindex warnings, line truncation -Warn when a source code line will be truncated. +Warn when a source code line will be truncated. This option is +implied by @option{-Wall}. @item -Wconversion @opindex @code{Wconversion} @@ -803,6 +805,14 @@ the expression after conversion. Implied by @optio @cindex conversion Warn about implicit conversions between different types and kinds. +@item -Wextra +@opindex @code{Wextra} +@cindex extra warnings +@cindex warnings, extra +Enables some warning options for usages of language features which +may be problematic. This currently includes @option{-Wcompare-reals} +and @option{-Wunused-parameter}. + @item -Wimplicit-interface @opindex @code{Wimplicit-interface} @cindex warnings, implicit interface @@ -884,7 +894,7 @@ encountered, which yield an UNDERFLOW during compi Warn if a user-defined procedure or module procedure has the same name as an intrinsic; in this case, an explicit interface or @code{EXTERNAL} or @code{INTRINSIC} declaration might be needed to get calls later resolved to -the desired intrinsic/procedure. +the desired intrinsic/procedure. This option is implied by @option{-Wall}. @item -Wunused-dummy-argument @opindex @code{Wunused-dummy-argument} @@ -939,6 +949,7 @@ allocatable variable; this includes scalars and de @item -Wcompare-reals @opindex @code{Wcompare-reals} Warn when comparing real or complex types for equality or inequality. +This option is implied by @option{-Wextra}. @item -Wtarget-lifetime @opindex @code{Wtargt-lifetime} Index: trans.c === --- trans.c (revision 192638) +++ trans.c (working copy) @@ -814,26 +814,23 @@ gfc_allocate_allocatable (stmtblock_t * block, tre } -/* Free a given variable, if it's not NULL. */ +/* Free a given variable. If it is NULL, free takes care of this + automatically. */ tree gfc_call_free (tree var) { stmtblock_t block; - tree tmp, cond, call; + tree call; if (TREE_TYPE (var) != TREE_TYPE (pvoid_type_node)) var = fold_convert (pvoid_type_node, var); gfc_start_block (&block); var = gfc_evaluate_now (var, &block); - cond = fold_build2_loc (input_location, NE_EXPR, boolean_type_node, var, - build_int_cst (pvoid_type_node, 0)); call = build_call_expr_loc (input_location, builtin_decl_expli
[patch, wwwdocs, committed] Fortran changes for 4.8
Hello world, I have committed the attached patch as obvious after checking with "tidy". Best regards Thomas 2012-10-21 Thomas Koenig * gcc-4.8/changes.html: Document that -Wc-binding-type is enabled by -Wall. Document that -Wcompare-reals is enabled by -Wextra. Index: changes.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-4.8/changes.html,v retrieving revision 1.47 diff -u -r1.47 changes.html --- changes.html 16 Oct 2012 22:48:07 - 1.47 +++ changes.html 21 Oct 2012 10:56:28 - @@ -149,7 +149,8 @@ particular, if the variable has been declared using an intrinsic type with default kind instead of using a kind parameter defined for C interoperability in the intrinsic ISO_C_Binding module. Before, -the warning was always printed. +the warning was always printed. The -Wc-binding-type flag +is enabled by -Wall. The http://gcc.gnu.org/onlinedocs/gfortran/Error-and-Warning-Options.html";> @@ -168,8 +169,8 @@ warnings are issued when comparing REAL or COMPLEX types for equality and inequality; consider replacing a == b by abs(a−b) < eps with a suitable -eps. The -Wcompare-reals flag is enabled by --Wall. +eps. The -Wcompare-reals flag is enabled by +-Wextra. The http://gcc.gnu.org/onlinedocs/gfortran/Error-and-Warning-Options.html";>
Re: *ping* [patch, fortran] Handle -Wextra, -fcompare-reals is implied with -Wextra
Am 27.10.2012 01:41, schrieb Andreas Schwab: Thomas Koenig writes: Index: trans.c === --- trans.c (revision 192638) +++ trans.c (working copy) @@ -814,26 +814,23 @@ gfc_allocate_allocatable (stmtblock_t * block, tre What's this? Something that I accidentally committed, which is part of a patch pending review (see http://gcc.gnu.org/ml/gcc-patches/2012-10/msg00613.html). Reverted as of revision 192875. Thomas
Re: [patch, fortran] PR 54833 Don't wrap calls to free(a) in if (a != NULL)
Hi Andreas, Thomas Koenig writes: PR fortran/54833 * trans.c (gfc_call_free): Do not wrap the call to __builtin_free in check for NULL. (gfc_deallocate_with_status): For automatic deallocation without status for non-coarrays, don't wrap call to __builtin_free in check for NULL. spawn /home/andreas/src/gcc/m68k/gcc/testsuite/gfortran/../../gfortran -B/home/andreas/src/gcc/m68k/gcc/testsuite/gfortran/../../ -B/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/ /home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90 -fno-diagnostics-show-caret -Os -pedantic-errors -B/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -L/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -L/home/andreas/src/gcc/m68k/m68k-linux/./libgfortran/.libs -lm -o ./alloc_comp_assign_1.exe /home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90: In function 'main': /home/andreas/src/gcc/gcc/gcc/testsuite/gfortran.dg/alloc_comp_assign_1.f90:57:0: internal compiler error: in cselib_record_set, at cselib.c:2379 0x1029ac87 cselib_record_set ../../gcc/gcc/cselib.c:2379 Strange, this looks more like a bug that is exposed through the patch. I also don't see how such a change to the Fortran front end can result in something invalid in postreload. Does not happen on the architectures that I used, and I do not have access to that architecture. Does a (rougly) equivalent C case compile? Thomas
Re: *ping* Re: [Patch, Fortran] Fix some libgfortran issues found by coverity
Hi Tobias, * ping * On 16.10.2012 23:18, Tobias Burnus wrote: In the Bessel-function algorithm, there was the useless code: ret->base_addr = ret->base_addr; The patch is OK. Thanks a lot! Thomas
Re: *ping* Re: [Patch, Fortran] PR54958 - Allow ac-implied-do and data-implied-do with INTENT(IN)
Hi Tobias, * ping * This is OK. Thanks for the patch! Thomas
[patch, RFC] PR 30146, warning/errors for potentially changing values in DO loops
Hello world, the attached patch, which is not in its final stage, implements some warnings for index variables of DO loops. For the following situations, errors/warnings are issued when an index loop variable is passed as an actual argument: - If the dummy argument has INTENT(OUT). I think an error should be issued unconditionally. - If the dummy argument has INTENT(INOUT). My opinion is that a warning should be issued unconditionally, but I am open to the opinions that an error would be better, or that it should depend on an option. - If the dummy argument has no INTENT, or if the procedure has no explicit interface, I think that there should be a warning depending on an option (which I haven't yet implemented). Opinions? If there is agreement on the question of which options should select which errors/warnings, then I will submit a final patch including some more comments, a ChangeLog entry and a deja-gnuified test case. Thomas Index: frontend-passes.c === --- frontend-passes.c (Revision 192894) +++ frontend-passes.c (Arbeitskopie) @@ -39,6 +39,7 @@ static bool optimize_trim (gfc_expr *); static bool optimize_lexical_comparison (gfc_expr *); static void optimize_minmaxloc (gfc_expr **); static bool empty_string (gfc_expr *e); +static void do_warn (gfc_namespace *); /* How deep we are inside an argument list. */ @@ -76,12 +77,29 @@ static bool in_omp_workshare; static int iterator_level; -/* Entry point - run all passes for a namespace. So far, only an - optimization pass is run. */ +/* Keep track of DO loop levels. */ +static gfc_code **do_list; +static int do_size, do_level; + +/* Vector of gfc_expr * to keep track of DO loops. */ + +struct my_struct *evec; + +/* Entry point - run all passes for a namespace. */ + void gfc_run_passes (gfc_namespace *ns) { + + /* Warn about dubious DO loops where the index might + change. */ + + do_size = 20; + do_list = XNEWVEC(gfc_code *, do_size); + do_warn (ns); + XDELETEVEC (do_list); + if (gfc_option.flag_frontend_optimize) { expr_size = 20; @@ -605,6 +623,7 @@ optimize_namespace (gfc_namespace *ns) current_ns = ns; forall_level = 0; iterator_level = 0; + do_level = 0; in_omp_workshare = false; gfc_code_walker (&ns->code, convert_do_while, dummy_expr_callback, NULL); @@ -1225,6 +1244,157 @@ optimize_minmaxloc (gfc_expr **e) mpz_set_ui (a->expr->value.integer, 1); } +static int +do_code (gfc_code **c, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + gfc_code *co; + int i; + gfc_formal_arglist *f; + gfc_actual_arglist *a; + + co = *c; + + switch (co->op) +{ +case EXEC_DO: + if (do_level >= do_size) + { + do_size = 2 * do_size; + do_list = XRESIZEVEC (gfc_code *, do_list, do_size); + } + + if (co->ext.iterator && co->ext.iterator->var) + do_list[do_level] = co; + else + do_list[do_level] = NULL; + break; + +case EXEC_CALL: + a = co->ext.actual; + f = co->symtree->n.sym->formal; + + while (a) + { + for (i=0; iext.iterator->var->symtree->n.sym; + + if (a->expr && a->expr->symtree + && a->expr->symtree->n.sym == do_sym) + { + if (f) + { + if (f->sym->attr.intent == INTENT_OUT) + gfc_error_now("Variable '%s' at %L redefined inside loop " + "beginning at %L as INTENT(OUT) argument to " + "subroutine '%s'", do_sym->name, &a->expr->where, + &do_list[i]->loc, co->symtree->n.sym->name); + else if (f->sym->attr.intent == INTENT_INOUT) + gfc_warning_now("Variable '%s' at %L may redefined inside loop " + "beginning at %L as INTENT(INOUT) argument to " + "subroutine '%s'", do_sym->name, &a->expr->where, + &do_list[i]->loc, co->symtree->n.sym->name); + else if (f->sym->attr.intent == INTENT_UNKNOWN) + gfc_warning_now("Variable '%s' at %L may redefined inside loop " + "beginning at %L as argument to " + "subroutine '%s'", do_sym->name, &a->expr->where, + &do_list[i]->loc, co->symtree->n.sym->name); + } + else + gfc_warning_now("Variable '%s' at %L may redefined inside loop " +"beginning at %L as argument to " +"subroutine '%s'", do_sym->name, &a->expr->where, +&do_list[i]->loc, co->symtree->n.sym->name); + } + } + a = a->next; + if (f) + f = f->next; + } + break; + +default: + break; +} + return 0; +} + +static int +do_function (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + gfc_formal_arglist *f; + gfc_actual_arglist *a; + gfc_expr *expr; + int i; + + expr = *e; + if (expr->expr_type != EXPR_FUNCTION) +return 0; + + /* Intrinsic functions don't modify their arguments. */ + + if (expr->value.function.isym) +return 0; + + a = expr->value.function.actual; + f = expr->symtree->n.sym->formal; + + while (a) +
Re: [patch, fortran] Inline MATMUL(A,TRANSPOSE(B)), PR 66094
Hi Toon, However, today I *did* run the test harness with your modification: ... Thanks for the testing! So, what do people think? Is the patch OK for trunk? Regards Thomas
Re: [Patch, Fortran] PR69397 and PR6844 Internal Compiler Errors2
Hi Jerry, The attached patch with new test cases fixes these by replacing gcc_assert and updating the error message depending on whether resolving an initialization expression or not. Regression tested on x86-64. OK for trunk? OK. Thanks for the patch! Thomas