This is the mail archive of the
fortran@gcc.gnu.org
mailing list for the GNU Fortran project.
Re: [Patch, Fortran] PR 57160: short-circuit IF only with -ffrontend-optimize
2018-07-23 23:05 GMT+02:00 Fritz Reese <fritzoreese@gmail.com>:
> On Mon, Jul 23, 2018 at 1:11 PM Janus Weil <janus@gcc.gnu.org> wrote:
>> 2018-07-23 9:40 GMT+02:00 Adam Hirst <adam@aphirst.karoo.co.uk>:
>> > I would err towards -Og here being paired with -O0, but I could see it
>> > being argued both ways - either way, I thought it might at least be
>> > worth making explicit?
>>
>> Well, yes, the current documentation for -ffrontend-optimize is not
>> horribly explicit, but it does say: " Enabled by default by any -O
>> option." Technically that includes -Og, I guess.
>>
>> Phenomenologically, it seems that -Og indeed behaves like -O{1,2,3} in
>> this respect. If one wanted to change that, one would probably do
>> this:
>>
>> Index: gcc/fortran/options.c
>> ===================================================================
>> --- gcc/fortran/options.c (revision 262908)
>> +++ gcc/fortran/options.c (working copy)
>> @@ -417,7 +417,7 @@
>> specified it directly. */
>>
>> if (flag_frontend_optimize == -1)
>> - flag_frontend_optimize = optimize;
>> + flag_frontend_optimize = optimize && !optimize_debug;
>>
>> /* Same for front end loop interchange. */
>>
>>
>> I tend to agree with you that this might be a good idea, but I also
>> don't have a strong opinion here. (Alternatively one could leave
>> -ffrontend-optimize as is, and just couple the short-circuiting
>> behavior to "flag_frontend_optimize && !optimize_debug", but that
>> seems less attractive to me.) Maybe others can comment?
>>
>
> IMO it makes sense to omit frontend optimizations with -Og since one
> probably expects -g/-Og to provide the most faithful reproduction of
> the code (least optimized). I would be OK including this in the patch.
Good, since we all seem to agree on that, I'm including it in the
patch (new version in the attachment).
>> The attached patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>
> I would recommend updating invoke.texi to include a comment regarding
> the effect of -ffrontend-optimize on short-circuiting. If you include
> the above regarding -Og, you should also clarify "Enabled by default
> by any -O option" (e.g. "Enabled by any -O option except -O0 and
> -Og").
Done.
> Normally I like to see testcase(s) enforcing the new behavior as well,
> unless there is a good reason not to. (That way any future changes to
> short-circuiting or -ffrontend-optimize should at least snag on the
> testcase and cause special consideration.)
I somehow thought that the change to actual_pointer_function_1 would
be enough, but of course this does not verify the general behavior wrt
to short-circuiting. I'm attaching two test cases in this direction
now.
> Otherwise looks OK.
Thanks for the review. The new patch also disables the warnings from
PR85599 if -ffrontend-optimize is not given, as noted by Dominique.
The attached is what I'd like to commit (and is regtesting now), but
I'll wait for further comments of course.
Cheers,
Janus
Index: gcc/fortran/invoke.texi
===================================================================
--- gcc/fortran/invoke.texi (revision 262950)
+++ gcc/fortran/invoke.texi (working copy)
@@ -1793,13 +1793,17 @@ if @option{-ffrontend-optimize} is in effect.
@opindex @code{frontend-optimize}
@cindex Front-end optimization
This option performs front-end optimization, based on manipulating
-parts the Fortran parse tree. Enabled by default by any @option{-O}
-option. Optimizations enabled by this option include inlining calls
-to @code{MATMUL}, elimination of identical function calls within
-expressions, removing unnecessary calls to @code{TRIM} in comparisons
-and assignments and replacing @code{TRIM(a)} with
-@code{a(1:LEN_TRIM(a))}. It can be deselected by specifying
-@option{-fno-frontend-optimize}.
+parts the Fortran parse tree. Enabled by default by any @option{-O} option
+except @option{-O0} and @option{-Og}. Optimizations enabled by this option
+include:
+@itemize @bullet
+@item inlining calls to @code{MATMUL},
+@item elimination of identical function calls within expressions,
+@item removing unnecessary calls to @code{TRIM} in comparisons and assignments,
+@item replacing @code{TRIM(a)} with @code{a(1:LEN_TRIM(a))} and
+@item short-circuiting of logical operators (@code{.AND.} and @code{.OR.}).
+@end itemize
+It can be deselected by specifying @option{-fno-frontend-optimize}.
@item -ffrontend-loop-interchange
@opindex @code{frontend-loop-interchange}
Index: gcc/fortran/options.c
===================================================================
--- gcc/fortran/options.c (revision 262950)
+++ gcc/fortran/options.c (working copy)
@@ -417,7 +417,7 @@ gfc_post_options (const char **pfilename)
specified it directly. */
if (flag_frontend_optimize == -1)
- flag_frontend_optimize = optimize;
+ flag_frontend_optimize = optimize && !optimize_debug;
/* Same for front end loop interchange. */
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c (revision 262950)
+++ gcc/fortran/resolve.c (working copy)
@@ -3982,7 +3982,8 @@ resolve_operator (gfc_expr *e)
else if (op2->ts.kind < e->ts.kind)
gfc_convert_type (op2, &e->ts, 2);
- if (e->value.op.op == INTRINSIC_AND || e->value.op.op == INTRINSIC_OR)
+ if (flag_frontend_optimize &&
+ (e->value.op.op == INTRINSIC_AND || e->value.op.op == INTRINSIC_OR))
{
/* Warn about short-circuiting
with impure function as second operand. */
Index: gcc/fortran/trans-expr.c
===================================================================
--- gcc/fortran/trans-expr.c (revision 262950)
+++ gcc/fortran/trans-expr.c (working copy)
@@ -3348,12 +3348,12 @@ gfc_conv_expr_op (gfc_se * se, gfc_expr * expr)
return;
case INTRINSIC_AND:
- code = TRUTH_ANDIF_EXPR;
+ code = flag_frontend_optimize ? TRUTH_ANDIF_EXPR : TRUTH_AND_EXPR;
lop = 1;
break;
case INTRINSIC_OR:
- code = TRUTH_ORIF_EXPR;
+ code = flag_frontend_optimize ? TRUTH_ORIF_EXPR : TRUTH_OR_EXPR;
lop = 1;
break;
Index: gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90
===================================================================
--- gcc/testsuite/gfortran.dg/actual_pointer_function_1.f90 (revision 262950)
+++ 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)
! { dg-do run }
! { dg-options "-O0" }
!
! PR 57160: short-circuit IF only with -ffrontend-optimize
!
! this checks that short-circuiting is not done with -O0
!
! Contributed by Janus Weil <janus@gcc.gnu.org>
program short_circuit
integer, save :: i = 0
logical :: flag
flag = .false.
flag = check() .and. flag
flag = flag .and. check()
if (i /= 2) stop 1
contains
logical function check()
i = i + 1
check = .true.
end function
end
! { dg-do run }
! { dg-options "-O3" }
!
! PR 57160: short-circuit IF only with -ffrontend-optimize
!
! this checks that short-circuiting is done with -O3
!
! Contributed by Janus Weil <janus@gcc.gnu.org>
program short_circuit
integer, save :: i = 0
logical :: flag
flag = .false.
flag = check() .and. flag
flag = flag .and. check()
if (i /= 1) stop 1
contains
logical function check()
i = i + 1
check = .true.
end function
end