| Message ID | CAKwh3qgbSjoVqRS9s-fYB8ODrBXvDV4ePoT-8E02LDn-JXxQ8g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions | expand |
Hi all, here is a minor update of the patch, which cures some problems with implicitly pure functions in the previous version. Most implicitly pure functions were actually detected correctly, but implicitly pure functions that called other implicitly pure functions were not detected properly, and therefore could trigger a warning. This is fixed in the current version, which still regtests cleanly (note that alloc-comp-3.f90 currently fails due to PR 86417). The test case is also enhanced to include the problematic case. Ok for trunk? Cheers, Janus 2018-07-11 23:06 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: > Hi all, > > after the dust of the heated discussion around this PR has settled a > bit, here is another attempt to implement at least some basic warnings > about compiler-dependent behavior concerning the short-circuiting of > logical expressions. > > As a reminder (and recap of the previous discussion), the Fortran > standard unfortunately is a bit sloppy in this area: It allows > compilers to short-circuit the second operand of .AND. / .OR. > operators, but does not require this. As a result, compilers can do > what they want without conflicting with the standard, and they do: > gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR), > ifort does not. > > I'm continuing here the least-invasive approach of keeping gfortran's > current behavior, but warning about cases where compilers may produce > different results. > > The attached patch is very close to the version I posted previously > (which was already approved by Janne), with the difference that the > warnings are now triggered by -Wextra and not -Wsurprising (which is > included in -Wall), as suggested by Nick Maclaren. I think this is > more reasonable, since not everyone may want to see these warnings. > > Note that I don't want to warn about all possible optimizations that > might be allowed by the standard, but only about those that are > actually problematic in practice and result in compiler-dependent > behavior. > > The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk? > > Cheers, > Janus > > > 2018-07-11 Thomas Koenig <tkoenig@gcc.gnu.org> > Janus Weil <janus@gcc.gnu.org> > > PR fortran/85599 > * dump-parse-tree (show_attr): Add handling of implicit_pure. > * resolve.c (impure_function_callback): New function. > (resolve_operator): Call it vial gfc_expr_walker. > > > 2018-07-11 Janus Weil <janus@gcc.gnu.org> > > PR fortran/85599 > * gfortran.dg/short_circuiting.f90: New test. Index: gcc/fortran/dump-parse-tree.c =================================================================== --- gcc/fortran/dump-parse-tree.c (revision 262563) +++ gcc/fortran/dump-parse-tree.c (working copy) @@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo fputs (" ELEMENTAL", dumpfile); if (attr->pure) fputs (" PURE", dumpfile); + if (attr->implicit_pure) + fputs (" IMPLICIT_PURE", dumpfile); if (attr->recursive) fputs (" RECURSIVE", dumpfile); Index: gcc/fortran/resolve.c =================================================================== --- gcc/fortran/resolve.c (revision 262563) +++ gcc/fortran/resolve.c (working copy) @@ -2982,6 +2982,19 @@ pure_function (gfc_expr *e, const char **name) } +static int +implicit_pure_function (gfc_expr *e) +{ + gfc_component *comp = gfc_get_proc_ptr_comp (e); + if (comp) + return gfc_implicit_pure (comp->ts.interface); + else if (e->value.function.esym) + return gfc_implicit_pure (e->value.function.esym); + else + return 0; +} + + static bool impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym, int *f ATTRIBUTE_UNUSED) @@ -3034,7 +3047,8 @@ static bool check_pure_function (gfc_expr *e) "within a PURE procedure", name, &e->where); return false; } - gfc_unset_implicit_pure (NULL); + if (!implicit_pure_function (e)) + gfc_unset_implicit_pure (NULL); } return true; } @@ -3822,6 +3836,46 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop } +/* Callback finding an impure function as an operand to an .and. or + .or. expression. Remember the last function warned about to + avoid double warnings when recursing. */ + +static int +impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + gfc_expr *f = *e; + const char *name; + static gfc_expr *last = NULL; + bool *found = (bool *) data; + + if (f->expr_type == EXPR_FUNCTION) + { + *found = 1; + if (f != last && !pure_function (f, &name)) + { + /* This could still be a function without side effects, i.e. + implicit pure. Do not warn for that case. */ + if (f->symtree == NULL || f->symtree->n.sym == NULL + || !gfc_implicit_pure (f->symtree->n.sym)) + { + if (name) + gfc_warning (OPT_Wextra, + "Function %qs at %L might not be evaluated", + name, &f->where); + else + gfc_warning (OPT_Wextra, + "Function at %L might not be evaluated", + &f->where); + } + } + last = f; + } + + return 0; +} + + /* Resolve an operator expression node. This can involve replacing the operation with a user defined function call. */ @@ -3930,6 +3984,14 @@ resolve_operator (gfc_expr *e) gfc_convert_type (op1, &e->ts, 2); 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) + { + /* Warn about short-circuiting + with impure function as second operand. */ + bool op2_f = false; + gfc_expr_walker (&op2, impure_function_callback, &op2_f); + } break; } ! { dg-do compile } ! { dg-additional-options "-Wextra" } ! ! PR 85599: warn about short-circuiting of logical expressions for non-pure functions ! ! Contributed by Janus Weil <janus@gcc.gnu.org> program short_circuit logical :: flag flag = .false. flag = check() .and. flag flag = flag .and. check() ! { dg-warning "might not be evaluated" } flag = flag .and. pure_check() flag = flag .and. impl_pure_1() flag = flag .and. impl_pure_2() contains logical function check() integer, save :: i = 1 print *, "check", i i = i + 1 check = .true. end function logical pure function pure_check() pure_check = .true. end function logical function impl_pure_1() impl_pure_1 = .true. end function logical function impl_pure_2() impl_pure_2 = impl_pure_1() end function end
Just noticed another problematic case: Calls to generic interfaces are not detected as implicitly pure, see enhanced test case in attachment. Will look into this problem on the weekend ... Cheers, Janus 2018-07-12 21:43 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: > Hi all, > > here is a minor update of the patch, which cures some problems with > implicitly pure functions in the previous version. > > Most implicitly pure functions were actually detected correctly, but > implicitly pure functions that called other implicitly pure functions > were not detected properly, and therefore could trigger a warning. > This is fixed in the current version, which still regtests cleanly > (note that alloc-comp-3.f90 currently fails due to PR 86417). The test > case is also enhanced to include the problematic case. > > Ok for trunk? > > Cheers, > Janus > > > > 2018-07-11 23:06 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: >> Hi all, >> >> after the dust of the heated discussion around this PR has settled a >> bit, here is another attempt to implement at least some basic warnings >> about compiler-dependent behavior concerning the short-circuiting of >> logical expressions. >> >> As a reminder (and recap of the previous discussion), the Fortran >> standard unfortunately is a bit sloppy in this area: It allows >> compilers to short-circuit the second operand of .AND. / .OR. >> operators, but does not require this. As a result, compilers can do >> what they want without conflicting with the standard, and they do: >> gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR), >> ifort does not. >> >> I'm continuing here the least-invasive approach of keeping gfortran's >> current behavior, but warning about cases where compilers may produce >> different results. >> >> The attached patch is very close to the version I posted previously >> (which was already approved by Janne), with the difference that the >> warnings are now triggered by -Wextra and not -Wsurprising (which is >> included in -Wall), as suggested by Nick Maclaren. I think this is >> more reasonable, since not everyone may want to see these warnings. >> >> Note that I don't want to warn about all possible optimizations that >> might be allowed by the standard, but only about those that are >> actually problematic in practice and result in compiler-dependent >> behavior. >> >> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk? >> >> Cheers, >> Janus >> >> >> 2018-07-11 Thomas Koenig <tkoenig@gcc.gnu.org> >> Janus Weil <janus@gcc.gnu.org> >> >> PR fortran/85599 >> * dump-parse-tree (show_attr): Add handling of implicit_pure. >> * resolve.c (impure_function_callback): New function. >> (resolve_operator): Call it vial gfc_expr_walker. >> >> >> 2018-07-11 Janus Weil <janus@gcc.gnu.org> >> >> PR fortran/85599 >> * gfortran.dg/short_circuiting.f90: New test. ! { dg-do compile } ! { dg-additional-options "-Wextra" } ! ! PR 85599: warn about short-circuiting of logical expressions for non-pure functions ! ! Contributed by Janus Weil <janus@gcc.gnu.org> module a interface impl_pure_a module procedure impl_pure_a1 end interface contains logical function impl_pure_a1() impl_pure_a1 = .true. end function end module program short_circuit use a logical :: flag flag = .false. flag = check() .and. flag flag = flag .and. check() ! { dg-warning "might not be evaluated" } flag = flag .and. pure_check() flag = flag .and. impl_pure_1() flag = flag .and. impl_pure_2() flag = flag .and. impl_pure_a1() flag = flag .and. impl_pure_a() ! bogus warning here contains logical function check() integer, save :: i = 1 print *, "check", i i = i + 1 check = .true. end function logical pure function pure_check() pure_check = .true. end function logical function impl_pure_1() impl_pure_1 = .true. end function logical function impl_pure_2() impl_pure_2 = impl_pure_1() end function end
Hi all,
here is another update of the patch. It cures the previously-mentioned
problems with generic interfaces, adds some documentation (as
suggested by Dominique) and slightly enhances the error message
(mentioning the impurity of the function we're warning about).
I tested it on a fairly large code base and found no further false
positives. Also it still regtests cleanly. Ok for trunk?
Cheers,
Janus
2018-07-15 Thomas Koenig <tkoenig@gcc.gnu.org>
Janus Weil <janus@gcc.gnu.org>
PR fortran/85599
* dump-parse-tree.c (show_attr): Add handling of implicit_pure.
* gfortran.texi: Add chapter on evaluation of logical expressions.
* resolve.c (implicit_pure_function): New function.
(check_pure_function): Use it here.
(impure_function_callback): New function.
(resolve_operator): Call it via gfc_expr_walker.
2018-07-15 Janus Weil <janus@gcc.gnu.org>
PR fortran/85599
* gfortran.dg/short_circuiting.f90: New test.
2018-07-13 10:02 GMT+02:00 Janus Weil <janus@gcc.gnu.org>:
> Just noticed another problematic case: Calls to generic interfaces are
> not detected as implicitly pure, see enhanced test case in attachment.
> Will look into this problem on the weekend ...
>
> Cheers,
> Janus
>
> 2018-07-12 21:43 GMT+02:00 Janus Weil <janus@gcc.gnu.org>:
>> Hi all,
>>
>> here is a minor update of the patch, which cures some problems with
>> implicitly pure functions in the previous version.
>>
>> Most implicitly pure functions were actually detected correctly, but
>> implicitly pure functions that called other implicitly pure functions
>> were not detected properly, and therefore could trigger a warning.
>> This is fixed in the current version, which still regtests cleanly
>> (note that alloc-comp-3.f90 currently fails due to PR 86417). The test
>> case is also enhanced to include the problematic case.
>>
>> Ok for trunk?
>>
>> Cheers,
>> Janus
>>
>>
>>
>> 2018-07-11 23:06 GMT+02:00 Janus Weil <janus@gcc.gnu.org>:
>>> Hi all,
>>>
>>> after the dust of the heated discussion around this PR has settled a
>>> bit, here is another attempt to implement at least some basic warnings
>>> about compiler-dependent behavior concerning the short-circuiting of
>>> logical expressions.
>>>
>>> As a reminder (and recap of the previous discussion), the Fortran
>>> standard unfortunately is a bit sloppy in this area: It allows
>>> compilers to short-circuit the second operand of .AND. / .OR.
>>> operators, but does not require this. As a result, compilers can do
>>> what they want without conflicting with the standard, and they do:
>>> gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR),
>>> ifort does not.
>>>
>>> I'm continuing here the least-invasive approach of keeping gfortran's
>>> current behavior, but warning about cases where compilers may produce
>>> different results.
>>>
>>> The attached patch is very close to the version I posted previously
>>> (which was already approved by Janne), with the difference that the
>>> warnings are now triggered by -Wextra and not -Wsurprising (which is
>>> included in -Wall), as suggested by Nick Maclaren. I think this is
>>> more reasonable, since not everyone may want to see these warnings.
>>>
>>> Note that I don't want to warn about all possible optimizations that
>>> might be allowed by the standard, but only about those that are
>>> actually problematic in practice and result in compiler-dependent
>>> behavior.
>>>
>>> The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
>>>
>>> Cheers,
>>> Janus
>>>
>>>
>>> 2018-07-11 Thomas Koenig <tkoenig@gcc.gnu.org>
>>> Janus Weil <janus@gcc.gnu.org>
>>>
>>> PR fortran/85599
>>> * dump-parse-tree (show_attr): Add handling of implicit_pure.
>>> * resolve.c (impure_function_callback): New function.
>>> (resolve_operator): Call it vial gfc_expr_walker.
>>>
>>>
>>> 2018-07-11 Janus Weil <janus@gcc.gnu.org>
>>>
>>> PR fortran/85599
>>> * gfortran.dg/short_circuiting.f90: New test.
Index: gcc/fortran/dump-parse-tree.c
===================================================================
--- gcc/fortran/dump-parse-tree.c (revision 262563)
+++ gcc/fortran/dump-parse-tree.c (working copy)
@@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo
fputs (" ELEMENTAL", dumpfile);
if (attr->pure)
fputs (" PURE", dumpfile);
+ if (attr->implicit_pure)
+ fputs (" IMPLICIT_PURE", dumpfile);
if (attr->recursive)
fputs (" RECURSIVE", dumpfile);
Index: gcc/fortran/gfortran.texi
===================================================================
--- gcc/fortran/gfortran.texi (revision 262563)
+++ gcc/fortran/gfortran.texi (working copy)
@@ -1177,6 +1177,7 @@ might in some way or another become visible to the
@menu
* KIND Type Parameters::
* Internal representation of LOGICAL variables::
+* Evaluation of logical expressions::
* Thread-safety of the runtime library::
* Data consistency and durability::
* Files opened without an explicit ACTION= specifier::
@@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo
See also @ref{Argument passing conventions} and @ref{Interoperability with C}.
+@node Evaluation of logical expressions
+@section Evaluation of logical expressions
+
+The Fortran standard does not require the compiler to evaluate all parts of an
+expression, if they do not contribute to the final result. For logical
+expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU
+Fortran will optimize out function calls (even to impure functions) if the
+result of the expression can be established without them. However, since not
+all compilers do that, and such an optimization can potentially modify the
+program flow and subsequent results, GNU Fortran throws warnings for such
+situations with the @option{-Wextra} flag.
+
+
@node Thread-safety of the runtime library
@section Thread-safety of the runtime library
@cindex thread-safety, threads
Index: gcc/fortran/resolve.c
===================================================================
--- gcc/fortran/resolve.c (revision 262563)
+++ gcc/fortran/resolve.c (working copy)
@@ -2982,6 +2982,21 @@ pure_function (gfc_expr *e, const char **name)
}
+/* Check if the expression is a reference to an implicitly pure function. */
+
+static int
+implicit_pure_function (gfc_expr *e)
+{
+ gfc_component *comp = gfc_get_proc_ptr_comp (e);
+ if (comp)
+ return gfc_implicit_pure (comp->ts.interface);
+ else if (e->value.function.esym)
+ return gfc_implicit_pure (e->value.function.esym);
+ else
+ return 0;
+}
+
+
static bool
impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym,
int *f ATTRIBUTE_UNUSED)
@@ -3034,7 +3049,8 @@ static bool check_pure_function (gfc_expr *e)
"within a PURE procedure", name, &e->where);
return false;
}
- gfc_unset_implicit_pure (NULL);
+ if (!implicit_pure_function (e))
+ gfc_unset_implicit_pure (NULL);
}
return true;
}
@@ -3822,6 +3838,40 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop
}
+/* Callback finding an impure function as an operand to an .and. or
+ .or. expression. Remember the last function warned about to
+ avoid double warnings when recursing. */
+
+static int
+impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED,
+ void *data)
+{
+ gfc_expr *f = *e;
+ const char *name;
+ static gfc_expr *last = NULL;
+ bool *found = (bool *) data;
+
+ if (f->expr_type == EXPR_FUNCTION)
+ {
+ *found = 1;
+ if (f != last && !pure_function (f, &name) && !implicit_pure_function (f))
+ {
+ if (name)
+ gfc_warning (OPT_Wextra,
+ "Impure function %qs at %L might not be evaluated",
+ name, &f->where);
+ else
+ gfc_warning (OPT_Wextra,
+ "Impure function at %L might not be evaluated",
+ &f->where);
+ }
+ last = f;
+ }
+
+ return 0;
+}
+
+
/* Resolve an operator expression node. This can involve replacing the
operation with a user defined function call. */
@@ -3930,6 +3980,14 @@ resolve_operator (gfc_expr *e)
gfc_convert_type (op1, &e->ts, 2);
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)
+ {
+ /* Warn about short-circuiting
+ with impure function as second operand. */
+ bool op2_f = false;
+ gfc_expr_walker (&op2, impure_function_callback, &op2_f);
+ }
break;
}
! { dg-do compile }
! { dg-additional-options "-Wextra" }
!
! PR 85599: warn about short-circuiting of logical expressions for non-pure functions
!
! Contributed by Janus Weil <janus@gcc.gnu.org>
module a
interface impl_pure_a
module procedure impl_pure_a1
end interface
contains
logical function impl_pure_a1()
impl_pure_a1 = .true.
end function
end module
program short_circuit
use a
logical :: flag
flag = .false.
flag = check() .and. flag
flag = flag .and. check() ! { dg-warning "might not be evaluated" }
flag = flag .and. pure_check()
flag = flag .and. impl_pure_1()
flag = flag .and. impl_pure_2()
flag = flag .and. impl_pure_a1()
flag = flag .and. impl_pure_a()
contains
logical function check()
integer, save :: i = 1
print *, "check", i
i = i + 1
check = .true.
end function
logical pure function pure_check()
pure_check = .true.
end function
logical function impl_pure_1()
impl_pure_1 = .true.
end function
logical function impl_pure_2()
impl_pure_2 = impl_pure_1()
end function
end
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
Hi Thomas, >> 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. IIUC you disagree in the sense that you would like gfortran to have more such optimizations (with more corresponding warnings). Is that correct? I hope you can at least agree that the warnings I'm adding in the patch are a) useful and b) sufficient for the current state of optimizations? Modifying gfortran's runtime behavior is really a separate question that we can continue to discuss later, probably with some amount of controversy. But before we even go there, I would really like to have warnings for the optimizations we have now ... > 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. I do see "-faggressive-function-elimination" (https://gcc.gnu.org/onlinedocs/gfortran/Code-Gen-Options.html), but that is about changing runtime behavior, not about throwing warnings. Do you propose to couple the warning to -faggressive-function-elimination (and also the short-circuiting behavior itself)? Or do you propose to add a flag named -Waggressive-function-elimination? Cheers, Janus
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
2018-07-16 21:50 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: > 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. Thanks for the comments. Makes sense. Updated patch attached. I'll wait two more days to allow for further comments and will commit this to trunk on Thursday if I hear no further complaints. Cheers, Janus Index: gcc/fortran/dump-parse-tree.c =================================================================== --- gcc/fortran/dump-parse-tree.c (revision 262563) +++ gcc/fortran/dump-parse-tree.c (working copy) @@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo fputs (" ELEMENTAL", dumpfile); if (attr->pure) fputs (" PURE", dumpfile); + if (attr->implicit_pure) + fputs (" IMPLICIT_PURE", dumpfile); if (attr->recursive) fputs (" RECURSIVE", dumpfile); Index: gcc/fortran/gfortran.texi =================================================================== --- gcc/fortran/gfortran.texi (revision 262563) +++ gcc/fortran/gfortran.texi (working copy) @@ -1177,6 +1177,7 @@ might in some way or another become visible to the @menu * KIND Type Parameters:: * Internal representation of LOGICAL variables:: +* Evaluation of logical expressions:: * Thread-safety of the runtime library:: * Data consistency and durability:: * Files opened without an explicit ACTION= specifier:: @@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo See also @ref{Argument passing conventions} and @ref{Interoperability with C}. +@node Evaluation of logical expressions +@section Evaluation of logical expressions + +The Fortran standard does not require the compiler to evaluate all parts of an +expression, if they do not contribute to the final result. For logical +expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU +Fortran will optimize out function calls (even to impure functions) if the +result of the expression can be established without them. However, since not +all compilers do that, and such an optimization can potentially modify the +program flow and subsequent results, GNU Fortran throws warnings for such +situations with the @option{-Wfunction-elimination} flag. + + @node Thread-safety of the runtime library @section Thread-safety of the runtime library @cindex thread-safety, threads Index: gcc/fortran/invoke.texi =================================================================== --- gcc/fortran/invoke.texi (revision 262563) +++ gcc/fortran/invoke.texi (working copy) @@ -1058,6 +1058,7 @@ off via @option{-Wno-align-commons}. See also @opt @cindex warnings, function elimination 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}. @item -Wrealloc-lhs @opindex @code{Wrealloc-lhs} Index: gcc/fortran/lang.opt =================================================================== --- gcc/fortran/lang.opt (revision 262563) +++ gcc/fortran/lang.opt (working copy) @@ -250,7 +250,7 @@ Fortran Var(flag_warn_frontend_loop_interchange) Warn if loops have been interchanged. Wfunction-elimination -Fortran Warning Var(warn_function_elimination) +Fortran Warning Var(warn_function_elimination) LangEnabledBy(Fortran,Wextra) Warn about function call elimination. Wimplicit-interface Index: gcc/fortran/resolve.c =================================================================== --- gcc/fortran/resolve.c (revision 262563) +++ gcc/fortran/resolve.c (working copy) @@ -2982,6 +2982,21 @@ pure_function (gfc_expr *e, const char **name) } +/* Check if the expression is a reference to an implicitly pure function. */ + +static int +implicit_pure_function (gfc_expr *e) +{ + gfc_component *comp = gfc_get_proc_ptr_comp (e); + if (comp) + return gfc_implicit_pure (comp->ts.interface); + else if (e->value.function.esym) + return gfc_implicit_pure (e->value.function.esym); + else + return 0; +} + + static bool impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym, int *f ATTRIBUTE_UNUSED) @@ -3034,7 +3049,8 @@ static bool check_pure_function (gfc_expr *e) "within a PURE procedure", name, &e->where); return false; } - gfc_unset_implicit_pure (NULL); + if (!implicit_pure_function (e)) + gfc_unset_implicit_pure (NULL); } return true; } @@ -3822,6 +3838,40 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop } +/* Callback finding an impure function as an operand to an .and. or + .or. expression. Remember the last function warned about to + avoid double warnings when recursing. */ + +static int +impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + gfc_expr *f = *e; + const char *name; + static gfc_expr *last = NULL; + bool *found = (bool *) data; + + if (f->expr_type == EXPR_FUNCTION) + { + *found = 1; + if (f != last && !pure_function (f, &name) && !implicit_pure_function (f)) + { + if (name) + gfc_warning (OPT_Wfunction_elimination, + "Impure function %qs at %L might not be evaluated", + name, &f->where); + else + gfc_warning (OPT_Wfunction_elimination, + "Impure function at %L might not be evaluated", + &f->where); + } + last = f; + } + + return 0; +} + + /* Resolve an operator expression node. This can involve replacing the operation with a user defined function call. */ @@ -3930,6 +3980,14 @@ resolve_operator (gfc_expr *e) gfc_convert_type (op1, &e->ts, 2); 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) + { + /* Warn about short-circuiting + with impure function as second operand. */ + bool op2_f = false; + gfc_expr_walker (&op2, impure_function_callback, &op2_f); + } break; }
> 2018-07-16 21:50 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: >> What I would suggest is to enable -Wfunction-eliminiation with >> -Wextra and also use that for your new warning. > > Thanks for the comments. Makes sense. Updated patch attached. Huh, after actually trying -Wfunction-elimination, I'm not so sure any more if it really makes sense to throw the new warnings in there, mainly for two reasons: a) -Wfunction-elimination is slightly different, in that it warns about removing *any* kind of function, not just impure ones. This makes it pretty verbose, and I'm not sure why one would need to know if a pure function call is removed. b) It gives warnings on places that I don't understand. Simple example: subroutine sub(r) implicit none real, intent(in) :: r if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then print *, "rrr" end if end Compiling this with "gfortran-8 -O1 -Wfunction-elimination" gives me: if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then 1 Warning: Removing call to function ‘abs’ at (1) [-Wfunction-elimination] Why can I remove the call to ABS there? If I read the dump correctly, then the two calls to ABS are optimized into a single one, which is used for both comparisons via a temporary. Is that what the warning is trying to tell me? And if yes, why should I care (if the function is pure)? The middle-end certainly does tons of optimizations that rearrange various expressions, without warning me about it ... In other words: Does it make sense to tone down -Wfunction-elimination, by only warning about impure functions? (We certainly have the diagnostic capabilites for this.) If not, I'd rather have a separate flag for the new warnings. -Wfunction-elimination is just too noisy for my taste in its current form. Cheers, Janus
2018-07-17 9:52 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: >> 2018-07-16 21:50 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: >>> What I would suggest is to enable -Wfunction-eliminiation with >>> -Wextra and also use that for your new warning. >> >> Thanks for the comments. Makes sense. Updated patch attached. > > > Huh, after actually trying -Wfunction-elimination, I'm not so sure any > more if it really makes sense to throw the new warnings in there, > mainly for two reasons: > > a) -Wfunction-elimination is slightly different, in that it warns > about removing *any* kind of function, not just impure ones. This > makes it pretty verbose, and I'm not sure why one would need to know > if a pure function call is removed. > b) It gives warnings on places that I don't understand. Simple example: > > subroutine sub(r) > implicit none > real, intent(in) :: r > if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then > print *, "rrr" > end if > end > > Compiling this with "gfortran-8 -O1 -Wfunction-elimination" gives me: > > if ((abs(r) > 1e6) .or. (abs(r) < 1e-6)) then > 1 > Warning: Removing call to function ‘abs’ at (1) [-Wfunction-elimination] > > > Why can I remove the call to ABS there? If I read the dump correctly, > then the two calls to ABS are optimized into a single one, which is > used for both comparisons via a temporary. Is that what the warning is > trying to tell me? And if yes, why should I care (if the function is > pure)? The middle-end certainly does tons of optimizations that > rearrange various expressions, without warning me about it ... > > 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 ... Cheers, Janus Index: gcc/fortran/dump-parse-tree.c =================================================================== --- gcc/fortran/dump-parse-tree.c (revision 262764) +++ gcc/fortran/dump-parse-tree.c (working copy) @@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo fputs (" ELEMENTAL", dumpfile); if (attr->pure) fputs (" PURE", dumpfile); + if (attr->implicit_pure) + fputs (" IMPLICIT_PURE", dumpfile); if (attr->recursive) fputs (" RECURSIVE", dumpfile); Index: gcc/fortran/frontend-passes.c =================================================================== --- gcc/fortran/frontend-passes.c (revision 262764) +++ gcc/fortran/frontend-passes.c (working copy) @@ -840,17 +840,17 @@ create_var (gfc_expr * e, const char *vname) static void do_warn_function_elimination (gfc_expr *e) { - if (e->expr_type != EXPR_FUNCTION) - return; - if (e->value.function.esym) - gfc_warning (OPT_Wfunction_elimination, - "Removing call to function %qs at %L", - e->value.function.esym->name, &(e->where)); - else if (e->value.function.isym) - gfc_warning (OPT_Wfunction_elimination, - "Removing call to function %qs at %L", - e->value.function.isym->name, &(e->where)); + const char *name; + if (e->expr_type == EXPR_FUNCTION && !gfc_pure_function (e, &name) && !gfc_implicit_pure_function (e)) + { + if (name) + gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function %qs at %L", name, &(e->where)); + else + gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function at %L", &(e->where)); + } } + + /* Callback function for the code walker for doing common function elimination. This builds up the list of functions in the expression and goes through them to detect duplicates, which it then replaces Index: gcc/fortran/gfortran.h =================================================================== --- gcc/fortran/gfortran.h (revision 262764) +++ gcc/fortran/gfortran.h (working copy) @@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *) bool gfc_explicit_interface_required (gfc_symbol *, char *, int); extern int gfc_do_concurrent_flag; const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *); +int gfc_pure_function (gfc_expr *e, const char **name); +int gfc_implicit_pure_function (gfc_expr *e); /* array.c */ Index: gcc/fortran/gfortran.texi =================================================================== --- gcc/fortran/gfortran.texi (revision 262764) +++ gcc/fortran/gfortran.texi (working copy) @@ -1177,6 +1177,7 @@ might in some way or another become visible to the @menu * KIND Type Parameters:: * Internal representation of LOGICAL variables:: +* Evaluation of logical expressions:: * Thread-safety of the runtime library:: * Data consistency and durability:: * Files opened without an explicit ACTION= specifier:: @@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo See also @ref{Argument passing conventions} and @ref{Interoperability with C}. +@node Evaluation of logical expressions +@section Evaluation of logical expressions + +The Fortran standard does not require the compiler to evaluate all parts of an +expression, if they do not contribute to the final result. For logical +expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU +Fortran will optimize out function calls (even to impure functions) if the +result of the expression can be established without them. However, since not +all compilers do that, and such an optimization can potentially modify the +program flow and subsequent results, GNU Fortran throws warnings for such +situations with the @option{-Wfunction-elimination} flag. + + @node Thread-safety of the runtime library @section Thread-safety of the runtime library @cindex thread-safety, threads Index: gcc/fortran/invoke.texi =================================================================== --- gcc/fortran/invoke.texi (revision 262764) +++ gcc/fortran/invoke.texi (working copy) @@ -1058,6 +1058,7 @@ off via @option{-Wno-align-commons}. See also @opt @cindex warnings, function elimination 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}. @item -Wrealloc-lhs @opindex @code{Wrealloc-lhs} Index: gcc/fortran/lang.opt =================================================================== --- gcc/fortran/lang.opt (revision 262764) +++ gcc/fortran/lang.opt (working copy) @@ -250,7 +250,7 @@ Fortran Var(flag_warn_frontend_loop_interchange) Warn if loops have been interchanged. Wfunction-elimination -Fortran Warning Var(warn_function_elimination) +Fortran Warning Var(warn_function_elimination) LangEnabledBy(Fortran,Wextra) Warn about function call elimination. Wimplicit-interface Index: gcc/fortran/resolve.c =================================================================== --- gcc/fortran/resolve.c (revision 262764) +++ gcc/fortran/resolve.c (working copy) @@ -2941,8 +2941,8 @@ is_external_proc (gfc_symbol *sym) static int pure_stmt_function (gfc_expr *, gfc_symbol *); -static int -pure_function (gfc_expr *e, const char **name) +int +gfc_pure_function (gfc_expr *e, const char **name) { int pure; gfc_component *comp; @@ -2982,6 +2982,21 @@ pure_stmt_function (gfc_expr *, gfc_symbol *); } +/* Check if the expression is a reference to an implicitly pure function. */ + +int +gfc_implicit_pure_function (gfc_expr *e) +{ + gfc_component *comp = gfc_get_proc_ptr_comp (e); + if (comp) + return gfc_implicit_pure (comp->ts.interface); + else if (e->value.function.esym) + return gfc_implicit_pure (e->value.function.esym); + else + return 0; +} + + static bool impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym, int *f ATTRIBUTE_UNUSED) @@ -2996,7 +3011,7 @@ impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym, || e->symtree->n.sym->attr.proc == PROC_ST_FUNCTION) return false; - return pure_function (e, &name) ? false : true; + return gfc_pure_function (e, &name) ? false : true; } @@ -3012,7 +3027,7 @@ pure_stmt_function (gfc_expr *e, gfc_symbol *sym) static bool check_pure_function (gfc_expr *e) { const char *name = NULL; - if (!pure_function (e, &name) && name) + if (!gfc_pure_function (e, &name) && name) { if (forall_flag) { @@ -3034,7 +3049,8 @@ static bool check_pure_function (gfc_expr *e) "within a PURE procedure", name, &e->where); return false; } - gfc_unset_implicit_pure (NULL); + if (!gfc_implicit_pure_function (e)) + gfc_unset_implicit_pure (NULL); } return true; } @@ -3822,6 +3838,40 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop } +/* Callback finding an impure function as an operand to an .and. or + .or. expression. Remember the last function warned about to + avoid double warnings when recursing. */ + +static int +impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + gfc_expr *f = *e; + const char *name; + static gfc_expr *last = NULL; + bool *found = (bool *) data; + + if (f->expr_type == EXPR_FUNCTION) + { + *found = 1; + if (f != last && !gfc_pure_function (f, &name) && !gfc_implicit_pure_function (f)) + { + if (name) + gfc_warning (OPT_Wfunction_elimination, + "Impure function %qs at %L might not be evaluated", + name, &f->where); + else + gfc_warning (OPT_Wfunction_elimination, + "Impure function at %L might not be evaluated", + &f->where); + } + last = f; + } + + return 0; +} + + /* Resolve an operator expression node. This can involve replacing the operation with a user defined function call. */ @@ -3930,6 +3980,14 @@ resolve_operator (gfc_expr *e) gfc_convert_type (op1, &e->ts, 2); 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) + { + /* Warn about short-circuiting + with impure function as second operand. */ + bool op2_f = false; + gfc_expr_walker (&op2, impure_function_callback, &op2_f); + } break; }
On Tue, Jul 17, 2018 at 10:32 AM Janus Weil <janus@gcc.gnu.org> wrote: > > 2018-07-17 9:52 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: > >> 2018-07-16 21:50 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: > >>> What I would suggest is to enable -Wfunction-eliminiation with > >>> -Wextra and also use that for your new warning. > >> > >> Thanks for the comments. Makes sense. Updated patch attached. > > > > > > Huh, after actually trying -Wfunction-elimination, I'm not so sure any > > more if it really makes sense to throw the new warnings in there, > > mainly for two reasons: [...] > > 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 ... > > Cheers, > Janus Would not this break the testcase function_optimize_5.f90 : write (unit=line, fmt='(4F7.2)') matmul(a,b) & ! { dg-warning "Removing call to function 'matmul'" } & + matmul(a,b) z = sin(x) + 2.0 + sin(x) ! { dg-warning "Removing call to function 'sin'" } print *,z x = ext_func(a) + 23 + ext_func(a) print *,d,x z = element(x) + element(x) ! { dg-warning "Removing call to function 'element'" } print *,z i = mypure(x) - mypure(x) ! { dg-warning "Removing call to function 'mypure'" } 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. Additionally: $ ./contrib/check_GNU_style.sh pr85599_v6.diff Lines should not exceed 80 characters. 33:+ if (e->expr_type == EXPR_FUNCTION && !gfc_pure_function (e, &name) && !gfc_implicit_pure_function (e)) 36:+ gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function %qs at %L", name, &(e->where)); 38:+ gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function at %L", &(e->where)); 201:+ if (f != last && !gfc_pure_function (f, &name) && !gfc_implicit_pure_function (f)) Blocks of 8 spaces should be replaced with tabs. 36:+ gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function %qs at %L", name, &(e->where)); 38:+ gfc_warning (OPT_Wfunction_elimination, "Removing call to impure function at %L", &(e->where)); 230:+ with impure function as second operand. */ Dot, space, space, new sentence. 79:+expression, if they do not contribute to the final result. For logical 82:+result of the expression can be established without them. However, since not Have you considered using levels for the flag, such that the original behavior of -Wfunction-elimination would be retained with e.g. -Wfunction-elimination=2 whereas one could use -Wfunction-elimination=1 (or similar) to omit warnings about impure functions? - Fritz
2018-07-17 17:18 GMT+02:00 Fritz Reese <fritzoreese@gmail.com>: >> 2018-07-17 9:52 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: >> > 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). It that is true, then my modifications practically disable the old -Wfunction-elimination warnings completely :/ > Have you considered using levels for the flag, such that the original > behavior of -Wfunction-elimination would be retained with e.g. > -Wfunction-elimination=2 whereas one could use > -Wfunction-elimination=1 (or similar) to omit warnings about impure > functions? One could do that, but IMHO it would be overkill. I don't see much sense in warning for the elimination of pure functions. But maybe I'm missing something? Cheers, Janus
Am 17.07.2018 um 19:19 schrieb Janus Weil: > 2018-07-17 17:18 GMT+02:00 Fritz Reese <fritzoreese@gmail.com>: >>> 2018-07-17 9:52 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: >>>> 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.
2018-07-17 19:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: > Am 17.07.2018 um 19:19 schrieb Janus Weil: >> 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. Ah, ok. > Problem is that, in all probability, nobody uses this option at the > moment. That's probably true, but it should get a little better once we move it into -Wextra. >> 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. Good, so let's do this. Attached is another update of my patch, which incorporates all of Fritz' comments and should regtest cleanly now. In function_optimize_5.f90 I have removed the warnings for pure functions and instead added -faggressive-function-elimination and the corresponding warnings for impure functions, which were apparently not covered by the testsuite before. I do hope that things have converged by now and that this will be the last incarnation of the patch. If there is no more feedback in the next 24 hours, I'll commit this tomorrow. Cheers, Janus Index: gcc/fortran/dump-parse-tree.c =================================================================== --- gcc/fortran/dump-parse-tree.c (revision 262828) +++ gcc/fortran/dump-parse-tree.c (working copy) @@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo fputs (" ELEMENTAL", dumpfile); if (attr->pure) fputs (" PURE", dumpfile); + if (attr->implicit_pure) + fputs (" IMPLICIT_PURE", dumpfile); if (attr->recursive) fputs (" RECURSIVE", dumpfile); Index: gcc/fortran/frontend-passes.c =================================================================== --- gcc/fortran/frontend-passes.c (revision 262828) +++ gcc/fortran/frontend-passes.c (working copy) @@ -840,17 +840,22 @@ create_var (gfc_expr * e, const char *vname) static void do_warn_function_elimination (gfc_expr *e) { - if (e->expr_type != EXPR_FUNCTION) - return; - if (e->value.function.esym) - gfc_warning (OPT_Wfunction_elimination, - "Removing call to function %qs at %L", - e->value.function.esym->name, &(e->where)); - else if (e->value.function.isym) - gfc_warning (OPT_Wfunction_elimination, - "Removing call to function %qs at %L", - e->value.function.isym->name, &(e->where)); + const char *name; + if (e->expr_type == EXPR_FUNCTION + && !gfc_pure_function (e, &name) && !gfc_implicit_pure_function (e)) + { + if (name) + gfc_warning (OPT_Wfunction_elimination, + "Removing call to impure function %qs at %L", name, + &(e->where)); + else + gfc_warning (OPT_Wfunction_elimination, + "Removing call to impure function at %L", + &(e->where)); + } } + + /* Callback function for the code walker for doing common function elimination. This builds up the list of functions in the expression and goes through them to detect duplicates, which it then replaces Index: gcc/fortran/gfortran.h =================================================================== --- gcc/fortran/gfortran.h (revision 262828) +++ gcc/fortran/gfortran.h (working copy) @@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *) bool gfc_explicit_interface_required (gfc_symbol *, char *, int); extern int gfc_do_concurrent_flag; const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *); +int gfc_pure_function (gfc_expr *e, const char **name); +int gfc_implicit_pure_function (gfc_expr *e); /* array.c */ Index: gcc/fortran/gfortran.texi =================================================================== --- gcc/fortran/gfortran.texi (revision 262828) +++ gcc/fortran/gfortran.texi (working copy) @@ -1177,6 +1177,7 @@ might in some way or another become visible to the @menu * KIND Type Parameters:: * Internal representation of LOGICAL variables:: +* Evaluation of logical expressions:: * Thread-safety of the runtime library:: * Data consistency and durability:: * Files opened without an explicit ACTION= specifier:: @@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo See also @ref{Argument passing conventions} and @ref{Interoperability with C}. +@node Evaluation of logical expressions +@section Evaluation of logical expressions + +The Fortran standard does not require the compiler to evaluate all parts of an +expression, if they do not contribute to the final result. For logical +expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU +Fortran will optimize out function calls (even to impure functions) if the +result of the expression can be established without them. However, since not +all compilers do that, and such an optimization can potentially modify the +program flow and subsequent results, GNU Fortran throws warnings for such +situations with the @option{-Wfunction-elimination} flag. + + @node Thread-safety of the runtime library @section Thread-safety of the runtime library @cindex thread-safety, threads Index: gcc/fortran/invoke.texi =================================================================== --- gcc/fortran/invoke.texi (revision 262828) +++ gcc/fortran/invoke.texi (working copy) @@ -1056,8 +1056,9 @@ off via @option{-Wno-align-commons}. See also @opt @opindex @code{Wfunction-elimination} @cindex function elimination @cindex warnings, function elimination -Warn if any calls to functions are eliminated by the optimizations +Warn if any calls to impure functions are eliminated by the optimizations enabled by the @option{-ffrontend-optimize} option. +This option is implied by @option{-Wextra}. @item -Wrealloc-lhs @opindex @code{Wrealloc-lhs} Index: gcc/fortran/lang.opt =================================================================== --- gcc/fortran/lang.opt (revision 262828) +++ gcc/fortran/lang.opt (working copy) @@ -250,7 +250,7 @@ Fortran Var(flag_warn_frontend_loop_interchange) Warn if loops have been interchanged. Wfunction-elimination -Fortran Warning Var(warn_function_elimination) +Fortran Warning Var(warn_function_elimination) LangEnabledBy(Fortran,Wextra) Warn about function call elimination. Wimplicit-interface Index: gcc/fortran/resolve.c =================================================================== --- gcc/fortran/resolve.c (revision 262828) +++ gcc/fortran/resolve.c (working copy) @@ -2941,8 +2941,8 @@ is_external_proc (gfc_symbol *sym) static int pure_stmt_function (gfc_expr *, gfc_symbol *); -static int -pure_function (gfc_expr *e, const char **name) +int +gfc_pure_function (gfc_expr *e, const char **name) { int pure; gfc_component *comp; @@ -2982,6 +2982,21 @@ pure_stmt_function (gfc_expr *, gfc_symbol *); } +/* Check if the expression is a reference to an implicitly pure function. */ + +int +gfc_implicit_pure_function (gfc_expr *e) +{ + gfc_component *comp = gfc_get_proc_ptr_comp (e); + if (comp) + return gfc_implicit_pure (comp->ts.interface); + else if (e->value.function.esym) + return gfc_implicit_pure (e->value.function.esym); + else + return 0; +} + + static bool impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym, int *f ATTRIBUTE_UNUSED) @@ -2996,7 +3011,7 @@ impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym, || e->symtree->n.sym->attr.proc == PROC_ST_FUNCTION) return false; - return pure_function (e, &name) ? false : true; + return gfc_pure_function (e, &name) ? false : true; } @@ -3012,7 +3027,7 @@ pure_stmt_function (gfc_expr *e, gfc_symbol *sym) static bool check_pure_function (gfc_expr *e) { const char *name = NULL; - if (!pure_function (e, &name) && name) + if (!gfc_pure_function (e, &name) && name) { if (forall_flag) { @@ -3034,7 +3049,8 @@ static bool check_pure_function (gfc_expr *e) "within a PURE procedure", name, &e->where); return false; } - gfc_unset_implicit_pure (NULL); + if (!gfc_implicit_pure_function (e)) + gfc_unset_implicit_pure (NULL); } return true; } @@ -3822,6 +3838,40 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop } +/* Callback finding an impure function as an operand to an .and. or + .or. expression. Remember the last function warned about to + avoid double warnings when recursing. */ + +static int +impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + gfc_expr *f = *e; + const char *name; + static gfc_expr *last = NULL; + bool *found = (bool *) data; + + if (f->expr_type == EXPR_FUNCTION) + { + *found = 1; + if (f != last && !gfc_pure_function (f, &name) && !gfc_implicit_pure_function (f)) + { + if (name) + gfc_warning (OPT_Wfunction_elimination, + "Impure function %qs at %L might not be evaluated", + name, &f->where); + else + gfc_warning (OPT_Wfunction_elimination, + "Impure function at %L might not be evaluated", + &f->where); + } + last = f; + } + + return 0; +} + + /* Resolve an operator expression node. This can involve replacing the operation with a user defined function call. */ @@ -3930,6 +3980,14 @@ resolve_operator (gfc_expr *e) gfc_convert_type (op1, &e->ts, 2); 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) + { + /* Warn about short-circuiting + with impure function as second operand. */ + bool op2_f = false; + gfc_expr_walker (&op2, impure_function_callback, &op2_f); + } break; } Index: gcc/testsuite/gfortran.dg/function_optimize_5.f90 =================================================================== --- gcc/testsuite/gfortran.dg/function_optimize_5.f90 (revision 262828) +++ gcc/testsuite/gfortran.dg/function_optimize_5.f90 (working copy) @@ -1,5 +1,5 @@ ! { dg-do compile } -! { dg-options "-ffrontend-optimize -finline-matmul-limit=0 -Wfunction-elimination" } +! { dg-options "-ffrontend-optimize -faggressive-function-elimination -finline-matmul-limit=0 -Wfunction-elimination" } ! Check the -ffrontend-optimize (in the absence of -O) and ! -Wfunction-elimination options. program main @@ -26,16 +26,16 @@ program main data a /2., 3., 5., 7./ data b /11., 13., 17., 23./ - write (unit=line, fmt='(4F7.2)') matmul(a,b) & ! { dg-warning "Removing call to function 'matmul'" } + write (unit=line, fmt='(4F7.2)') matmul(a,b) & & + matmul(a,b) - z = sin(x) + 2.0 + sin(x) ! { dg-warning "Removing call to function 'sin'" } + z = sin(x) + 2.0 + sin(x) print *,z - x = ext_func(a) + 23 + ext_func(a) + x = ext_func(a) + 23 + ext_func(a) ! { dg-warning "Removing call to impure function" } print *,d,x - z = element(x) + element(x) ! { dg-warning "Removing call to function 'element'" } + z = element(x) + element(x) print *,z - i = mypure(x) - mypure(x) ! { dg-warning "Removing call to function 'mypure'" } + i = mypure(x) - mypure(x) print *,i - z = elem_impure(x) - elem_impure(x) + z = elem_impure(x) - elem_impure(x) ! { dg-warning "Removing call to impure function" } print *,z end program main
On Tue, Jul 17, 2018 at 2:36 PM Janus Weil <janus@gcc.gnu.org> wrote: > > 2018-07-17 19:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: > > Am 17.07.2018 um 19:19 schrieb Janus Weil: [...] > > I do hope that things have converged by now and that this will be the > last incarnation of the patch. If there is no more feedback in the > next 24 hours, I'll commit this tomorrow. > > Cheers, > Janus I hate to be pedantic but it is still worth fixing the style discrepancies: $ ./contrib/check_GNU_style.sh pr85599_v7.diff > > Lines should not exceed 80 characters. > 209:+ if (f != last && !gfc_pure_function (f, &name) && !gfc_implicit_pure_function (f)) > > Blocks of 8 spaces should be replaced with tabs. > 37:+ gfc_warning (OPT_Wfunction_elimination, > 41:+ gfc_warning (OPT_Wfunction_elimination, > 238:+ with impure function as second operand. */ > > Dot, space, space, new sentence. > 84:+expression, if they do not contribute to the final result. For logical > 87:+result of the expression can be established without them. However, since not My only other comment is I am not sure why you make pure_function()/gfc_implicit_pure_function() public... but I have no real problem with it. Just means rebuilding all of f951 instead of one object. :-( Otherwise if the patch does what it appears to do and passes tests then it seems fine to me. Fritz
2018-07-17 20:55 GMT+02:00 Fritz Reese <fritzoreese@gmail.com>: > On Tue, Jul 17, 2018 at 2:36 PM Janus Weil <janus@gcc.gnu.org> wrote: >> >> 2018-07-17 19:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: >> > Am 17.07.2018 um 19:19 schrieb Janus Weil: > [...] >> >> I do hope that things have converged by now and that this will be the >> last incarnation of the patch. If there is no more feedback in the >> next 24 hours, I'll commit this tomorrow. > > I hate to be pedantic but it is still worth fixing the style discrepancies: Oh, sure. Such things are non-optional in GCC, I was just a bit sloppy with this. Thanks for catching! Should be fixed in the attached update. > My only other comment is I am not sure why you make > pure_function()/gfc_implicit_pure_function() public... but I have no > real problem with it. Just means rebuilding all of f951 instead of one > object. :-( Well, originally they were only used in resolve.c, but now I need them also in frontend-passes.c, therefore they have to be public. > Otherwise if the patch does what it appears to do and passes tests > then it seems fine to me. Thanks for the review! Cheers, Janus Index: gcc/fortran/dump-parse-tree.c =================================================================== --- gcc/fortran/dump-parse-tree.c (revision 262828) +++ gcc/fortran/dump-parse-tree.c (working copy) @@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo fputs (" ELEMENTAL", dumpfile); if (attr->pure) fputs (" PURE", dumpfile); + if (attr->implicit_pure) + fputs (" IMPLICIT_PURE", dumpfile); if (attr->recursive) fputs (" RECURSIVE", dumpfile); Index: gcc/fortran/frontend-passes.c =================================================================== --- gcc/fortran/frontend-passes.c (revision 262828) +++ gcc/fortran/frontend-passes.c (working copy) @@ -840,17 +840,22 @@ create_var (gfc_expr * e, const char *vname) static void do_warn_function_elimination (gfc_expr *e) { - if (e->expr_type != EXPR_FUNCTION) - return; - if (e->value.function.esym) - gfc_warning (OPT_Wfunction_elimination, - "Removing call to function %qs at %L", - e->value.function.esym->name, &(e->where)); - else if (e->value.function.isym) - gfc_warning (OPT_Wfunction_elimination, - "Removing call to function %qs at %L", - e->value.function.isym->name, &(e->where)); + const char *name; + if (e->expr_type == EXPR_FUNCTION + && !gfc_pure_function (e, &name) && !gfc_implicit_pure_function (e)) + { + if (name) + gfc_warning (OPT_Wfunction_elimination, + "Removing call to impure function %qs at %L", name, + &(e->where)); + else + gfc_warning (OPT_Wfunction_elimination, + "Removing call to impure function at %L", + &(e->where)); + } } + + /* Callback function for the code walker for doing common function elimination. This builds up the list of functions in the expression and goes through them to detect duplicates, which it then replaces Index: gcc/fortran/gfortran.h =================================================================== --- gcc/fortran/gfortran.h (revision 262828) +++ gcc/fortran/gfortran.h (working copy) @@ -3275,6 +3275,8 @@ bool gfc_resolve_intrinsic (gfc_symbol *, locus *) bool gfc_explicit_interface_required (gfc_symbol *, char *, int); extern int gfc_do_concurrent_flag; const char* gfc_lookup_function_fuzzy (const char *, gfc_symtree *); +int gfc_pure_function (gfc_expr *e, const char **name); +int gfc_implicit_pure_function (gfc_expr *e); /* array.c */ Index: gcc/fortran/gfortran.texi =================================================================== --- gcc/fortran/gfortran.texi (revision 262828) +++ gcc/fortran/gfortran.texi (working copy) @@ -1177,6 +1177,7 @@ might in some way or another become visible to the @menu * KIND Type Parameters:: * Internal representation of LOGICAL variables:: +* Evaluation of logical expressions:: * Thread-safety of the runtime library:: * Data consistency and durability:: * Files opened without an explicit ACTION= specifier:: @@ -1251,6 +1252,19 @@ values: @code{1} for @code{.TRUE.} and @code{0} fo See also @ref{Argument passing conventions} and @ref{Interoperability with C}. +@node Evaluation of logical expressions +@section Evaluation of logical expressions + +The Fortran standard does not require the compiler to evaluate all parts of an +expression, if they do not contribute to the final result. For logical +expressions with @code{.AND.} or @code{.OR.} operators, in particular, GNU +Fortran will optimize out function calls (even to impure functions) if the +result of the expression can be established without them. However, since not +all compilers do that, and such an optimization can potentially modify the +program flow and subsequent results, GNU Fortran throws warnings for such +situations with the @option{-Wfunction-elimination} flag. + + @node Thread-safety of the runtime library @section Thread-safety of the runtime library @cindex thread-safety, threads Index: gcc/fortran/invoke.texi =================================================================== --- gcc/fortran/invoke.texi (revision 262828) +++ gcc/fortran/invoke.texi (working copy) @@ -1056,8 +1056,9 @@ off via @option{-Wno-align-commons}. See also @opt @opindex @code{Wfunction-elimination} @cindex function elimination @cindex warnings, function elimination -Warn if any calls to functions are eliminated by the optimizations +Warn if any calls to impure functions are eliminated by the optimizations enabled by the @option{-ffrontend-optimize} option. +This option is implied by @option{-Wextra}. @item -Wrealloc-lhs @opindex @code{Wrealloc-lhs} Index: gcc/fortran/lang.opt =================================================================== --- gcc/fortran/lang.opt (revision 262828) +++ gcc/fortran/lang.opt (working copy) @@ -250,7 +250,7 @@ Fortran Var(flag_warn_frontend_loop_interchange) Warn if loops have been interchanged. Wfunction-elimination -Fortran Warning Var(warn_function_elimination) +Fortran Warning Var(warn_function_elimination) LangEnabledBy(Fortran,Wextra) Warn about function call elimination. Wimplicit-interface Index: gcc/fortran/resolve.c =================================================================== --- gcc/fortran/resolve.c (revision 262828) +++ gcc/fortran/resolve.c (working copy) @@ -2941,8 +2941,8 @@ is_external_proc (gfc_symbol *sym) static int pure_stmt_function (gfc_expr *, gfc_symbol *); -static int -pure_function (gfc_expr *e, const char **name) +int +gfc_pure_function (gfc_expr *e, const char **name) { int pure; gfc_component *comp; @@ -2982,6 +2982,21 @@ pure_stmt_function (gfc_expr *, gfc_symbol *); } +/* Check if the expression is a reference to an implicitly pure function. */ + +int +gfc_implicit_pure_function (gfc_expr *e) +{ + gfc_component *comp = gfc_get_proc_ptr_comp (e); + if (comp) + return gfc_implicit_pure (comp->ts.interface); + else if (e->value.function.esym) + return gfc_implicit_pure (e->value.function.esym); + else + return 0; +} + + static bool impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym, int *f ATTRIBUTE_UNUSED) @@ -2996,7 +3011,7 @@ impure_stmt_fcn (gfc_expr *e, gfc_symbol *sym, || e->symtree->n.sym->attr.proc == PROC_ST_FUNCTION) return false; - return pure_function (e, &name) ? false : true; + return gfc_pure_function (e, &name) ? false : true; } @@ -3012,7 +3027,7 @@ pure_stmt_function (gfc_expr *e, gfc_symbol *sym) static bool check_pure_function (gfc_expr *e) { const char *name = NULL; - if (!pure_function (e, &name) && name) + if (!gfc_pure_function (e, &name) && name) { if (forall_flag) { @@ -3034,7 +3049,8 @@ static bool check_pure_function (gfc_expr *e) "within a PURE procedure", name, &e->where); return false; } - gfc_unset_implicit_pure (NULL); + if (!gfc_implicit_pure_function (e)) + gfc_unset_implicit_pure (NULL); } return true; } @@ -3822,6 +3838,41 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop } +/* Callback finding an impure function as an operand to an .and. or + .or. expression. Remember the last function warned about to + avoid double warnings when recursing. */ + +static int +impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + gfc_expr *f = *e; + const char *name; + static gfc_expr *last = NULL; + bool *found = (bool *) data; + + if (f->expr_type == EXPR_FUNCTION) + { + *found = 1; + if (f != last && !gfc_pure_function (f, &name) + && !gfc_implicit_pure_function (f)) + { + if (name) + gfc_warning (OPT_Wfunction_elimination, + "Impure function %qs at %L might not be evaluated", + name, &f->where); + else + gfc_warning (OPT_Wfunction_elimination, + "Impure function at %L might not be evaluated", + &f->where); + } + last = f; + } + + return 0; +} + + /* Resolve an operator expression node. This can involve replacing the operation with a user defined function call. */ @@ -3930,6 +3981,14 @@ resolve_operator (gfc_expr *e) gfc_convert_type (op1, &e->ts, 2); 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) + { + /* Warn about short-circuiting + with impure function as second operand. */ + bool op2_f = false; + gfc_expr_walker (&op2, impure_function_callback, &op2_f); + } break; } Index: gcc/testsuite/gfortran.dg/function_optimize_5.f90 =================================================================== --- gcc/testsuite/gfortran.dg/function_optimize_5.f90 (revision 262828) +++ gcc/testsuite/gfortran.dg/function_optimize_5.f90 (working copy) @@ -1,5 +1,5 @@ ! { dg-do compile } -! { dg-options "-ffrontend-optimize -finline-matmul-limit=0 -Wfunction-elimination" } +! { dg-options "-ffrontend-optimize -faggressive-function-elimination -finline-matmul-limit=0 -Wfunction-elimination" } ! Check the -ffrontend-optimize (in the absence of -O) and ! -Wfunction-elimination options. program main @@ -26,16 +26,16 @@ program main data a /2., 3., 5., 7./ data b /11., 13., 17., 23./ - write (unit=line, fmt='(4F7.2)') matmul(a,b) & ! { dg-warning "Removing call to function 'matmul'" } + write (unit=line, fmt='(4F7.2)') matmul(a,b) & & + matmul(a,b) - z = sin(x) + 2.0 + sin(x) ! { dg-warning "Removing call to function 'sin'" } + z = sin(x) + 2.0 + sin(x) print *,z - x = ext_func(a) + 23 + ext_func(a) + x = ext_func(a) + 23 + ext_func(a) ! { dg-warning "Removing call to impure function" } print *,d,x - z = element(x) + element(x) ! { dg-warning "Removing call to function 'element'" } + z = element(x) + element(x) print *,z - i = mypure(x) - mypure(x) ! { dg-warning "Removing call to function 'mypure'" } + i = mypure(x) - mypure(x) print *,i - z = elem_impure(x) - elem_impure(x) + z = elem_impure(x) - elem_impure(x) ! { dg-warning "Removing call to impure function" } print *,z end program main
I have now finally committed this as r262860. Thanks everyone for helping to bring this to a constructive end (and especially to Thomas for the useful input and for putting up with me). Cheers, Janus PS: The next step here would be to tackle PR57160 and avoid the short-circuiting with -O0 (I might actually look into that soon). Once that is done we could even talk about further optimizations (as previously suggested by Thomas), provided they are only done with -O... flags and warned about with appropriate -W... flags. 2018-07-17 21:21 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: > 2018-07-17 20:55 GMT+02:00 Fritz Reese <fritzoreese@gmail.com>: >> On Tue, Jul 17, 2018 at 2:36 PM Janus Weil <janus@gcc.gnu.org> wrote: >>> >>> 2018-07-17 19:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: >>> > Am 17.07.2018 um 19:19 schrieb Janus Weil: >> [...] >>> >>> I do hope that things have converged by now and that this will be the >>> last incarnation of the patch. If there is no more feedback in the >>> next 24 hours, I'll commit this tomorrow. >> >> I hate to be pedantic but it is still worth fixing the style discrepancies: > > Oh, sure. Such things are non-optional in GCC, I was just a bit sloppy > with this. Thanks for catching! Should be fixed in the attached > update. > > >> My only other comment is I am not sure why you make >> pure_function()/gfc_implicit_pure_function() public... but I have no >> real problem with it. Just means rebuilding all of f951 instead of one >> object. :-( > > Well, originally they were only used in resolve.c, but now I need them > also in frontend-passes.c, therefore they have to be public. > > >> Otherwise if the patch does what it appears to do and passes tests >> then it seems fine to me. > > Thanks for the review! > > Cheers, > Janus
Index: gcc/fortran/dump-parse-tree.c =================================================================== --- gcc/fortran/dump-parse-tree.c (revision 262563) +++ gcc/fortran/dump-parse-tree.c (working copy) @@ -716,6 +716,8 @@ show_attr (symbol_attribute *attr, const char * mo fputs (" ELEMENTAL", dumpfile); if (attr->pure) fputs (" PURE", dumpfile); + if (attr->implicit_pure) + fputs (" IMPLICIT_PURE", dumpfile); if (attr->recursive) fputs (" RECURSIVE", dumpfile); Index: gcc/fortran/resolve.c =================================================================== --- gcc/fortran/resolve.c (revision 262563) +++ gcc/fortran/resolve.c (working copy) @@ -3822,6 +3822,46 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop } +/* Callback finding an impure function as an operand to an .and. or + .or. expression. Remember the last function warned about to + avoid double warnings when recursing. */ + +static int +impure_function_callback (gfc_expr **e, int *walk_subtrees ATTRIBUTE_UNUSED, + void *data) +{ + gfc_expr *f = *e; + const char *name; + static gfc_expr *last = NULL; + bool *found = (bool *) data; + + if (f->expr_type == EXPR_FUNCTION) + { + *found = 1; + if (f != last && !pure_function (f, &name)) + { + /* This could still be a function without side effects, i.e. + implicit pure. Do not warn for that case. */ + if (f->symtree == NULL || f->symtree->n.sym == NULL + || !gfc_implicit_pure (f->symtree->n.sym)) + { + if (name) + gfc_warning (OPT_Wextra, + "Function %qs at %L might not be evaluated", + name, &f->where); + else + gfc_warning (OPT_Wextra, + "Function at %L might not be evaluated", + &f->where); + } + } + last = f; + } + + return 0; +} + + /* Resolve an operator expression node. This can involve replacing the operation with a user defined function call. */ @@ -3930,6 +3970,14 @@ resolve_operator (gfc_expr *e) gfc_convert_type (op1, &e->ts, 2); 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) + { + /* Warn about short-circuiting + with impure function as second operand. */ + bool op2_f = false; + gfc_expr_walker (&op2, impure_function_callback, &op2_f); + } break; }
Hi all, after the dust of the heated discussion around this PR has settled a bit, here is another attempt to implement at least some basic warnings about compiler-dependent behavior concerning the short-circuiting of logical expressions. As a reminder (and recap of the previous discussion), the Fortran standard unfortunately is a bit sloppy in this area: It allows compilers to short-circuit the second operand of .AND. / .OR. operators, but does not require this. As a result, compilers can do what they want without conflicting with the standard, and they do: gfortran does short-circuiting (via TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR), ifort does not. I'm continuing here the least-invasive approach of keeping gfortran's current behavior, but warning about cases where compilers may produce different results. The attached patch is very close to the version I posted previously (which was already approved by Janne), with the difference that the warnings are now triggered by -Wextra and not -Wsurprising (which is included in -Wall), as suggested by Nick Maclaren. I think this is more reasonable, since not everyone may want to see these warnings. Note that I don't want to warn about all possible optimizations that might be allowed by the standard, but only about those that are actually problematic in practice and result in compiler-dependent behavior. The patch regtests cleanly on x86_64-linux-gnu. Ok for trunk? Cheers, Janus 2018-07-11 Thomas Koenig <tkoenig@gcc.gnu.org> Janus Weil <janus@gcc.gnu.org> PR fortran/85599 * dump-parse-tree (show_attr): Add handling of implicit_pure. * resolve.c (impure_function_callback): New function. (resolve_operator): Call it vial gfc_expr_walker. 2018-07-11 Janus Weil <janus@gcc.gnu.org> PR fortran/85599 * gfortran.dg/short_circuiting.f90: New test.