[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 janus at gcc dot gnu.org changed: What|Removed |Added Status|ASSIGNED|RESOLVED See Also||https://gcc.gnu.org/bugzill ||a/show_bug.cgi?id=58224 Resolution|--- |FIXED Assignee|tkoenig at gcc dot gnu.org |janus at gcc dot gnu.org Target Milestone|--- |9.0 --- Comment #15 from janus at gcc dot gnu.org --- The issue is fixed with r263471. As a follow up, the -fcheck diagnostics should be improved (see e.g. PR 58224), so that cases like the one in comment #0 give a proper runtime error instead of a segfault.
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 --- Comment #14 from janus at gcc dot gnu.org --- Author: janus Date: Fri Aug 10 14:08:53 2018 New Revision: 263471 URL: https://gcc.gnu.org/viewcvs?rev=263471=gcc=rev Log: 2018-08-10 Janus Weil PR fortran/57160 * invoke.texi (frontend-optimize): Mention short-circuiting. * options.c (gfc_post_options): Disable -ffrontend-optimize with -Og. * resolve.c (resolve_operator): Warn about short-circuiting only with -ffrontend-optimize. * trans-expr.c (gfc_conv_expr_op): Use short-circuiting operators only with -ffrontend-optimize. Without that flag, make sure that both operands are evaluated. 2018-08-10 Janus Weil PR fortran/57160 * gfortran.dg/actual_pointer_function_1.f90: Fix invalid test case. * gfortran.dg/inline_matmul_23.f90: Add option "-ffrontend-optimize". * gfortran.dg/short_circuiting_2.f90: New test case. * gfortran.dg/short_circuiting_3.f90: New test case. Added: trunk/gcc/testsuite/gfortran.dg/short_circuiting_2.f90 trunk/gcc/testsuite/gfortran.dg/short_circuiting_3.f90 Modified: trunk/gcc/fortran/ChangeLog trunk/gcc/fortran/invoke.texi trunk/gcc/fortran/options.c trunk/gcc/fortran/resolve.c trunk/gcc/fortran/trans-expr.c trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90 trunk/gcc/testsuite/gfortran.dg/inline_matmul_23.f90
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 janus at gcc dot gnu.org changed: What|Removed |Added Keywords||accepts-invalid --- Comment #13 from janus at gcc dot gnu.org --- It should be noted that ifort (version 18.3) gives a nice runtime error for the code in comment 0 with the proper checking flags: $ ifort -check all -traceback c0.f90 $ ./a.out forrtl: severe (408): fort: (7): Attempt to use pointer M when it is not associated with a target Image PCRoutineLineSource a.out 004056C0 Unknown Unknown Unknown a.out 00402CA3 m1_mp_s1_ 8 c0.f90 a.out 00402DAE MAIN__ 16 c0.f90 a.out 00402BAE Unknown Unknown Unknown libc-2.23.so 7FF4052CF830 __libc_start_main Unknown Unknown a.out 00402AA9 Unknown Unknown Unknown This underlines once more that the code is actually invalid (in case there was still doubt about that).
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 --- Comment #12 from Dominique d'Humieres --- > Patch posted at: https://gcc.gnu.org/ml/fortran/2018-07/msg00086.html Don't abuse any optimization option.
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 --- Comment #11 from janus at gcc dot gnu.org --- Patch posted at: https://gcc.gnu.org/ml/fortran/2018-07/msg00086.html
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 --- Comment #10 from janus at gcc dot gnu.org --- (In reply to janus from comment #9) > As noted already somewhere in the discussion of PR85599 on the mailing list, > this breaks actual_pointer_function_1.f90 in the testsuite ... but apart from that the patch in comment #9 regtests cleanly.
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 janus at gcc dot gnu.org changed: What|Removed |Added CC||janus at gcc dot gnu.org --- Comment #9 from janus at gcc dot gnu.org --- (In reply to Thomas Koenig from comment #6) > The problem there is what we should consider for a warning. I think getting the warnings right for all possible cases is pretty tough. OTOH, following Joost's original suggestion to do short-circuiting only with -ffrontend-optimize is almost trivial, so I'd vote to go with that. Here's the patch: Index: gcc/fortran/trans-expr.c === --- gcc/fortran/trans-expr.c(revision 262859) +++ gcc/fortran/trans-expr.c(working copy) @@ -3348,12 +3348,18 @@ gfc_conv_expr_op (gfc_se * se, gfc_expr * expr) return; case INTRINSIC_AND: - code = TRUTH_ANDIF_EXPR; + if (flag_frontend_optimize) + code = TRUTH_ANDIF_EXPR; + else + code = TRUTH_AND_EXPR; lop = 1; break; case INTRINSIC_OR: - code = TRUTH_ORIF_EXPR; + if (flag_frontend_optimize) + code = TRUTH_ORIF_EXPR; + else + code = TRUTH_OR_EXPR; lop = 1; break; As noted already somewhere in the discussion of PR85599 on the mailing list, this breaks actual_pointer_function_1.f90 in the testsuite, which is very similar to comment 0 (and apparently also contributed by Joost). Both are invalid code. The former is fixed by: Index: gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90 === --- gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90 (revision 262859) +++ gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90 (working copy) @@ -17,7 +17,11 @@ CONTAINS logical function cp_logger_log(logger) TYPE(cp_logger_type), POINTER ::logger -cp_logger_log = associated (logger) .and. (logger%a .eq. 42) +if (associated (logger)) then + cp_logger_log = (logger%a .eq. 42) +else + cp_logger_log = .false. +end if END function FUNCTION cp_get_default_logger(v) RESULT(res)
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 --- Comment #8 from Thomas Koenig --- (In reply to Dominique d'Humieres from comment #7) > > It is not actually a complete duplicate of PR 85599, > > it is just similar. > > What is different? Is "different chunks of code are required" good enough? :-) > IMO the two PRs should be closed as WONTFIX. We will discuss it on the mailing list when I submit the patch, I suppose.
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 --- Comment #7 from Dominique d'Humieres --- > It is not actually a complete duplicate of PR 85599, > it is just similar. What is different? IMO the two PRs should be closed as WONTFIX.
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 Thomas Koenig changed: What|Removed |Added Status|RESOLVED|ASSIGNED Resolution|DUPLICATE |--- Assignee|unassigned at gcc dot gnu.org |tkoenig at gcc dot gnu.org --- Comment #6 from Thomas Koenig --- It is not actually a complete duplicate of PR 85599, it is just similar. I have a patch which introduces the ASSOCIATE warning, but not (yet) the division part. The problem there is what we should consider for a warning. if (a != 0 .and. 1/a > 10.) ? Probably. if (a>0 .and. b>0 .and 1/(a+b)) ? Hm. Any suggestions?
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 Dominique d'Humieres changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |DUPLICATE --- Comment #5 from Dominique d'Humieres --- Duplicate of pr85599. *** This bug has been marked as a duplicate of bug 85599 ***
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 Dominique d'Humieres dominiq at lps dot ens.fr changed: What|Removed |Added Status|UNCONFIRMED |WAITING Last reconfirmed||2014-12-21 Ever confirmed|0 |1 --- Comment #3 from Dominique d'Humieres dominiq at lps dot ens.fr --- Is there still some interest for this PR?
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 Joost VandeVondele Joost.VandeVondele at mat dot ethz.ch changed: What|Removed |Added Status|WAITING |NEW CC||Joost.VandeVondele at mat dot ethz ||.ch --- Comment #4 from Joost VandeVondele Joost.VandeVondele at mat dot ethz.ch --- (In reply to Dominique d'Humieres from comment #3) Is there still some interest for this PR? Yes, seems still nice.
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 Thomas Koenig tkoenig at gcc dot gnu.org changed: What|Removed |Added CC||tkoenig at gcc dot gnu.org --- Comment #2 from Thomas Koenig tkoenig at gcc dot gnu.org --- I would really prefer if there was a compile-time warning for IF (ASSOCIATED(m) .AND. m%T) THEN which might be doable. Same thing for if (x /= 0 .and. 2/x 0) then
[Bug fortran/57160] short-circuit IF only with -ffrontend-optimize
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57160 Tobias Burnus burnus at gcc dot gnu.org changed: What|Removed |Added CC||burnus at gcc dot gnu.org --- Comment #1 from Tobias Burnus burnus at gcc dot gnu.org 2013-05-04 19:23:08 UTC --- GCC (the middle end) has TRUTH_AND_EXPR (matching Fortran's .AND.) and TRUTH_ANDIF_EXPR (matching C's ) - besides the IAND/ which is BIT_AND_EXPR. Currently, the code generation directly translates all .AND. into TRUTH_AND_EXPR. Hence, the middle end/target-code generation might decide to evaluate A.AND.B as A andif B, B andif A or both as A and B. That's really outside the scope of the Fortran front end. What you would like is that both A and B are /always/ evaluated with .AND. That's quite some work with little gain. As I know for experience, the current TRUTH_AND_EXPR does no short-circuit evaluation in the given order - I already had segfaults for code similar to your's. * * * As a side note, see http://www.j3-fortran.org/doc/year/13/13-234.txt for a proposal for the next Fortran standard which allows to explicitly require short-circuit evaluation, using: if ( IF (ASSOCIATED(m)) THEN (m%T) ELSE (.false.) ) then write (6,*) s1(m) end if Or even as: write (6,*) IF (ASSOCIATED(m)) THEN ( IF(m%T)then(X)ELSE() ) ELSE ()
