| Message ID | e2698932-14bb-e7e8-c9dd-6b59be1e39db@netcologne.de |
|---|---|
| State | New |
| Headers | show |
| Series | [fortran] Handling of .and. and .or. expressions | expand |
Hi Thomas, >the attached patch introduces the following changes: thanks a lot for working on this! >If a logical .and. or .or. expression contains a reference to a >function >which is impure and which also does not behave like a pure function >(i.e. does not have the implicit_pure attribute set), it emits a >warning with -Wsurprising that the function might not be evaluated. >(-Wsurprising is enabled by -Wall). I think you are warning about too many cases here. Warning about an impure function as the *second* operand of the logical operators should be enough, I think. AFAICS there is no danger of the first operand not being evaluated, right? >It special cases the idiom if (associated(m) .and. m%t) which >people appear to use. I don't fully understand why you do this, but in any case it should not be necessary if you warn about the second operand only. >And, if there is an expression like func() .and. flag , it >reverses the test as an optimization. I really don't like this part. It actually introduces more problems of the sort that we're trying to warn about ... Cheers, Janus >2018-06-11 Thomas Koenig <tkoenig@gcc.gnu.org> > > PR fortran/57160 > 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. Special-case > if (associated(m) .and. m%t). If an .and. or .or. expression > has a function or a non-function, exchange the operands. > >2018-06-11 Thomas Koenig <tkoenig@gcc.gnu.org> > > PR fortran/57160 > PR fortran/85599 > * gfortran.dg/logical_evaluation_1.f90: New test. > * gfortran.dg/alloc_comp_default_init_2.f90: Fix code which > implicitly depends on short-circuiting.
Am 13.06.2018 um 14:20 schrieb Janus Weil: > Hi Thomas, > > >> the attached patch introduces the following changes: > > thanks a lot for working on this! > > >> If a logical .and. or .or. expression contains a reference to a >> function >> which is impure and which also does not behave like a pure function >> (i.e. does not have the implicit_pure attribute set), it emits a >> warning with -Wsurprising that the function might not be evaluated. >> (-Wsurprising is enabled by -Wall). > > I think you are warning about too many cases here. Warning about an impure function as the *second* operand of the logical operators should be enough, I think. AFAICS there is no danger of the first operand not being evaluated, right? Not necessarily. The middle end, at the moment, may lack this optimization; at least nobody has so far come up with a test case that demonstrates otherwise. On the other hand, people who know the middle end extremely well have indicated that they expect optimiazations to happen with TRUTH_AND_EXPR, so I am far from certaint that this behavior will continue. > >> It special cases the idiom if (associated(m) .and. m%t) which >> people appear to use. > > I don't fully understand why you do this, but in any case it should not be necessary if you warn about the second operand only. Well, it is an easy mistake to make, and it is (as above) liable to break at any time. Also ASSOCIATED (and ALLOCATED) are pure, so normally I would not warn on these. >> And, if there is an expression like func() .and. flag , it >> reverses the test as an optimization. > > I really don't like this part. It actually introduces more problems of the sort that we're trying to warn about ... If we actually perform this operation, we will have warned about this before (with -Wsurprising). And of course, the compiler can always reverse the order... Regards Thomas
On Mon, Jun 11, 2018 at 09:22:27PM +0200, Thomas Koenig wrote: > > Regression-tested (which found one bug in the testsuite). > > OK for trunk? > I fine with the intent of the patch (see below). PS: I think there may be some confusion on what IMPURE implies. > Index: fortran/resolve.c > =================================================================== > --- fortran/resolve.c (Revision 261388) > +++ fortran/resolve.c (Arbeitskopie) > @@ -3807,7 +3807,43 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop > return gfc_closest_fuzzy_match (op, candidates); > } > > +/* 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; Why not true? *found is declared to be bool. > + if (name) > + gfc_warning (OPT_Wsurprising, "Impure function %qs at %L " > + "might not be evaluated", name, &f->where); > + else > + gfc_warning (OPT_Wsurprising, "Impure function at %L " > + "might not be evaluated", &f->where); I think that this can simply be "Function %qs at %L may not be evaluated" > + /* Some people code which depends on the short-circuiting that > + Fortran does not provide, such as The above seems a little muddled. I suggest a shorter comment. "Some programmers assume that Fortran supports short-circuiting in logical expression, which can lead to surprising behavior. For example, in the following > + > + if (associated(m) .and. m%t) then m%t may be evaluated before associated(m). > + So, warn about this idiom. However, avoid breaking > + it on purpose. */ > + > + if (op1->expr_type == EXPR_FUNCTION && op1->value.function.isym > + && op1->value.function.isym->id == GFC_ISYM_ASSOCIATED) > + { > + gfc_expr *e = op1->value.function.actual->expr; > + gfc_expr *en = op1->value.function.actual->next->expr; > + if (en == NULL && gfc_check_dependency (e, op2, true)) > + { > + gfc_warning (OPT_Wsurprising, "%qs function call at %L does " > + "not guard expression at %L", "ASSOCIATED", > + &op1->where, &op2->where); > + dont_move = true; > + } > + } > + > + /* A bit of optimization: Transfer if (f(x) .and. flag) > + into if (flag .and. f(x)), to save evaluation of a s/transfer/transform I would also put quotes around the Fortran code. > + function. The middle end should be capable of doing > + this with a TRUTH_AND_EXPR, but it currently does not do > + so. See PR 85599. */ The rest looks ok, but I'm not sure if this addresses Janus' concerns.
On Mon, Jun 11, 2018 at 10:22 PM, Thomas Koenig <tkoenig@netcologne.de> wrote: > Hello world, > > the attached patch introduces the following changes: > > If a logical .and. or .or. expression contains a reference to a function > which is impure and which also does not behave like a pure function > (i.e. does not have the implicit_pure attribute set), it emits a > warning with -Wsurprising that the function might not be evaluated. > (-Wsurprising is enabled by -Wall). > > It special cases the idiom if (associated(m) .and. m%t) which > people appear to use. > > And, if there is an expression like func() .and. flag , it > reverses the test as an optimization. The middle end should be > capable of doing this, but apparently it doesn't, so the front > end might as well do this. > What about more complicated expressions like, say, "func() .and. flag .and func2() .and. flag2" etc.? Can it move all the function calls to the end? > > What it does not do is one part of PR 57160, i.e. warn against > if (a /= 0 .and. 1/a > 5) which people who are used to C might > also like to write. > > There is already quite some discussion in the PRs, especially 85599, > where not all people were of the same opinion. Let us see where the > discussion here leads us. > IMHO it's a mistake that the standard refuses to specify the evaluation strategy in the name of some rather minor hypothetical performance benefit. Either a) short-circuiting in left-to-right order or b) must evaluate all the arguments in left-to-right order My preference would be for a) as that is what most languages are doing, but even b) would be better than leaving it undefined. Also, there are AFAIU other similar weirdness with impure functions. The standard allows a compiler to optimize y = f(x) + f(x) into y = 2 * f(x) even if f is impure, which is totally bonkers. Or even not call f at all, if the compiler determines that y is not needed.
Am 14. Juni 2018 10:05:24 MESZ schrieb Janne Blomqvist <blomqvist.janne@gmail.com>: >> There is already quite some discussion in the PRs, especially 85599, >> where not all people were of the same opinion. Let us see where the >> discussion here leads us. > >IMHO it's a mistake that the standard refuses to specify the evaluation >strategy in the name of some rather minor hypothetical performance >benefit. +1 I think it's pretty much the worst strategy the standard can take, and it may even harm optimization in the end. Optimization is always a process that involves both the programmer and the compiler. If the programmer does not know what the compiler can and will do with his code in the end, it's hard to write 'good' code (in the sense that it does what was intended, and with good performance). >Either > >a) short-circuiting in left-to-right order > >or > >b) must evaluate all the arguments in left-to-right order > >My preference would be for a) as that is what most languages are doing, Well, C/C++ has two different kinds of operators that give you the choice to specify whether you want short-circuiting or not. >but >even b) would be better than leaving it undefined. Agreed. In my opinion the best strategy for gfortran in the current situation would be to apply short-circuiting whenever it can be proven that it has no observable consequences. E.g. a function call should not be optimized away unless it is pure. >Also, there are AFAIU other similar weirdness with impure functions. >The >standard allows a compiler to optimize > >y = f(x) + f(x) > >into > >y = 2 * f(x) > >even if f is impure, which is totally bonkers. Or even not call f at >all, >if the compiler determines that y is not needed. Yes, that is the same kind of craziness. I hope gfortran does not actually do this? Cheers, Janus
On Thu, Jun 14, 2018 at 11:38 AM, Janus Weil <jaydub66@gmail.com> wrote: > > Am 14. Juni 2018 10:05:24 MESZ schrieb Janne Blomqvist < > blomqvist.janne@gmail.com>: > > >Either > > > >a) short-circuiting in left-to-right order > > > >or > > > >b) must evaluate all the arguments in left-to-right order > > > >My preference would be for a) as that is what most languages are doing, > > Well, C/C++ has two different kinds of operators that give you the choice > to specify whether you want short-circuiting or not. > Presumably you're thinking of "&" and "|". But to be pedantic, those are bitwise operators, not non-short-circuiting boolean operators, although in some cases they can be used as such. There are however situations where they are not equivalent to hypothetical non-short-circuiting boolean operators! >but > >even b) would be better than leaving it undefined. > > Agreed. > > In my opinion the best strategy for gfortran in the current situation > would be to apply short-circuiting whenever it can be proven that it has no > observable consequences. E.g. a function call should not be optimized away > unless it is pure. > Hmm, why? I don't see why always evaluating everything is less wrong than short-circuiting? I think the best we can do is to print a warning, and try to pressure the committee to choose an evaluation strategy so that this unfortunate situation could eventually be resolved (is Toon still on the committée?). > >Also, there are AFAIU other similar weirdness with impure functions. > >The > >standard allows a compiler to optimize > > > >y = f(x) + f(x) > > > >into > > > >y = 2 * f(x) > > > >even if f is impure, which is totally bonkers. Or even not call f at > >all, > >if the compiler determines that y is not needed. > > Yes, that is the same kind of craziness. I hope gfortran does not actually > do this? > I would hope so too, but I haven't verified.
Am 13. Juni 2018 22:39:38 MESZ schrieb Thomas Koenig <tkoenig@netcologne.de>: >>> If a logical .and. or .or. expression contains a reference to a >>> function >>> which is impure and which also does not behave like a pure function >>> (i.e. does not have the implicit_pure attribute set), it emits a >>> warning with -Wsurprising that the function might not be evaluated. >>> (-Wsurprising is enabled by -Wall). >> >> I think you are warning about too many cases here. Warning about an >impure function as the *second* operand of the logical operators should >be enough, I think. AFAICS there is no danger of the first operand not >being evaluated, right? > >Not necessarily. The middle end, at the moment, may >lack this optimization; at least nobody has so far come >up with a test case that demonstrates otherwise. On the >other hand, people who know the middle end extremely well have >indicated that they expect optimiazations to happen with >TRUTH_AND_EXPR, so I am far from certaint that this behavior >will continue. IIUC, TRUTH_AND_EXPR corresponds to C's && operator, which is inherently assymetric and guaranteed to short-circuit. So I don't think the middle-end would ever switch operands here. I hope your ominous "people who know the middle end extremely well" will correct me if I'm wrong :) >>> It special cases the idiom if (associated(m) .and. m%t) which >>> people appear to use. >> >> I don't fully understand why you do this, but in any case it should >not be necessary if you warn about the second operand only. > >Well, it is an easy mistake to make, and it is (as above) liable >to break at any time. You still haven't stated clearly the rationale for this warning. As I see it, Fortran does not require the compiler to do short-circuiting, but gfortran still does it. So AFAICS there is currently no reason to throw a warning on this case, right? I don't think we need warnings for hypothetical situations. In fact, ifort might have reason to warn about this case, because it does not do short-circuiting. >>> And, if there is an expression like func() .and. flag , it >>> reverses the test as an optimization. >> >> I really don't like this part. It actually introduces more problems >of the sort that we're trying to warn about ... > >If we actually perform this operation, we will have warned about this >before (with -Wsurprising). But we do not perform this operation at present, and I really don't think we should. >And of course, the compiler can always reverse the order... If you're talking about the middle-end here, I don't think it can (see discussion above). Cheers, Janus
On Wed, Jun 13, 2018 at 10:39:38PM +0200, Thomas Koenig wrote: > Am 13.06.2018 um 14:20 schrieb Janus Weil: > > Hi Thomas, > > > > > > > the attached patch introduces the following changes: > > > > thanks a lot for working on this! > > > > > > > If a logical .and. or .or. expression contains a reference to a > > > function > > > which is impure and which also does not behave like a pure function > > > (i.e. does not have the implicit_pure attribute set), it emits a > > > warning with -Wsurprising that the function might not be evaluated. > > > (-Wsurprising is enabled by -Wall). > > > > I think you are warning about too many cases here. Warning about an impure function as the *second* operand of the logical operators should be enough, I think. AFAICS there is no danger of the first operand not being evaluated, right? > > Not necessarily. The middle end, at the moment, may > lack this optimization; at least nobody has so far come > up with a test case that demonstrates otherwise. On the > other hand, people who know the middle end extremely well have > indicated that they expect optimiazations to happen with > TRUTH_AND_EXPR, so I am far from certaint that this behavior > will continue. So what exactly is the purpose of the warning? Teach users that think .and./.or. in Fortran works like C/C++ &&/|| that no, that is not the case, and that it actually works instead like &/| ? In that case it makes sense to warn only about side-effects in the second operand that they will be called even if the first argument is .false. . Or warn users that there is no evaluation ordering between the first and second operand, that both operands are evaluated and it is unspecified which is evaluated first? Wouldn't you then just warn all the time? Even without any impure procedures, you could have l = associated (m) if (l .and. m%t) or similar and m%t could be evaluated before l. I bet gfortran evaluates the side-effects left-to-right and evaluates both arguments always, right? Jakub
Am 14. Juni 2018 11:41:03 MESZ schrieb Janne Blomqvist <blomqvist.janne@gmail.com>: >On Thu, Jun 14, 2018 at 11:38 AM, Janus Weil <jaydub66@gmail.com> >wrote: > >> >> Am 14. Juni 2018 10:05:24 MESZ schrieb Janne Blomqvist < >> blomqvist.janne@gmail.com>: >> >> >Either >> > >> >a) short-circuiting in left-to-right order >> > >> >or >> > >> >b) must evaluate all the arguments in left-to-right order >> > >> >My preference would be for a) as that is what most languages are >doing, >> >> Well, C/C++ has two different kinds of operators that give you the >choice >> to specify whether you want short-circuiting or not. > >Presumably you're thinking of "&" and "|". But to be pedantic, those >are >bitwise operators, not non-short-circuiting boolean operators Yes, but they work perfectly fine as logical operators on booleans, don't they? >There are however situations where >they are not equivalent to hypothetical non-short-circuiting boolean >operators! Could you give a simple example of such a case? >>but >> >even b) would be better than leaving it undefined. >> >> Agreed. >> >> In my opinion the best strategy for gfortran in the current situation >> would be to apply short-circuiting whenever it can be proven that it >has no >> observable consequences. E.g. a function call should not be optimized >away >> unless it is pure. > >Hmm, why? I don't see why always evaluating everything is less wrong >than >short-circuiting? I'm not saying it is "less wrong", but it can be less efficient. In that sense my proposed strategy is a compromise that tries to avoid harmful optimizations but still does optimization where it is safe. Cheers, Janus
On Thu, Jun 14, 2018 at 1:14 PM, Janus Weil <janus@gcc.gnu.org> wrote: > > > Am 14. Juni 2018 11:41:03 MESZ schrieb Janne Blomqvist < > blomqvist.janne@gmail.com>: > >On Thu, Jun 14, 2018 at 11:38 AM, Janus Weil <jaydub66@gmail.com> > >wrote: > > > >> > >> Am 14. Juni 2018 10:05:24 MESZ schrieb Janne Blomqvist < > >> blomqvist.janne@gmail.com>: > >> > >> >Either > >> > > >> >a) short-circuiting in left-to-right order > >> > > >> >or > >> > > >> >b) must evaluate all the arguments in left-to-right order > >> > > >> >My preference would be for a) as that is what most languages are > >doing, > >> > >> Well, C/C++ has two different kinds of operators that give you the > >choice > >> to specify whether you want short-circuiting or not. > > > >Presumably you're thinking of "&" and "|". But to be pedantic, those > >are > >bitwise operators, not non-short-circuiting boolean operators > > Yes, but they work perfectly fine as logical operators on booleans, don't > they? > > > >There are however situations where > >they are not equivalent to hypothetical non-short-circuiting boolean > >operators! > > Could you give a simple example of such a case? > With the usual representation of integers, for A=1, B=2, both A and B by themselves will evaluate as true, A&&B will evaluate to true, but A&B will evaluate to false. >>but > >> >even b) would be better than leaving it undefined. > >> > >> Agreed. > >> > >> In my opinion the best strategy for gfortran in the current situation > >> would be to apply short-circuiting whenever it can be proven that it > >has no > >> observable consequences. E.g. a function call should not be optimized > >away > >> unless it is pure. > > > >Hmm, why? I don't see why always evaluating everything is less wrong > >than > >short-circuiting? > > I'm not saying it is "less wrong", but it can be less efficient. In that > sense my proposed strategy is a compromise that tries to avoid harmful > optimizations but still does optimization where it is safe. > If the user assumes short-circuiting, then evaluating everything is surprising. If the user assumes everything will be evaluated, short-circuiting is surprising. So whatever is done, somebody will think we're doing it wrong. So in lieu of the standards committee getting their act together on this issue, I suggest we do whatever is most efficient, and generate a warning.
On Thu, Jun 14, 2018 at 11:55:45AM +0200, Jakub Jelinek wrote: > > I bet gfortran evaluates the side-effects left-to-right and evaluates both > arguments always, right? > Currently, gfortran evaluates a logical expression left-to-right due to its use of the middle-end's TRUTH_AND_EXPR. The Fortran standard does not require this left-to-right evaluation. Janus found that with '.false. .and. check()' gfortran will not evaluate check(). This is a valid Fortran optimization in that '.false.' and '.false. .and. check()' are equivalent expessions no matter what the return value of check() is. It does not matter if check() is PURE or IMPURE. He then found with the operands reversed that check() is evaluated. He traced this behavior to TRUTH_AND_EXPR, where C allows short-circuiting and it performs left-to-right evaluation. I've already provided a sloppy patch in comment #33 of PR85599 that forces gfortran to evaluate both operands. The patch is equivalent to writing tmp1 = .false. tmp2 = check() if (tmp1 .and. tmp2) ... The patch is sloppy because this forced evaulation should be enabled by -fno-short-circuit (or some such option). Thomas' patch with a warning is simply meant to help programmers who aren't familiar with the Fortran standard. He's trying to use PUREness as a way to surpress the warning. A PURE function by design does not have side-effects so eliminating the evaluation of PURE function should be okay. An IMPURE function or an unmarked function may or may not have side effects. The moral of the story: Don't use functions with side-effects.
Am 14. Juni 2018 12:40:19 MESZ schrieb Janne Blomqvist <blomqvist.janne@gmail.com>: >> >> >Either >> >> > >> >> >a) short-circuiting in left-to-right order >> >> > >> >> >or >> >> > >> >> >b) must evaluate all the arguments in left-to-right order >> >> > >> >> >My preference would be for a) as that is what most languages are >> >doing, >> >> >> >> Well, C/C++ has two different kinds of operators that give you the >> >choice >> >> to specify whether you want short-circuiting or not. >> > >> >Presumably you're thinking of "&" and "|". But to be pedantic, those >> >are >> >bitwise operators, not non-short-circuiting boolean operators >> >> Yes, but they work perfectly fine as logical operators on booleans, >don't >> they? >> >> >> >There are however situations where >> >they are not equivalent to hypothetical non-short-circuiting boolean >> >operators! >> >> Could you give a simple example of such a case? >> > >With the usual representation of integers, for A=1, B=2, both A and B >by >themselves will evaluate as true, A&&B will evaluate to true, but A&B >will >evaluate to false. Yes, sure, with integers you can play such tricks. But as long as you handle only boolean values (in Fortran: .true. and .false.), & and | are proper non-short-circuiting boolean operators. So, to repeat my earlier statement: C/C++ gives you the choice for boolean expressions whether you want short-circuiting or not. No ambiguities, no confusion, no compiler-dependent code. In Fortran, it still feels like functions as such are second-class citizens. People seriously advise against using them. Doesn't really help the attractivity of the language. >>>but >> >> >even b) would be better than leaving it undefined. >> >> >> >> Agreed. >> >> >> >> In my opinion the best strategy for gfortran in the current >situation >> >> would be to apply short-circuiting whenever it can be proven that >it >> >has no >> >> observable consequences. E.g. a function call should not be >optimized >> >away >> >> unless it is pure. >> > >> >Hmm, why? I don't see why always evaluating everything is less wrong >> >than >> >short-circuiting? >> >> I'm not saying it is "less wrong", but it can be less efficient. In >that >> sense my proposed strategy is a compromise that tries to avoid >harmful >> optimizations but still does optimization where it is safe. >> > >If the user assumes short-circuiting, then evaluating everything is >surprising. If the user assumes everything will be evaluated, >short-circuiting is surprising. So whatever is done, somebody will >think >we're doing it wrong. Strongly disagree here. My proposal is to apply short-circuiting only where there are absolutely no surprises. If a user writes my_variable = .true. and my_pure_function() then there is no way at all for him to check if the function was called or not (if the function is pure). And in fact it makes zero difference for the overall program flow and all results that are produced. To not call the function here is an absolutely valid optimization to make, because it does not change any results, in contrast to the case of impure functions. IMHO it is completely insane to optimize out impure functions, just for a little bit of speedup, but sacrificing compiler-independent results. I really don't understand why I'm so alone here with this opinion. Cheers, Janus
Am 15. Juni 2018 11:22:44 MESZ schrieb Janus Weil <janus@gcc.gnu.org>: >>>>but >>> >> >even b) would be better than leaving it undefined. >>> >> >>> >> Agreed. >>> >> >>> >> In my opinion the best strategy for gfortran in the current >>situation >>> >> would be to apply short-circuiting whenever it can be proven that >>it >>> >has no >>> >> observable consequences. E.g. a function call should not be >>optimized >>> >away >>> >> unless it is pure. >>> > >>> >Hmm, why? I don't see why always evaluating everything is less >wrong >>> >than >>> >short-circuiting? >>> >>> I'm not saying it is "less wrong", but it can be less efficient. In >>that >>> sense my proposed strategy is a compromise that tries to avoid >>harmful >>> optimizations but still does optimization where it is safe. >>> >> >>If the user assumes short-circuiting, then evaluating everything is >>surprising. If the user assumes everything will be evaluated, >>short-circuiting is surprising. So whatever is done, somebody will >>think >>we're doing it wrong. > >Strongly disagree here. My proposal is to apply short-circuiting only >where there are absolutely no surprises. If a user writes > >my_variable = .true. and my_pure_function() Sorry, of course this was meant to read: my_variable = .false. .and. my_pure_function() I'm currently travelling with a 5-inch screen only, which somewhat limits my patch-review capabilities ... Cheers, Janus >then there is no way at all for him to check if the function was called >or not (if the function is pure). And in fact it makes zero difference >for the overall program flow and all results that are produced. > >To not call the function here is an absolutely valid optimization to >make, because it does not change any results, in contrast to the case >of impure functions. > >IMHO it is completely insane to optimize out impure functions, just for >a little bit of speedup, but sacrificing compiler-independent results. > >I really don't understand why I'm so alone here with this opinion. > >Cheers, >Janus
Hi Janne, > What about more complicated expressions like, say, "func() .and. flag .and > func2() .and. flag2" etc.? Can it move all the function calls to the end? Not at the moment. Like many of the front-end optimizations, this aims for the easy 80% which are relatively easy to achieve. > IMHO it's a mistake that the standard refuses to specify the evaluation > strategy in the name of some rather minor hypothetical performance benefit. Well, it is the way it is, and it is not going to change. We might as well make the best of the situation as it is. There are other standards, such as the C standard, whose type-based aliasing rules can be used for optimization, but can also introduce hard-to-find bugs. gcc's philosophy is to use the optimization possibilities, and I see no reason for gfortran to deviate from that. Regards Thomas
Am 14.06.2018 um 10:38 schrieb Janus Weil: >> Also, there are AFAIU other similar weirdness with impure functions. >> The >> standard allows a compiler to optimize >> >> y = f(x) + f(x) >> >> into >> >> y = 2 * f(x) >> >> even if f is impure, which is totally bonkers. Or even not call f at >> all, >> if the compiler determines that y is not needed. > Yes, that is the same kind of craziness. I hope gfortran does not actually do this? I would vote for this, but currently it is not done unless -faggressive-function-elimination is specified. By the way, there is a bit of strangeness about this. People use -Ofast knowing it will break all sorts of standards for numercial computation, but they balk at using an optimization that is explicitly permitted by the standard. Oh well...
Am 14.06.2018 um 11:55 schrieb Jakub Jelinek: > Or warn users that there is no evaluation ordering between the first and > second operand, that both operands are evaluated and it is unspecified which > is evaluated first? Wouldn't you then just warn all the time? Even without > any impure procedures, you could have > l = associated (m) > if (l .and. m%t) > or similar and m%t could be evaluated before l. It is not the aim of this patch to catch any and all dubious programming practices. It is the aim of this patch to flag a few well-known errors that compiler writers use. > I bet gfortran evaluates the side-effects left-to-right and evaluates both > arguments always, right? gfortran hands a TRUTH_AND_EXPR to the middle end. You know better what this means than I do.
On Fri, Jun 15, 2018 at 8:06 PM, Thomas Koenig <tkoenig@netcologne.de> wrote: > Hi Janne, > > What about more complicated expressions like, say, "func() .and. flag .and >> func2() .and. flag2" etc.? Can it move all the function calls to the end? >> > > Not at the moment. > > Like many of the front-end optimizations, this aims for the easy 80% > which are relatively easy to achieve. > Come to think about it, is this optimization also done for impure functions? E.g. if func() changes the value of flag, then exchanging the order they are evaluated might change the result. Of course, this is all the fault of the programmer, but still.. But at least for pure functions, this optimization looks Ok. IMHO it's a mistake that the standard refuses to specify the evaluation >> strategy in the name of some rather minor hypothetical performance >> benefit. >> > > Well, it is the way it is, and it is not going to change. We might > as well make the best of the situation as it is. > > There are other standards, such as the C standard, whose type-based > aliasing rules can be used for optimization, but can also introduce > hard-to-find bugs. gcc's philosophy is to use the optimization > possibilities, and I see no reason for gfortran to deviate from that. > I agree, though I hope that the Fortran standard will eventually change to fix this (and other craziness wrt. impure functions as well). But for the time being, I agree we can do this.
On Fri, Jun 15, 2018 at 12:22 PM, Janus Weil <janus@gcc.gnu.org> wrote: > Am 14. Juni 2018 12:40:19 MESZ schrieb Janne Blomqvist < > blomqvist.janne@gmail.com>: > In Fortran, it still feels like functions as such are second-class > citizens. People seriously advise against using them. Doesn't really help > the attractivity of the language. Yes. Back when I followed c.l.f, several experts did advise people to never use functions unless they were pure (or more or less effectively so, if they didn't fulfill the standard requirements for purity). Considering that at least some of those same experts were also part of the Fortran standards committee, I just find it very strange that, to the best of my knowledge, no effort to fix this has been done. IMHO it is completely insane to optimize out impure functions, just for a > little bit of speedup, but sacrificing compiler-independent results. > > I really don't understand why I'm so alone here with this opinion. > I would agree with you if there were some substantial majority opinion among Fortran programmers that all the parts of a logical expression are always evaluated, contrary to what the standard actually guarantees. But as we have seen e.g. in the PR's that this patch attempt to fix, some people actually seem to assume short-circuiting, e.g. by writing code like a /= 0 .and. b/a > c or associated(t) .and. func(t)
On Fri, Jun 15, 2018 at 08:27:49PM +0300, Janne Blomqvist wrote: > On Fri, Jun 15, 2018 at 8:06 PM, Thomas Koenig <tkoenig@netcologne.de> > wrote: > > > > > What about more complicated expressions like, say, "func() .and. flag .and > >> func2() .and. flag2" etc.? Can it move all the function calls to the end? > >> > > > > Not at the moment. > > > > Like many of the front-end optimizations, this aims for the easy 80% > > which are relatively easy to achieve. > > > > Come to think about it, is this optimization also done for impure > functions? E.g. if func() changes the value of flag, then exchanging the > order they are evaluated might change the result. Of course, this is all > the fault of the programmer, but still.. It doesn't matter. The code is invalid. A compiler can do anything. F2018: 10.1.4 Evaluation of operations An intrinsic operation requires the values of its operands. Execution of a function reference in the logical expression in an IF statement (11.1.8.4), the mask expression in a WHERE statement (10.2.3.1), or the concurrent-limits and concurrent-steps in a FORALL statement (10.2.4) is permitted to define variables in the subsidiary action-stmt, where-assignment-stmt, or forall-assignment-stmt respectively. Except in those cases: · the evaluation of a function reference shall neither affect nor be affected by the evaluation of any other entity within the statement; · if a function reference causes definition or undefinition of an actual argument of the function, that argument or any associated entities shall not appear elsewhere in the same statement. > > But at least for pure functions, this optimization looks Ok. > Why is everyone fixated on PURE vs IMPURE functions? The Fortran standard allows short circuiting regardless of the pureness of a function. F2018: 10.1.5.4.2 Evaluation of logical intrinsic operations Once the interpretation of a logical intrinsic operation is established, the processor may evaluate any other expression that is logically equivalent, provided that the integrity of parentheses in any expression is not violated. Two expressions of type logical are logically equivalent if their values are equal for all possible values of their primaries. '.false. .and. func()' and 'func() .and. .false.' are logically equivalent to '.false.' F2018: 10.1.7 Evaluation of operands It is not necessary for a processor to evaluate all of ther operands of an expression, or to evaluate entirely each operand, if the value of the expression can be determined otherwise. In fact, F2018 NOTE 10.28 describes this exact situation. NOTE 10.28 This principle is most often applicable to logical expressions, zero-sized arrays, and zero-length strings, but it applies to all expressions. For example, in evaluating the expression X > Y .OR. L(Z) where X, Y, and Z are real and L is a function of type logical, the function reference L(Z) need not be evaluated if X is greater than Y. There is no statement about L(Z) being PURE or not. Thomas' patch is simply trying to alert users to the fact that their code may not do what they think. His check for pureness versus impureness is an attempt to reduce warnings where the the non-evaluation of pure function cannot change the outcome of program. Finally, I think that he can change the warning from + if (f->symtree == NULL || f->symtree->n.sym == NULL + || !gfc_implicit_pure (f->symtree->n.sym)) + { + if (name) + gfc_warning (OPT_Wsurprising, "Impure function %qs at %L " + "might not be evaluated", name, &f->where); + else + gfc_warning (OPT_Wsurprising, "Impure function at %L " + "might not be evaluated", &f->where); + } to + if (f->symtree == NULL || f->symtree->n.sym == NULL + || !gfc_implicit_pure (f->symtree->n.sym)) + { + if (name) + gfc_warning (OPT_Wsurprising, "Function %qs at %L " + "might not be evaluated", name, &f->where); + else + gfc_warning (OPT_Wsurprising, "Function at %L " + "might not be evaluated", &f->where); + } That is, remove the word 'Impure' as it can be misleading.
Am 15. Juni 2018 19:10:01 MESZ schrieb Thomas Koenig <tkoenig@netcologne.de>: >Am 14.06.2018 um 10:38 schrieb Janus Weil: >>> Also, there are AFAIU other similar weirdness with impure functions. >>> The >>> standard allows a compiler to optimize >>> >>> y = f(x) + f(x) >>> >>> into >>> >>> y = 2 * f(x) >>> >>> even if f is impure, which is totally bonkers. Or even not call f at >>> all, >>> if the compiler determines that y is not needed. >> Yes, that is the same kind of craziness. I hope gfortran does not >actually do this? > >I would vote for this, but currently it is not done unless >-faggressive-function-elimination is specified. How about putting the short-circuiting of logical expressions with impure functions under the same flag? >By the way, there is a bit of strangeness about this. People use -Ofast >knowing it will break all sorts of standards for numercial computation Things are a bit different with -Ofast and -ffast-math. They are not enabled by default, their effects are mentioned in the documentation, and they usually don't change the results totally. Short-circuiting is being done by default, is not properly documented and can potentially have a huge impact on results (an impure function that is optimized away can totally alter the program flow and all numerical results). >but they balk at using an optimization that is explicitly permitted >by the standard. The standard allowing it is really not any consolation, if my results are changed by this 'optimization' and I don't have any guarantee that different compilers do the same thing with my code. That's simply a bad idea, no matter how many official approval stamps it gets. Cheers, Janus
Am 15. Juni 2018 19:49:42 MESZ schrieb Janne Blomqvist <blomqvist.janne@gmail.com>: >On Fri, Jun 15, 2018 at 12:22 PM, Janus Weil <janus@gcc.gnu.org> wrote: > >> Am 14. Juni 2018 12:40:19 MESZ schrieb Janne Blomqvist < >> blomqvist.janne@gmail.com>: >> In Fortran, it still feels like functions as such are second-class >> citizens. People seriously advise against using them. Doesn't really >help >> the attractivity of the language. > > >Yes. Back when I followed c.l.f, several experts did advise people to >never >use functions unless they were pure (or more or less effectively so, if >they didn't fulfill the standard requirements for purity). Considering >that >at least some of those same experts were also part of the Fortran >standards >committee, I just find it very strange that, to the best of my >knowledge, >no effort to fix this has been done. Very strange indeed. If someone tells me "don't use functions in Fortran", then what I hear is "don't use Fortran, it's broken". Why would you introduce a language feature that is not meant to be used? >IMHO it is completely insane to optimize out impure functions, just for >a >> little bit of speedup, but sacrificing compiler-independent results. >> >> I really don't understand why I'm so alone here with this opinion. >> > >I would agree with you if there were some substantial majority opinion >among Fortran programmers that all the parts of a logical expression >are >always evaluated Look, in principle I don't really care how many parts of an expression are evaluated. If the compiler can prove that a certain part of the expression can not change the end result and has no side effects, great, please optimize it away. If the compiler can not prove that, the expression should not be altered. An 'optimization' that changes results is just not useful. It's broken. Cheers, Janus
Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl <sgk@troutmask.apl.washington.edu>: >> But at least for pure functions, this optimization looks Ok. >> > >Why is everyone fixated on PURE vs IMPURE functions? Simply because it makes a difference in this context! That the Fortran standard does not acknowledge this fact is more than surprising to me, given that Fortran actually has a way to mark functions as pure/impure. Not all languages have that. What's the use of a PURE keyword, if not to indicate to the compiler that certain optimizations can safely be done? Cheers, Janus
Am 16.06.2018 um 12:53 schrieb Janus Weil: >> Yes. Back when I followed c.l.f, several experts did advise people to >> never >> use functions unless they were pure (or more or less effectively so, if >> they didn't fulfill the standard requirements for purity). Considering >> that >> at least some of those same experts were also part of the Fortran >> standards >> committee, I just find it very strange that, to the best of my >> knowledge, >> no effort to fix this has been done. That is a bit distorted, at least from what I remember. The advice was rather "If you use functions with side effects, you may have problems". Something quite different. > Very strange indeed. > > If someone tells me "don't use functions in Fortran", then what I hear is "don't use Fortran, it's broken". Well, nobody did, to the best of my knowledge (and I followed the same discussions).
On Sat, Jun 16, 2018 at 01:09:36PM +0200, Janus Weil wrote: > > > Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl <sgk@troutmask.apl.washington.edu>: > >> But at least for pure functions, this optimization looks Ok. > >> > > > >Why is everyone fixated on PURE vs IMPURE functions? > > Simply because it makes a difference in this context! It does not! A function marked as PURE by a programmer must meet certain requirement of the Fortran standard. An unmarked function or a function marked as IMPURE can still meet those same requirements. Marking something as IMPURE has only a single requirement in the standard. An impure elemental procedure processes array arguments in array element order. That's it. Marking a function as IMPURE does not mean the function has side effects. It does not mean that a function must be evaluated for each reference. Are you advocating that gfortran must evaluate ping() twice for impure real function ping() end function ping x = ping() + 0 * ping() end
Am 16. Juni 2018 18:38:40 MESZ schrieb Steve Kargl <sgk@troutmask.apl.washington.edu>: >On Sat, Jun 16, 2018 at 01:09:36PM +0200, Janus Weil wrote: >> >> >> Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl ><sgk@troutmask.apl.washington.edu>: >> >> But at least for pure functions, this optimization looks Ok. >> >> >> > >> >Why is everyone fixated on PURE vs IMPURE functions? >> >> Simply because it makes a difference in this context! > >It does not! A function marked as PURE by a programmer >must meet certain requirement of the Fortran standard. >An unmarked function or a function marked as IMPURE can >still meet those same requirements. Marking something as >IMPURE has only a single requirement in the standard. > > An impure elemental procedure processes array arguments > in array element order. > >That's it. Marking a function as IMPURE does not mean >the function has side effects. It does not mean that >a function must be evaluated for each reference. Are >you advocating that gfortran must evaluate ping() >twice for > > impure real function ping() > end function ping > > x = ping() + 0 * ping() > end Absolutely, sir. That's what I'm advocating. If someone deliberately declares a function as impure, he should be prepared to deal with the consequences. In general, though, one can wonder whether the compiler could not warn that the impure declaration is not necessary (or, in other words, that the function would be better declared as pure). In any case, the example you're showing is probably not the most problematic one. I assume a much more frequent and critical case would be the one where people forget to mark a function that is effectively pure with the PURE attribute. However, IIUC, gfortran has the ability to detect that a function meets all pureness requirements and mark it as implicit_pure, so that it can be treated like a pure procedure, even without any explicit PURE declaration by the programmer. If that's the case, the scheme I proposed should not even have any major performance penalty, since the pureness of a function without explicit pure/impure declaration can be automatically detected, and appropriate optimizations can be chosen. Therefore the most reasonable strategy I can see is still to apply short-circuiting only to pure functions (explicit or implicit) and avoid it for impure ones. Cheers, Janus
On Sat, Jun 16, 2018 at 09:21:16PM +0200, Janus Weil wrote: > > > Am 16. Juni 2018 18:38:40 MESZ schrieb Steve Kargl <sgk@troutmask.apl.washington.edu>: > >On Sat, Jun 16, 2018 at 01:09:36PM +0200, Janus Weil wrote: > >> > >> > >> Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl > ><sgk@troutmask.apl.washington.edu>: > >> >> But at least for pure functions, this optimization looks Ok. > >> >> > >> > > >> >Why is everyone fixated on PURE vs IMPURE functions? > >> > >> Simply because it makes a difference in this context! > > > >It does not! A function marked as PURE by a programmer > >must meet certain requirement of the Fortran standard. > >An unmarked function or a function marked as IMPURE can > >still meet those same requirements. Marking something as > >IMPURE has only a single requirement in the standard. > > > > An impure elemental procedure processes array arguments > > in array element order. > > > >That's it. Marking a function as IMPURE does not mean > >the function has side effects. It does not mean that > >a function must be evaluated for each reference. Are > >you advocating that gfortran must evaluate ping() > >twice for > > > > impure real function ping() > > end function ping > > > > x = ping() + 0 * ping() > > end > > Absolutely, sir. That's what I'm advocating. If someone > deliberately declares a function as impure, he should be > prepared to deal with the consequences. In general, though, > one can wonder whether the compiler could not warn that > the impure declaration is not necessary (or, in other words,i > that the function would be better declared as pure). This is a different answer than what you gave in the PR when I asked if you were special casing .and. and .or. It is trivial to force the evaluation of operands. I already posted the diff in the PR where I special cased .and. and .or. op1 .op. op2 needs to be converted to tmp1 = op1 tmp2 = op2 x = tmp1 .op. tmp for all binary operators. > In any case, the example you're showing is probably not > the most problematic one. I assume a much more frequent > and critical case would be the one where people forget > to mark a function that is effectively pure with the > PURE attribute. See Fortran 2018, Note 10.28 (if I remember correctly). That example explicitly involves .and., and it explicitly states that a function need not be evaluate. It does not sya "pure function". It says "function".
On Sat, Jun 16, 2018 at 12:43:08PM -0700, Steve Kargl wrote: > > This is a different answer than what you gave in > the PR when I asked if you were special casing > .and. and .or. It is trivial to force the evaluation > of operands. I already posted the diff in the PR > where I special cased .and. and .or. > > op1 .op. op2 > > needs to be converted to > > tmp1 = op1 > tmp2 = op2 > x = tmp1 .op. tmp > > for all binary operators. > Untested. Index: trans-expr.c =================================================================== --- trans-expr.c (revision 261674) +++ trans-expr.c (working copy) @@ -3429,6 +3429,10 @@ gfc_conv_expr_op (gfc_se * se, gfc_expr * expr) gfc_conv_expr (&rse, expr->value.op.op2); gfc_add_block_to_block (&se->pre, &rse.pre); + /* Force evaluation of op1 and op2 to prevent shorting-circuiting. */ + lse.expr = gfc_evaluate_now (lse.expr); + rse.expr = gfc_evaluate_noe (rse.expr); + if (checkstring) { gfc_conv_string_parameter (&lse);
On Sat, Jun 16, 2018 at 01:00:14PM -0700, Steve Kargl wrote: > Untested. > > Index: trans-expr.c > =================================================================== > --- trans-expr.c (revision 261674) > +++ trans-expr.c (working copy) > @@ -3429,6 +3429,10 @@ gfc_conv_expr_op (gfc_se * se, gfc_expr * expr) > gfc_conv_expr (&rse, expr->value.op.op2); > gfc_add_block_to_block (&se->pre, &rse.pre); > > + /* Force evaluation of op1 and op2 to prevent shorting-circuiting. */ > + lse.expr = gfc_evaluate_now (lse.expr); > + rse.expr = gfc_evaluate_noe (rse.expr); s/noe/now/ Or just use TRUTH_AND_EXPR/TRUTH_OR_EXPR instead of TRUTH_ANDIF_EXPR/TRUTH_ORIF_EXPR, the difference between those is exactly in short-circuiting. ANDIF/ORIF are for C/C++ &&/||, AND/OR are for C/C++ &/|. Jakub
Am 16. Juni 2018 21:43:08 MESZ schrieb Steve Kargl <sgk@troutmask.apl.washington.edu>: >On Sat, Jun 16, 2018 at 09:21:16PM +0200, Janus Weil wrote: >> >> >> Am 16. Juni 2018 18:38:40 MESZ schrieb Steve Kargl ><sgk@troutmask.apl.washington.edu>: >> >On Sat, Jun 16, 2018 at 01:09:36PM +0200, Janus Weil wrote: >> >> >> >> >> >> Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl >> ><sgk@troutmask.apl.washington.edu>: >> >> >> But at least for pure functions, this optimization looks Ok. >> >> >> >> >> > >> >> >Why is everyone fixated on PURE vs IMPURE functions? >> >> >> >> Simply because it makes a difference in this context! >> > >> >It does not! A function marked as PURE by a programmer >> >must meet certain requirement of the Fortran standard. >> >An unmarked function or a function marked as IMPURE can >> >still meet those same requirements. Marking something as >> >IMPURE has only a single requirement in the standard. >> > >> > An impure elemental procedure processes array arguments >> > in array element order. >> > >> >That's it. Marking a function as IMPURE does not mean >> >the function has side effects. It does not mean that >> >a function must be evaluated for each reference. Are >> >you advocating that gfortran must evaluate ping() >> >twice for >> > >> > impure real function ping() >> > end function ping >> > >> > x = ping() + 0 * ping() >> > end >> >> Absolutely, sir. That's what I'm advocating. If someone >> deliberately declares a function as impure, he should be >> prepared to deal with the consequences. In general, though, >> one can wonder whether the compiler could not warn that >> the impure declaration is not necessary (or, in other words,i >> that the function would be better declared as pure). > >This is a different answer than what you gave in >the PR when I asked if you were special casing >.and. and .or. Possibly we're having a communication issue here. I don't quite understand what you mean by "special casing". >It is trivial to force the evaluation >of operands. I already posted the diff in the PR I know it's rather trivial to do this. Instead of using temporaries, as you propose, one could also use a proper middle-end operator, like BIT_AND_EXPR instead of TRUTH_AND_EXPR. The non-trivial part is obviously to find a consensus about what should be done. >> In any case, the example you're showing is probably not >> the most problematic one. I assume a much more frequent >> and critical case would be the one where people forget >> to mark a function that is effectively pure with the >> PURE attribute. > >See Fortran 2018, Note 10.28 (if I remember correctly). >That example explicitly involves .and., and it explicitly >states that a function need not be evaluate. It does >not sya "pure function". It says "function". You can stop throwing standard quotes. I do know what the standard says about this by now. Point is: It does not sound very reasonable to me and I agree with Janne that the standard's treatment of impure functions is somewhere between careless and insane. If the standard says it's ok to optimize out any kind of function, I would tend to take this less as an encouragement to compiler writers to absolutely do this under any circumstance, but rather as saying: "Those compiler people are smart enough. They can figure out where it makes sense all by themselves." I happen to hold the opinion that optimizing out a call to a pure function may be reasonable if it does not influence the result of an expression, but optimizing out an effectively impure function (i.e. one with side effects) is not a good idea at any time, since such an 'optimization' can drastically change the program flow and all numerical results of a piece of code. (Note the the standard does not require this kind of optimization either, and other popular compilers, like ifort, do not do it.) If we can find some kind of agreement that what I'm proposing is a reasonable thing to do, I will certainly be happy to provide a patch (however I will not be able to get to it before next weekend). Right now I feel like I'm running into various walls. Janne is basically the only one so far who expressed at least partial agreement with some of my notions, so I certainly don't feel very encouraged to even start working on a patch ... Cheers, Janus
Hi Janus, > I happen to hold the opinion that optimizing out a call to a pure > function may be reasonable if it does not influence the result of an > expression, but optimizing out an effectively impure function (i.e. one > with side effects) is not a good idea at any time, since such an > 'optimization' can drastically change the program flow and all numerical > results of a piece of code. Well, I am of a different opinion, and so is the Fortran standard. I think the compiler should strive to, in that order, - conform to the language standard - generate fast programs - warn about features which may trip the user In my patch, I have tried to do all three things at the same time, and after this discussion, I still think that this is the right path to follow. So, here is an update on the patch, which also covers ALLOCATED. Regression-tested. OK? Thomas ! { dg-do compile } ! { dg-additional-options "-Wsurprising -fdump-tree-original" } ! PR 85599 - check warning that impure function calls might be removed, ! and that logical expressions involving .and. and .or. will be ! reordered. MODULE M1 TYPE T1 LOGICAL :: T=.TRUE. END TYPE T1 CONTAINS SUBROUTINE S1(m) TYPE(T1), POINTER :: m TYPE(T1), ALLOCATABLE :: x IF (ASSOCIATED(m) .AND. m%T) THEN ! { dg-warning "does not guard expression" } WRITE(6,*) "X" ENDIF IF (ALLOCATED(x) .AND. x%T) THEN ! { dg-warning "does not guard expression" } WRITE(6,*) "" ENDIF END SUBROUTINE END MODULE module x logical :: flag = .true. integer :: count = 0 contains pure function f() logical :: f f = .true. end function f function g() logical :: g g = .false. end function g real function h() h = 1.2 count = count + 1 end function h end module x program main use x print *, g() .and. f() ! No warning, because g() follows all the rules of a pure function print *, f() .and. flag print *, h() > 1.0 .and. flag ! { dg-warning "might not be evaluated" } print *, h() < 1.0 .or. flag ! { dg-warning "might not be evaluated" } end program main ! { dg-final { scan-tree-dump-times "flag &&" 2 "original" } } ! { dg-final { scan-tree-dump-times "flag \\|\\|" 1 "original" } } Index: dump-parse-tree.c =================================================================== --- dump-parse-tree.c (Revision 261388) +++ dump-parse-tree.c (Arbeitskopie) @@ -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: resolve.c =================================================================== --- resolve.c (Revision 261388) +++ resolve.c (Arbeitskopie) @@ -3807,7 +3807,43 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop return gfc_closest_fuzzy_match (op, candidates); } +/* 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_Wsurprising, "Function %qs at %L " + "might not be evaluated", name, &f->where); + else + gfc_warning (OPT_Wsurprising, "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. */ @@ -3910,6 +3946,8 @@ resolve_operator (gfc_expr *e) case INTRINSIC_NEQV: if (op1->ts.type == BT_LOGICAL && op2->ts.type == BT_LOGICAL) { + bool dont_move = false; + e->ts.type = BT_LOGICAL; e->ts.kind = gfc_kind_max (op1, op2); if (op1->ts.kind < e->ts.kind) @@ -3916,6 +3954,67 @@ 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) + { + bool op1_f, op2_f; + + op1_f = false; + op2_f = false; + gfc_expr_walker (&op1, impure_function_callback, &op1_f); + gfc_expr_walker (&op2, impure_function_callback, &op2_f); + + /* Some people code which depends on the short-circuiting that + Fortran does not provide, such as + + if (associated(m) .and. m%t) then + + So, warn about this idiom. However, avoid breaking + it on purpose. */ + + if (op1->expr_type == EXPR_FUNCTION && op1->value.function.isym) + { + gfc_expr *e; + bool warn = false; + gfc_isym_id id; + + id = op1->value.function.isym->id; + if (id == GFC_ISYM_ASSOCIATED) + { + e = op1->value.function.actual->expr; + warn = op1->value.function.actual->next->expr == NULL; + } + else if (id == GFC_ISYM_ALLOCATED) + { + e = op1->value.function.actual->expr; + warn = true; + } + + if (warn && gfc_check_dependency (e, op2, true)) + { + gfc_warning (OPT_Wsurprising, "%qs function call at " + "%L does not guard expression at %L", + op1->value.function.isym->name, + &op1->where, &op2->where); + dont_move = true; + } + } + + /* A bit of optimization: Transfer if (f(x) .and. flag) + into if (flag .and. f(x)), to save evaluation of a + function. The middle end should be capable of doing + this with a TRUTH_AND_EXPR, but it currently does not do + so. See PR 85599. */ + + if (!dont_move && op1_f && !op2_f) + { + e->value.op.op1 = op2; + e->value.op.op2 = op1; + op1 = e->value.op.op1; + op2 = e->value.op.op2; + } + } + break; }
On Sat, Jun 16, 2018 at 10:42:16PM +0200, Janus Weil wrote: > > Am 16. Juni 2018 21:43:08 MESZ schrieb Steve Kargl <sgk@troutmask.apl.washington.edu>: > >On Sat, Jun 16, 2018 at 09:21:16PM +0200, Janus Weil wrote: > >> > >> > >> Am 16. Juni 2018 18:38:40 MESZ schrieb Steve Kargl > ><sgk@troutmask.apl.washington.edu>: > >> >On Sat, Jun 16, 2018 at 01:09:36PM +0200, Janus Weil wrote: > >> >> > >> >> > >> >> Am 15. Juni 2018 20:38:17 MESZ schrieb Steve Kargl > >> ><sgk@troutmask.apl.washington.edu>: > >> >> >> But at least for pure functions, this optimization looks Ok. > >> >> >> > >> >> > > >> >> >Why is everyone fixated on PURE vs IMPURE functions? > >> >> > >> >> Simply because it makes a difference in this context! > >> > > >> >It does not! A function marked as PURE by a programmer > >> >must meet certain requirement of the Fortran standard. > >> >An unmarked function or a function marked as IMPURE can > >> >still meet those same requirements. Marking something as > >> >IMPURE has only a single requirement in the standard. > >> > > >> > An impure elemental procedure processes array arguments > >> > in array element order. > >> > > >> >That's it. Marking a function as IMPURE does not mean > >> >the function has side effects. It does not mean that > >> >a function must be evaluated for each reference. Are > >> >you advocating that gfortran must evaluate ping() > >> >twice for > >> > > >> > impure real function ping() > >> > end function ping > >> > > >> > x = ping() + 0 * ping() > >> > end > >> > >> Absolutely, sir. That's what I'm advocating. If someone > >> deliberately declares a function as impure, he should be > >> prepared to deal with the consequences. In general, though, > >> one can wonder whether the compiler could not warn that > >> the impure declaration is not necessary (or, in other words,i > >> that the function would be better declared as pure). > > > >This is a different answer than what you gave in > >the PR when I asked if you were special casing > >.and. and .or. > > Possibly we're having a communication issue here. It seems so. > >> In any case, the example you're showing is probably not > >> the most problematic one. I assume a much more frequent > >> and critical case would be the one where people forget > >> to mark a function that is effectively pure with the > >> PURE attribute. > > > >See Fortran 2018, Note 10.28 (if I remember correctly). > >That example explicitly involves .and., and it explicitly > >states that a function need not be evaluate. It does > >not sya "pure function". It says "function". > > You can stop throwing standard quotes. I do know what the > standard says about this by now. > > Point is: It does not sound very reasonable to me and I agree > with Janne that the standard's treatment of impure functions > is somewhere between careless and insane. > > If the standard says it's ok to optimize out any kind of > function, That is exactly what the Fortran standard says! I've quoted the relevant parts of Fortran standard, but you seem relucant to accept it and have ask me to stop supporting my position. For the logical expression, x = .false. .and. check() a Fortran compiler can replace this expression with x = .false. without evaluating check() (regardless of PURE vs IMPURE). That is exactly what Fortran 2018 Note 10.28 says. I won't quote it again.
On Sat, Jun 16, 2018 at 11:14:08PM +0200, Thomas Koenig wrote: > Hi Janus, > > > I happen to hold the opinion that optimizing out a call to a pure > > function may be reasonable if it does not influence the result of an > > expression, but optimizing out an effectively impure function (i.e. one > > with side effects) is not a good idea at any time, since such an > > 'optimization' can drastically change the program flow and all numerical > > results of a piece of code. > > Well, I am of a different opinion, and so is the Fortran standard. > > I think the compiler should strive to, in that order, > > - conform to the language standard > - generate fast programs > - warn about features which may trip the user > > In my patch, I have tried to do all three things at the same time, and > after this discussion, I still think that this is the right path > to follow. > +1
Am 16. Juni 2018 23:14:08 MESZ schrieb Thomas Koenig <tkoenig@netcologne.de>: >Hi Janus, > >> I happen to hold the opinion that optimizing out a call to a pure >> function may be reasonable if it does not influence the result of an >> expression, but optimizing out an effectively impure function (i.e. >one >> with side effects) is not a good idea at any time, since such an >> 'optimization' can drastically change the program flow and all >numerical >> results of a piece of code. > >Well, I am of a different opinion Of course opinions can differ. But could you explain what in my above statements sounds unreasonable to you? Otherwise it's hard to have an argument-based discussion. >and so is the Fortran standard. I don't think anything I wrote is in conflict with the standard. As I see it, the standard neither demands nor forbids short-circuiting. I'm just trying to fill that void in a reasonable way (having as much optimization as possible without altering results). >I think the compiler should strive to, in that order, > >- conform to the language standard >- generate fast programs >- warn about features which may trip the user I certainly agree to all of that, with the amendment that generating correct results should have precedence over generating fast programs. >In my patch, I have tried to do all three things at the same time, and >after this discussion, I still think that this is the right path >to follow. > >So, here is an update on the patch, which also covers ALLOCATED. > >Regression-tested. OK? I absolutely welcome the warnings for impure functions as second operand to .and./.or. operators, but (as noted earlier) I disagree with changing the ordering of operands. As our lengthy discussions show, things are already complicated enough, and this additional optimization just adds to the confusion IMHO. Cheers, Janus
Hi Thomas, hi all, I'm back from holidays and have more time to deal with this now ... 2018-06-17 0:38 GMT+02:00 Janus Weil <janus@gcc.gnu.org>: > > Am 16. Juni 2018 23:14:08 MESZ schrieb Thomas Koenig <tkoenig@netcologne.de>: >>In my patch, I have tried to do all three things at the same time, and >>after this discussion, I still think that this is the right path >>to follow. >> >>So, here is an update on the patch, which also covers ALLOCATED. >> >>Regression-tested. OK? > > I absolutely welcome the warnings for impure functions as second operand to .and./.or. operators, but (as noted earlier) I disagree with changing the ordering of operands. As our lengthy discussions show, things are already complicated enough, and this additional optimization just adds to the confusion IMHO. the previously proposed patch by Thomas did various things at once, not all of which we could agree upon unfortunately. However, there also were some things that everyone seemed to agree upon, namely that there should be warnings for the cases where impure functions are currently short-circuited. The patch in the attachment contains those parts of Thomas' patch that implement that warning, but does no further optimizations or special-casing. Regtests cleanly on x86_64-linux-gnu. Ok for trunk? Cheers, Janus 2018-06-25 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-06-25 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 262024) +++ 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 262024) +++ gcc/fortran/resolve.c (working copy) @@ -3821,6 +3821,44 @@ 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_Wsurprising, "Function %qs at %L " + "might not be evaluated", name, &f->where); + else + gfc_warning (OPT_Wsurprising, "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. */ @@ -3929,6 +3967,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 "-Wsurprising" } ! ! PR 85599: Prevent 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() 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 end
Hi Janus, with your patch, we would only warn about var .and. func() and not about func() .and. var. AFAIK, TRUTH_AND_EXPR does not guarantee that func() will always be exectued, or if it does, I have not seen the documentation; it just happens to match what we currently see (which may be due to missed optimizatins in the back end, or the lack of test cases exposing this). So, I do not think that we should only warn about the first case here. Regards Thomas
On Tue, Jun 26, 2018 at 11:12:40PM +0200, Thomas Koenig wrote: > Hi Janus, > > with your patch, we would only warn about > > var .and. func() > > and not about > > func() .and. var. > > AFAIK, TRUTH_AND_EXPR does not guarantee that func() will > always be exectued, or if it does, I have not seen the > documentation; it just happens to match what we currently > see (which may be due to missed optimizatins in the back end, > or the lack of test cases exposing this). If you are talking about what the middle-end does, TRUTH_AND_EXPR always evaluates side-effects in both operands, while for TRUTH_ANDIF_EXPR it evaluates the second operand only if the first operand is not false. Jakub
2018-06-26 23:16 GMT+02:00 Jakub Jelinek <jakub@redhat.com>: > On Tue, Jun 26, 2018 at 11:12:40PM +0200, Thomas Koenig wrote: >> Hi Janus, >> >> with your patch, we would only warn about >> >> var .and. func() >> >> and not about >> >> func() .and. var. >> >> AFAIK, TRUTH_AND_EXPR does not guarantee that func() will >> always be exectued, or if it does, I have not seen the >> documentation; it just happens to match what we currently >> see (which may be due to missed optimizatins in the back end, >> or the lack of test cases exposing this). > > If you are talking about what the middle-end does, TRUTH_AND_EXPR > always evaluates side-effects in both operands, while for > TRUTH_ANDIF_EXPR it evaluates the second operand only if the first operand > is not false. thanks for the comments. Looking into gfc_conv_expr_op, I see: case INTRINSIC_AND: code = TRUTH_ANDIF_EXPR; lop = 1; break; case INTRINSIC_OR: code = TRUTH_ORIF_EXPR; lop = 1; break; That seems to indicate that Fortran's logical .AND. operator is translated into TRUTH_ANDIF_EXPR, right? Also the empirically observed behavior matches Jakub's description of TRUTH_ANDIF_EXPR, where the first operator is always evaluated and the second one only on demand. Therefore I think my patch is correct in warning about an impure function only if it occurs in the second operand of an and/or operator. Cheers, Janus
On Wed, Jun 27, 2018 at 07:52:59AM +0200, Janus Weil wrote: > >> with your patch, we would only warn about > >> > >> var .and. func() > >> > >> and not about > >> > >> func() .and. var. > >> > >> AFAIK, TRUTH_AND_EXPR does not guarantee that func() will > >> always be exectued, or if it does, I have not seen the > >> documentation; it just happens to match what we currently > >> see (which may be due to missed optimizatins in the back end, > >> or the lack of test cases exposing this). > > > > If you are talking about what the middle-end does, TRUTH_AND_EXPR > > always evaluates side-effects in both operands, while for > > TRUTH_ANDIF_EXPR it evaluates the second operand only if the first operand > > is not false. > > thanks for the comments. Looking into gfc_conv_expr_op, I see: > > case INTRINSIC_AND: > code = TRUTH_ANDIF_EXPR; > lop = 1; > break; > > case INTRINSIC_OR: > code = TRUTH_ORIF_EXPR; > lop = 1; > break; > > That seems to indicate that Fortran's logical .AND. operator is > translated into TRUTH_ANDIF_EXPR, right? Also the empirically observed > behavior matches Jakub's description of TRUTH_ANDIF_EXPR, where the > first operator is always evaluated and the second one only on demand. > Therefore I think my patch is correct in warning about an impure > function only if it occurs in the second operand of an and/or > operator. Is that what we want though? Are there in Fortran any ways to have side-effects in expressions other than function calls? E.g. VOLATILE variable reads? Is it ok not to evaluate those on .and. or .or. expressions? I think best would be to change the above to code = TRUTH_AND_EXPR and code = TRUTH_OR_EXPR and have some non-default aggressive optimization option that would optimize away in the FE impure function calls from those operands if one operand is cheap to evaluate and another one contains function calls, by optimizing it into TRUTH_{AND,OR}IF_EXPR where the cheap operand is the first operand and operand with function calls is the second. Jakub
2018-06-27 8:15 GMT+02:00 Jakub Jelinek <jakub@redhat.com>: > On Wed, Jun 27, 2018 at 07:52:59AM +0200, Janus Weil wrote: >> >> with your patch, we would only warn about >> >> >> >> var .and. func() >> >> >> >> and not about >> >> >> >> func() .and. var. >> >> >> >> AFAIK, TRUTH_AND_EXPR does not guarantee that func() will >> >> always be exectued, or if it does, I have not seen the >> >> documentation; it just happens to match what we currently >> >> see (which may be due to missed optimizatins in the back end, >> >> or the lack of test cases exposing this). >> > >> > If you are talking about what the middle-end does, TRUTH_AND_EXPR >> > always evaluates side-effects in both operands, while for >> > TRUTH_ANDIF_EXPR it evaluates the second operand only if the first operand >> > is not false. >> >> thanks for the comments. Looking into gfc_conv_expr_op, I see: >> >> case INTRINSIC_AND: >> code = TRUTH_ANDIF_EXPR; >> lop = 1; >> break; >> >> case INTRINSIC_OR: >> code = TRUTH_ORIF_EXPR; >> lop = 1; >> break; >> >> That seems to indicate that Fortran's logical .AND. operator is >> translated into TRUTH_ANDIF_EXPR, right? Also the empirically observed >> behavior matches Jakub's description of TRUTH_ANDIF_EXPR, where the >> first operator is always evaluated and the second one only on demand. >> Therefore I think my patch is correct in warning about an impure >> function only if it occurs in the second operand of an and/or >> operator. > > Is that what we want though? Probably depends a bit on who "we" is ;) See the discussion upthread and in PR85599. > Are there in Fortran any ways to have > side-effects in expressions other than function calls? E.g. > VOLATILE variable reads? Don't think so. > Is it ok not to evaluate those on .and. or .or. > expressions? I think best would be to change the above to > code = TRUTH_AND_EXPR and code = TRUTH_OR_EXPR I don't think it's ok to not evaluate expressions that have side effects, and I would highly welcome a change to TRUTH_{AND,OR}_EXPR, but there was quite some opposition to that position. The Fortran standard unfortunately is somewhat agnostic of that matter (and leaves quite some freedom to the compiler), which essentially is the origin of all this discussion. > and have some non-default aggressive > optimization option that would optimize away in the FE impure function calls IMHO an optimization to remove functions calls is unproblematic only for pure functions, but as long as it is guarded by a non-default optimization flag, I'm perfectly happy with optimizing away stuff with side effects as well. Cheers, Janus
On Wed, Jun 27, 2018 at 09:35:59AM +0200, Janus Weil wrote: > > and have some non-default aggressive > > optimization option that would optimize away in the FE impure function calls > > IMHO an optimization to remove functions calls is unproblematic only > for pure functions, but as long as it is guarded by a non-default For pure functions, which are hopefully marked as ECF_PURE for the middle-end the middle-end can already optimize away those calls where the result isn't needed. > optimization flag, I'm perfectly happy with optimizing away stuff with > side effects as well. Jakub
2018-06-27 9:42 GMT+02:00 Jakub Jelinek <jakub@redhat.com>: > On Wed, Jun 27, 2018 at 09:35:59AM +0200, Janus Weil wrote: >> > and have some non-default aggressive >> > optimization option that would optimize away in the FE impure function calls >> >> IMHO an optimization to remove functions calls is unproblematic only >> for pure functions, but as long as it is guarded by a non-default > > For pure functions, which are hopefully marked as ECF_PURE for the > middle-end Not sure if that's the case. Grepping for ECF_PURE in trunk/gcc/fortran only yields a single occurrence: f95-lang.c:#define ATTR_PURE_NOTHROW_LEAF_LIST (ECF_NOTHROW | ECF_LEAF | ECF_PURE) Seems like that is only used for built-ins? > the middle-end can already optimize away those calls where the > result isn't needed. Ah, great. Even with TRUTH_AND_EXPR, you mean? Cheers, Janus
On Wed, Jun 27, 2018 at 09:52:04AM +0200, Janus Weil wrote: > 2018-06-27 9:42 GMT+02:00 Jakub Jelinek <jakub@redhat.com>: > > On Wed, Jun 27, 2018 at 09:35:59AM +0200, Janus Weil wrote: > >> > and have some non-default aggressive > >> > optimization option that would optimize away in the FE impure function calls > >> > >> IMHO an optimization to remove functions calls is unproblematic only > >> for pure functions, but as long as it is guarded by a non-default > > > > For pure functions, which are hopefully marked as ECF_PURE for the > > middle-end > > Not sure if that's the case. Grepping for ECF_PURE in > trunk/gcc/fortran only yields a single occurrence: > > f95-lang.c:#define ATTR_PURE_NOTHROW_LEAF_LIST (ECF_NOTHROW | > ECF_LEAF | ECF_PURE) > > Seems like that is only used for built-ins? No, fortran/trans-decl.c has: /* Set attributes for PURE functions. A call to PURE function in the Fortran 95 sense is both pure and without side effects in the C sense. */ if (sym->attr.pure || sym->attr.implicit_pure) { if (sym->attr.function && !gfc_return_by_reference (sym)) DECL_PURE_P (fndecl) = 1; and calls.c has: if (DECL_PURE_P (exp)) flags |= ECF_PURE; > Ah, great. Even with TRUTH_AND_EXPR, you mean? If one operand of TRUTH_AND_EXPR is optimized into false, then yes, it will fold into false and thus the function result will be dead and the call will be removed. Though, if you mean an optimization that checks if one operand can be computed cheaply and the other one is expensive with function calls and optimize expensive & cheap into cheap && expensive, then no, we don't have such an optimization (perhaps it would be worth it, but would probably need to be done early (in the FEs or during gimplification at latest)). Jakub
On Wed, Jun 27, 2018 at 10:52 AM, Janus Weil <janus@gcc.gnu.org> wrote: > 2018-06-27 9:42 GMT+02:00 Jakub Jelinek <jakub@redhat.com>: > > On Wed, Jun 27, 2018 at 09:35:59AM +0200, Janus Weil wrote: > >> > and have some non-default aggressive > >> > optimization option that would optimize away in the FE impure > function calls > >> > >> IMHO an optimization to remove functions calls is unproblematic only > >> for pure functions, but as long as it is guarded by a non-default > > > > For pure functions, which are hopefully marked as ECF_PURE for the > > middle-end > > Not sure if that's the case. Grepping for ECF_PURE in > trunk/gcc/fortran only yields a single occurrence: > > f95-lang.c:#define ATTR_PURE_NOTHROW_LEAF_LIST (ECF_NOTHROW | > ECF_LEAF | ECF_PURE) > > Seems like that is only used for built-ins? > > > > the middle-end can already optimize away those calls where the > > result isn't needed. > > Ah, great. Even with TRUTH_AND_EXPR, you mean? > How about committing a patch changing to use TRUTH_{AND|OR}_EXPR, and then check benchmark results (polyhedron, spec etc.)? If performance worsens, we revert, if it improves, great, lets keep it? To clarify, I'm not objecting to using TRUTH_{AND|OR}_EXPR, I'm objecting to the notion that this is somehow "more correct" or "less bad" than the current short-circuiting. The Fortran standard does not specify an evaluation strategy; IMHO very unfortunately, but it is what it is.
2018-06-27 10:09 GMT+02:00 Janne Blomqvist <blomqvist.janne@gmail.com>: > On Wed, Jun 27, 2018 at 10:52 AM, Janus Weil <janus@gcc.gnu.org> wrote: >> >> 2018-06-27 9:42 GMT+02:00 Jakub Jelinek <jakub@redhat.com>: >> > On Wed, Jun 27, 2018 at 09:35:59AM +0200, Janus Weil wrote: >> >> > and have some non-default aggressive >> >> > optimization option that would optimize away in the FE impure >> >> > function calls >> >> >> >> IMHO an optimization to remove functions calls is unproblematic only >> >> for pure functions, but as long as it is guarded by a non-default >> > >> > For pure functions, which are hopefully marked as ECF_PURE for the >> > middle-end >> >> Not sure if that's the case. Grepping for ECF_PURE in >> trunk/gcc/fortran only yields a single occurrence: >> >> f95-lang.c:#define ATTR_PURE_NOTHROW_LEAF_LIST (ECF_NOTHROW | >> ECF_LEAF | ECF_PURE) >> >> Seems like that is only used for built-ins? >> >> >> > the middle-end can already optimize away those calls where the >> > result isn't needed. >> >> Ah, great. Even with TRUTH_AND_EXPR, you mean? > > How about committing a patch changing to use TRUTH_{AND|OR}_EXPR, and then > check benchmark results (polyhedron, spec etc.)? If performance worsens, we > revert, if it improves, great, lets keep it? I would definitely support that. I cannot imagine that it will have a large impact on benchmarks, but it's certainly a good idea to check. (After all: ifort, which is usually perceived as being slightly ahead of GCC in terms of performance, does not do this kind of short-circuiting either.) If the middle-end already does all 'non-problematic' optimizations by itself, all the better. > To clarify, I'm not objecting to using TRUTH_{AND|OR}_EXPR, I'm objecting to > the notion that this is somehow "more correct" or "less bad" than the > current short-circuiting. The Fortran standard does not specify an > evaluation strategy; IMHO very unfortunately, but it is what it is. It is very unfortunate, and it means that the Fortran standard simply does not provide a measure for "more correct" here. (My common-sense drop-in notion of correctness would be that an optimization is 'correct' as long as it can be proven to not change any results.) Cheers, Janus
On Jun 27 2018, Janus Weil wrote: > >It is very unfortunate, and it means that the Fortran standard simply >does not provide a measure for "more correct" here. (My common-sense >drop-in notion of correctness would be that an optimization is >'correct' as long as it can be proven to not change any results.) Actually, yes, it does. Fortran 2008 7.1.4p2, 7.1.5.2.4p2, 7.1.5.3.2p1, 7.1.5.4.2p1, 7.1.5.5.2p1 and 7.1.6.3p1/2. The problem is that the Fortran standard has never resolved the inconsistency that OTHER side-effects in functions are not explicitly forbidden - it's one of the few places in Fortran where there are conforming constructs that do not have specified semantics or at least a clearly specified intention. Many, many people have tried to sort that out, and all have failed dismally. I regret that the proposal to deprecate impure functions did not succeed, because at least that would have made one sane direction clear. This has been made much worse by IEEE 754 and (worse) coarray support. Like almost all languages with threading and shared-memory, Fortran's concept of pure is satisfactory for single-threaded code but not parallel. You need the stronger concept, where a pure function does not depend on anything that might change in its scope. Look on the bright side. If you thought this problem is a mess (it is), I can assure you that most other languages get it far, far worse - often with logically impossible or contradictory promises and requirements. Regards, Nick Maclaren.
2018-06-27 15:43 GMT+02:00 N.M. Maclaren <nmm1@cam.ac.uk>: > On Jun 27 2018, Janus Weil wrote: >> >> >> It is very unfortunate, and it means that the Fortran standard simply >> does not provide a measure for "more correct" here. (My common-sense >> drop-in notion of correctness would be that an optimization is >> 'correct' as long as it can be proven to not change any results.) > > > Actually, yes, it does. Fortran 2008 7.1.4p2, 7.1.5.2.4p2, 7.1.5.3.2p1, > 7.1.5.4.2p1, 7.1.5.5.2p1 and 7.1.6.3p1/2. I'm not completely sure what you deduce from all these quoted paragraphs, but applied to the cases we're discussing here, e.g. A = .false. .and. my_boldly_impure_function() I read them as saying that a compiler does not have to invoke the function if it doesn't feel like it, but with no definite obligation to remove the call either. Do you agree? > The problem is that the Fortran > standard has never resolved the inconsistency that OTHER side-effects in > functions are not explicitly forbidden Exactly. > it's one of the few places in > Fortran where there are conforming constructs that do not have specified > semantics or at least a clearly specified intention. And I really wonder why one would do such a thing. Like, ever. I'm curious: Are there actually other such cases in Fortran that you're aware of? > Many, many people have tried to sort that out, and all have failed dismally. > I regret that the proposal to deprecate impure functions did not succeed, > because at least that would have made one sane direction clear. I don't know. Why do so many people in the Fortran community have such a big problem with functions? Ok, they haven't been around in 77, but if you even bother to introduce them, why do it so half-heartedly and not just embrace them in their full beauty? What is so complicated about putting a statement into the Fortran standard that says: "Ok, if this function has side effects, we definitely must evaluate it, otherwise we lose the side effects. They might be important." ??? > This has been made much worse by IEEE 754 and (worse) coarray support. Like > almost all languages with threading and shared-memory, Fortran's concept of > pure is satisfactory for single-threaded code but not parallel. You need the > stronger concept, where a pure function does not depend on anything that > might change in its scope. We don't even need to start discussing these things if we can't even figure out what to do with an impure function in a simple single-threaded program. > Look on the bright side. If you thought this problem is a mess (it is), I > can assure you that most other languages get it far, far worse - often with > logically impossible or contradictory promises and requirements. I don't quite see that. Which languages are you talking about specifically? I see that C/C++ gives you a nice choice of writing A = false & my_boldly_impure_function() or A = false && my_boldly_impure_function() where the first option guarantees that your function is evaluated, while the second one allows (or even demands?) it to be optimized away. That's a much clearer situation than the Fortran mess, isn't it? My wish would be that instead of deprecating impure functions, the Fortran standard should rather introduce .ANDIF. / .ORELSE. operators. Cheers, Janus
On Wed, Jun 27, 2018 at 08:15:13AM +0200, Jakub Jelinek wrote: > > I think best would be to change the above to > code = TRUTH_AND_EXPR and code = TRUTH_OR_EXPR and have some > non-default aggressive optimization option that would optimize > away in the FE impure function calls from those operands if > one operand is cheap to evaluate and another one > contains function calls, by optimizing it into TRUTH_{AND,OR}IF_EXPR where > the cheap operand is the first operand and operand with function calls is > the second. I completely disagree. The Fortran standards (ie., F95, F2003, F2008, and F2018) clearly state that the operands of an expression need not be evaluate if the result of an expression can be determined. If an option is introduced, then it should be -fforce-evaluation. The standards also state that the order of evaluation is unspecified, which permits the optimization introduced by Thomas to provide the lack of symmetry that Janus observed in '.false. .and. check()' and 'check() .and. .false.' should be committed.
Hi Janus, > I don't think it's ok to not evaluate expressions that have side > effects The Fortran standard disagrees with you (as you know, this has been quoted previously). Evaluating a function in such a case is a missed optimization. > but as long as it is guarded by a non-default > optimization flag, I'm perfectly happy with optimizing away stuff with > side effects as well. Why would we need a non-default flag for an optimization that is perfectly OK with the language standard? I think we should follow the example of -fstrict-aliasing: Optimize by default according to what the language rules permit, and warn (with a flag included with -Wall) if we do so. I had always thought this to be the gcc way. Or would you also like to change the behavior of this flag? Regards Thomas
On Wed, Jun 27, 2018 at 11:09:56AM +0300, Janne Blomqvist wrote: > > How about committing a patch changing to use TRUTH_{AND|OR}_EXPR, and then > check benchmark results (polyhedron, spec etc.)? If performance worsens, we > revert, if it improves, great, lets keep it? > > To clarify, I'm not objecting to using TRUTH_{AND|OR}_EXPR, I'm objecting > to the notion that this is somehow "more correct" or "less bad" than the > current short-circuiting. The Fortran standard does not specify an > evaluation strategy; IMHO very unfortunately, but it is what it is. > Benchmarks aren't going to prove anything unless the benchmarks have the specific construct under discuss. Here's my benchmark that uses '.false. .and. check()' construct. Note, check() has no side-effect. % cat b.f90 program b logical, external :: check if (.false. .and. check()) then print *, 'here' else print *, 'there' end if end program b % cat a.f90 logical function check() real x 10 call random_number(x) if (x < 2) goto 10 check = .false. end function check % gfortran -o z a.f90 b.f90 && ./z there
On Jun 27 2018, Janus Weil wrote: > >I'm not completely sure what you deduce from all these quoted >paragraphs, but applied to the cases we're discussing here, e.g. > >A = .false. .and. my_boldly_impure_function() > >I read them as saying that a compiler does not have to invoke the >function if it doesn't feel like it, but with no definite obligation >to remove the call either. Do you agree? Yes. >> it's one of the few places in >> Fortran where there are conforming constructs that do not have specified >> semantics or at least a clearly specified intention. > >And I really wonder why one would do such a thing. Like, ever. > >I'm curious: Are there actually other such cases in Fortran that >you're aware of? Yes. Some in the I/O area, where every language has problems, because systems vary so much in their basic models. Some in the coarray area, especially atomics. And a few others that I now forget where. I would have to search my files (and memory!) to remind myself of exactly which issues I was referring to, even in the areas that I can remember have such issues. But see below. Where impure functions are different is because the others are all either in extreme cases or in areas that everybody knows are iffy. You can avoid all of them (including the impure function ones) except a few of the I/O ones by writing in a highly disciplined fashion - which is the key to high RAS, and portability over long periods of time and wildly different systems. >> Many, many people have tried to sort that out, and all have failed >> dismally. I regret that the proposal to deprecate impure functions did >> not succeed, because at least that would have made one sane direction >> clear. > >I don't know. Why do so many people in the Fortran community have such >a big problem with functions? Ok, they haven't been around in 77, but >if you even bother to introduce them, why do it so half-heartedly and >not just embrace them in their full beauty? Er, functions have been present in Fortran since 1958, and wording with similar effects to the current wording since 1966. This is an OLD problem :-( >What is so complicated about putting a statement into the Fortran >standard that says: "Ok, if this function has side effects, we >definitely must evaluate it, otherwise we lose the side effects. They >might be important." ??? Because that would mean a complete redesign of Fortran's semantic model of execution. It would also NOT be what a lot of people want. Inter alia, you would have to define what a side-effect is - and my point about IEEE 754 is very relevant here. >> Look on the bright side. If you thought this problem is a mess (it is), >> I can assure you that most other languages get it far, far worse - often >> with logically impossible or contradictory promises and requirements. > >I don't quite see that. Which languages are you talking about >specifically? I see that C/C++ gives you a nice choice of writing > >A = false & my_boldly_impure_function() >or >A = false && my_boldly_impure_function() > >where the first option guarantees that your function is evaluated, >while the second one allows (or even demands?) it to be optimized >away. That's a much clearer situation than the Fortran mess, isn't it? Demands. But I can assure you that things are NOT that simple, and C and C++ were among the languages I was referring to. I could go into horrible detail, but this is not the place to do that. And, no, the problems are not where you probably think they are. I never counted them up, but my estimate was that they were at least ten times as numerous and serious as for Fortran and, worse, they could NOT be avoided even in very clean code. I base this on being involved with the standards, personal experience of writing highly-portable code, and experience of being the local expert of last resort in the languages. Regards, Nick Maclaren.
On Wed, Jun 27, 2018 at 7:46 PM, Steve Kargl < sgk@troutmask.apl.washington.edu> wrote: > On Wed, Jun 27, 2018 at 11:09:56AM +0300, Janne Blomqvist wrote: > > > > How about committing a patch changing to use TRUTH_{AND|OR}_EXPR, and > then > > check benchmark results (polyhedron, spec etc.)? If performance worsens, > we > > revert, if it improves, great, lets keep it? > > > > To clarify, I'm not objecting to using TRUTH_{AND|OR}_EXPR, I'm objecting > > to the notion that this is somehow "more correct" or "less bad" than the > > current short-circuiting. The Fortran standard does not specify an > > evaluation strategy; IMHO very unfortunately, but it is what it is. > > > > Benchmarks aren't going to prove anything unless the > benchmarks have the specific construct under discuss. > What I'm saying is that as the Fortran standard doesn't specify exactly how logical expressions are to be evaluated, we are free to use either short-circuiting or non-short-circuiting (or choosing between them based on the phase of the moon). While I'm sure we may produce synthetic benchmarks to show either to be better [1], what matters is actual applications. Hence we should check benchmarks based on actual applications, like polyhedron or spec. Now, if this hypothetical patch has no impact on those benchmarks, then, well, I don't have any particular opinion whether it should be kept. [1] While obviously short-circuiting can avoid the evaluation of an expensive function like in your example, a naive translation of short-circuiting would produce more branches which aren't good either (though I guess and hope the middle end can get rid of most of them). Thus IMHO the decision should be based on the performance impact on real applications.
On Wed, Jun 27, 2018 at 8:26 PM, N.M. Maclaren <nmm1@cam.ac.uk> wrote: > On Jun 27 2018, Janus Weil wrote: > >> What is so complicated about putting a statement into the Fortran >> standard that says: "Ok, if this function has side effects, we >> definitely must evaluate it, otherwise we lose the side effects. They >> might be important." ??? >> > > Because that would mean a complete redesign of Fortran's semantic model > of execution. If the semantic model is unclear on whether a function with potential side-effects might or might not be evaluated, then IMNSHO the semantic model is shit, and should be fixed or replaced. > It would also NOT be what a lot of people want. Huh? Who wants that? And why?? > Inter > alia, you would have to define what a side-effect is - and my point > about IEEE 754 is very relevant here. > AFAIU most languages that take purity seriously, including in particular Haskell, explicitly consider the IEEE exceptions outside the purity model. Similarly, evaluating a pure expression might cause you to run out of memory and the program is killed, which is most definitely a side-effect but not one that is taken into account in the Haskell concept of purity. I could certainly live with such compromises on purity, and I don't think those examples are a good excuse to do nothing.
Am 27.06.2018 um 21:15 schrieb Janne Blomqvist: > If the semantic model is unclear on whether a function with potential > side-effects might or might not be evaluated, then IMNSHO the semantic > model is shit, and should be fixed or replaced. I disagree here, strongly. We are talking Fortran, and not C (where everything is a function, and often the whole point of calling a function is its side effects). And we are not going to change Fortran semantics. And I also oppose defining our own subset of Fortran in the hope that people will make fewer mistakes. A function is something that produces a value. If the value is not needed to produce the correct result, the function need not be called. Regards Thomas
On Jun 27 2018, Janne Blomqvist wrote: > >If the semantic model is unclear on whether a function with potential >side-effects might or might not be evaluated, then IMNSHO the semantic >model is shit, and should be fixed or replaced. Your opinion is noted. For all its faults, the Fortran semantic model has proved to be much more usable, much more robust and MUCH less full of gotchas than those of either C or C++, to name but two. No, I don't love it, but everything is relative. >> It would also NOT be what a lot of people want. > >Huh? Who wants that? And why?? Because side-effects include not just things that control the flow of execution, but things like diagnostics. Fortran takes purity seriously and does not allow I/O in pure procedures. It could be allowed, but doing that would need a major change to its I/O model! There is also code like 'X > Y .AND. SQRT(X) > Z'. Do you REALLY want SQRT called just to set the inexact flag, especially if the program isn't testing it? Not to say code like 'RANDOM() < 0.5 .and. pure_function(RANDOM()) > 1.0', which is common in functions that generate some statistical distributions, and in some open code that used random numbers. >> Inter >> alia, you would have to define what a side-effect is - and my point >> about IEEE 754 is very relevant here. > >AFAIU most languages that take purity seriously, including in particular >Haskell, explicitly consider the IEEE exceptions outside the purity model. >Similarly, evaluating a pure expression might cause you to run out of >memory and the program is killed, which is most definitely a side-effect >but not one that is taken into account in the Haskell concept of purity. > >I could certainly live with such compromises on purity, and I don't think >those examples are a good excuse to do nothing. Fortran is similar, though not the same as Haskell, in both respects. This is a different issue, does NOT apply to Fortran PURE procedures, and is partly for backwards compatibility. Tightening the rules significantly would break many existing programs; loosening them significantly would have a serious impact on performance. Languages that do not have to (or can't be bothered to) to take backwards compatibility seriously find it easier to change things. Regards, Nick Maclaren.
2018-06-27 21:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: > > And we are not going to change Fortran semantics. And I also oppose > defining our own subset of Fortran in the hope that people will make > fewer mistakes. > > A function is something that produces a value. If the value is not > needed to produce the correct result, the function need not be called. That's a bit oversimplified, isn't it? The thing that you're talking about, producing a only a value and nothing else, is actually a pure function. In general a function can be much more than that: It can not only produce a single value, but several ones via intent(out) arguments, it can do I/O, it can modify global/module state. I'd rather say a function is the same as a subroutine, but with a return value (in addition to everything a subroutine can do). And I neither want to change Fortran semantics, nor define my own subset of Fortran. The primary thing I'm hoping for is a certain level of compiler-independence, which is supposed to be a central point of any kind of 'standard'. So, since it's so incredibly hard to come to any kind of agreement here, maybe we can come back to the central part of your original patch, namely throwing a warning for constructs that can not be guaranteed to be executed in a compiler-independent fashion, i.e. logical expressions with impure functions. The patch I proposed at https://gcc.gnu.org/ml/fortran/2018-06/msg00160.html is the bare minimum that I hope we can agree upon. Does anyone see any problems with this, or can I possibly get an ok for committing this to trunk? Cheers, Janus
On Wed, Jun 27, 2018 at 10:46:05PM +0200, Janus Weil wrote: > 2018-06-27 21:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: > > > > And we are not going to change Fortran semantics. And I also oppose > > defining our own subset of Fortran in the hope that people will make > > fewer mistakes. > > > > A function is something that produces a value. If the value is not > > needed to produce the correct result, the function need not be called. > > That's a bit oversimplified, isn't it? Technically, the standard says an operand need not be evaluate, but you've asked people not to cite the Standard. I've also pointed you to F2018 Note 10.28 where it very clearly says that a function need not be evaluated with example nearly identical to the one in the PR. PURE vs IMPURE (or unmarked) function is a red herring. The standard makes no distinction.
2018-06-27 23:47 GMT+02:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: > On Wed, Jun 27, 2018 at 10:46:05PM +0200, Janus Weil wrote: >> 2018-06-27 21:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: >> > >> > And we are not going to change Fortran semantics. And I also oppose >> > defining our own subset of Fortran in the hope that people will make >> > fewer mistakes. >> > >> > A function is something that produces a value. If the value is not >> > needed to produce the correct result, the function need not be called. >> >> That's a bit oversimplified, isn't it? > > Technically, the standard says an operand need not be evaluate, > but you've asked people not to cite the Standard. I've also > pointed you to F2018 Note 10.28 where it very clearly says that > a function need not be evaluated with example nearly identical > to the one in the PR. PURE vs IMPURE (or unmarked) function > is a red herring. The standard makes no distinction. Look, Steve. This argument has been going in circles for weeks now. I think we can stop it here. So, again: I know that the Fortran standard allows the short-circuiting of impure functions. I'm not trying to change that behavior. (I don't like it but I certainly have to accept it.) But that still leaves us with a problem: The standard allows the short-circuiting but it doesn't require it. Meaning that any other compiler that does not do it (like ifort) is not in conflict with the standard either. Many people may want to avoid such compiler-dependent behavior. That's why we need a warning. All of the discussion here has shown that using impure functions in logical expressions is not a good idea in Fortran. It is not illegal, but it should be considered bad style. That's why we need a warning: https://gcc.gnu.org/ml/fortran/2018-06/msg00160.html Does anyone agree with me that this is a useful diagnostic? Whether this warning should be included in -Wall or -Wextra or -Whereever can be debated. Is this patch ok for trunk? Cheers, Janus 2018-06-27 23:47 GMT+02:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: > On Wed, Jun 27, 2018 at 10:46:05PM +0200, Janus Weil wrote: >> 2018-06-27 21:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: >> > >> > And we are not going to change Fortran semantics. And I also oppose >> > defining our own subset of Fortran in the hope that people will make >> > fewer mistakes. >> > >> > A function is something that produces a value. If the value is not >> > needed to produce the correct result, the function need not be called. >> >> That's a bit oversimplified, isn't it? > > Technically, the standard says an operand need not be evaluate, > but you've asked people not to cite the Standard. I've also > pointed you to F2018 Note 10.28 where it very clearly says that > a function need not be evaluated with example nearly identical > to the one in the PR. PURE vs IMPURE (or unmarked) function > is a red herring. The standard makes no distinction. > > -- > Steve
Hi Janus, if we add a warning, we should add it both for if (flag .and. func()) and for if (func() .and. flag) because the standard also allows reversing the test (which my original test did). Regards Thomas
On Thu, Jun 28, 2018 at 8:58 AM, Janus Weil <janus@gcc.gnu.org> wrote: > But that still leaves us with a problem: The standard allows the > short-circuiting but it doesn't require it. Meaning that any other > compiler that does not do it (like ifort) is not in conflict with the > standard either. > > Many people may want to avoid such compiler-dependent behavior. That's > why we need a warning. All of the discussion here has shown that using > impure functions in logical expressions is not a good idea in Fortran. > It is not illegal, but it should be considered bad style. That's why > we need a warning: > > https://gcc.gnu.org/ml/fortran/2018-06/msg00160.html > > Does anyone agree with me that this is a useful diagnostic? Whether > this warning should be included in -Wall or -Wextra or -Whereever can > be debated. Is this patch ok for trunk? > Ok. (I think -Wsurprising is the correct place for this)
On Wed, Jun 27, 2018 at 10:34 PM, Thomas Koenig <tkoenig@netcologne.de> wrote: > Am 27.06.2018 um 21:15 schrieb Janne Blomqvist: > > If the semantic model is unclear on whether a function with potential >> side-effects might or might not be evaluated, then IMNSHO the semantic >> model is shit, and should be fixed or replaced. >> > > I disagree here, strongly. We are talking Fortran, and not C (where > everything is a function, and often the whole point of calling a > function is its side effects). > If you can't robustly use functions with side-effects, they shouldn't be in the language in the first place, IMHO. So either there should be some sensible semantics for them, or they should be removed. Removing them would probably break the vast majority of existing Fortran code, so I think tightening up the semantics is the only reasonable solution. > And we are not going to change Fortran semantics. No, not we, here. I'm just expressing some wishful thinking that the Fortran standards committee would improve this aspect of the standard. But given Nicks statement that this has been tried, and failed, repeatedly in the past maybe I shouldn't get my hopes up.. And I also oppose > defining our own subset of Fortran in the hope that people will make > fewer mistakes. > I agree, and I hope my writings in this thread hasn't been interpreted as support for such a position. For one thing, if ever the Fortran standards committee decides on fixing the semantics of functions and on an evaluation strategy for logical expressions, and it's different from what our hypothetical "GNU Fortran standard" does, we have caused a lot of headache for our users and ourselves. I think the best we can do is to generate a warning, and make standard conforming code run as fast as possible.
Hi Thomas, > if we add a warning, we should add it both for > > if (flag .and. func()) > > and for > > if (func() .and. flag) > > because the standard also allows reversing the test (which my > original test did). well, I really don't want to warn for hypothetical problems. Since we currently do not apply such an optimization, we should not warn about it (unless you find any other compiler which actually does). Whether to add this optimization in the future is a different topic of course, and can still be discussed later. Cheers, Janus
On Jun 28 2018, Janus Weil wrote: > >> if we add a warning, we should add it both for >> >> if (flag .and. func()) >> and for >> if (func() .and. flag) >> >> because the standard also allows reversing the test (which my >> original test did). > >well, I really don't want to warn for hypothetical problems. Since we >currently do not apply such an optimization, we should not warn about >it (unless you find any other compiler which actually does). I have used such compilers; they used to be fairly common and may still be. I agree with Thomas, and think it should be in -Wextra. -Wsurprising is in -Wall, and this is done in so many programs (usually with not-pure functions that are actually pure, or effectively so) that the number of warnings would put people off using -Wall. Basically, it it is warning people about a feature of Fortran that they might not know, can almost always be resolved by modernising the code, and might just cause trouble in future gfortrans or some other compilers. It is not in the same class as the examples for -Wsurprising in the documentation. It's far more similar to those for -Wextra. I am looking at: https://gcc.gnu.org/onlinedocs/gcc-5.5.0/gfortran/Error-and-Warning-Options.html Regards, Nick Maclaren.
2018-06-28 13:05 GMT+02:00 N.M. Maclaren <nmm1@cam.ac.uk>: > On Jun 28 2018, Janus Weil wrote: >> >> >>> if we add a warning, we should add it both for >>> >>> if (flag .and. func()) >>> and for >>> if (func() .and. flag) >>> >>> because the standard also allows reversing the test (which my >>> original test did). >> >> >> well, I really don't want to warn for hypothetical problems. Since we >> currently do not apply such an optimization, we should not warn about >> it (unless you find any other compiler which actually does). > > > I have used such compilers; they used to be fairly common and may still be. You mean compilers which transform "if (func() .and. flag)" into "if (flag .and. func())", and then possibly remove the call? If yes, could you tell us which compilers you are talking about specifically? > I agree with Thomas, and think it should be in -Wextra. -Wsurprising is in > -Wall, and this is done in so many programs (usually with not-pure functions > that are actually pure, or effectively so) that the number of warnings would > put people off using -Wall. I agree that -Wextra might be more suitable than -Wall. Btw, effectively pure functions which are not marked with the PURE attribute should hopefully not be a problem, since gfortran has implicit_pure detection that should deal with that. (However, it might need some further improvements, I think.) Regarding warnings for "if (func() .and. flag)", I would like to have a very concrete reason, otherwise it will not be very useful. Which other compilers optimize this call away? Thanks for the constructive comments! Cheers, Janus
On Jun 28 2018, Janus Weil wrote: > > You mean compilers which transform "if (func() .and. flag)" into "if > (flag .and. func())", and then possibly remove the call? If yes, could > you tell us which compilers you are talking about specifically? I am 70, and haven't supported multiple compilers for over a decade! No, I can't remember. If I recall, the most widespread optimising compiler of the 1980s did (IBM Fortran Q), as well as several others of that era. I can't remember which of those I have used recently did, because it was not something I looked at - I had known for ages that it was likely, so followed the simple rule that ANY function call might disappear. Yes, even in statements like X = FRED(), because the value of X might not be needed. >Btw, effectively pure functions which are not marked with the PURE >attribute should hopefully not be a problem, since gfortran has >implicit_pure detection that should deal with that. (However, it might >need some further improvements, I think.) Most practical programmers use external libraries, often supplied as binary. The only information the compiler can rely on is whether the function is marked as PURE. It's moot whether such a warning should rely on implicit pure detection, because the gotcha with doing so is the programmer modifies the function, adds a side-effect, does NOT recompile the calling code, and can't work out why nothing has changed! Been there - done that :-) Response to self on locating it: you IDIOT, Nick! Regards, Nick Maclaren.
2018-06-28 13:59 GMT+02:00 N.M. Maclaren <nmm1@cam.ac.uk>: > On Jun 28 2018, Janus Weil wrote: >> >> You mean compilers which transform "if (func() .and. flag)" into "if (flag >> .and. func())", and then possibly remove the call? If yes, could you tell us >> which compilers you are talking about specifically? > > I am 70, and haven't supported multiple compilers for over a decade! > No, I can't remember. If I recall, the most widespread optimising > compiler of the 1980s did (IBM Fortran Q), as well as several others of > that era. I am roughly half your age, so the 80s for me fall under the category "a really long time ago" ;) Unfortunately I have no access to the IBM compiler, plus I have no idea which other compilers were relevant in that era (I was more into dinosaurs at that time and had no idea about programming yet ;) If anyone can point me to a present-decade compiler which does this, I'll be happy to add the warning. >> Btw, effectively pure functions which are not marked with the PURE >> attribute should hopefully not be a problem, since gfortran has >> implicit_pure detection that should deal with that. (However, it might >> need some further improvements, I think.) > > Most practical programmers use external libraries, often supplied as > binary. The only information the compiler can rely on is whether the > function is marked as PURE. > > It's moot whether such a warning should rely on implicit pure detection, > because the gotcha with doing so is the programmer modifies the function, > adds a side-effect, does NOT recompile the calling code, and can't work > out why nothing has changed! Been there - done that :-) Response to > self on locating it: you IDIOT, Nick! I hope that nowadays the necessity for such self-insults is significantly decreased as more people use a proper build system (which takes care of dependencies and rebuilds everything as needed). cmake does a pretty good job with this by now, in particular in connection with gfortran. So I wouldn't put too much weight into this argument ... Cheers, Janus
On Jun 28 2018, Janus Weil wrote: >> >> It's moot whether such a warning should rely on implicit pure detection, >> because the gotcha with doing so is the programmer modifies the function, >> adds a side-effect, does NOT recompile the calling code, and can't work >> out why nothing has changed! Been there - done that :-) Response to >> self on locating it: you IDIOT, Nick! > >I hope that nowadays the necessity for such self-insults is >significantly decreased as more people use a proper build system >(which takes care of dependencies and rebuilds everything as needed). >cmake does a pretty good job with this by now, in particular in >connection with gfortran. So I wouldn't put too much weight into this >argument ... Oh, yes, but the last time I fell into it is one that will be with us for always. I had 'de-PUREd' some procedures in order to do some tracing, forgot that a particular subroutine was called from a function as well as other subroutines, and that function was called in an expression of the sort we are discussing. In particular, when I compiled the procedure that contained the expression, the subroutine at the very bottom WAS still pure! It was only when I added the tracing that it wasn't. That's why I prefer diagnostics based on the stated interface, and not on what some other code currently does. Relying on the latter rather than a clear specification is one of the besetting sins of modern software, and is a major obstacle to improvements. Regards, Nick Maclaren.
On Thu, Jun 28, 2018 at 07:58:22AM +0200, Janus Weil wrote: > 2018-06-27 23:47 GMT+02:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: > > On Wed, Jun 27, 2018 at 10:46:05PM +0200, Janus Weil wrote: > >> 2018-06-27 21:34 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>: > >> > > >> > And we are not going to change Fortran semantics. And I also oppose > >> > defining our own subset of Fortran in the hope that people will make > >> > fewer mistakes. > >> > > >> > A function is something that produces a value. If the value is not > >> > needed to produce the correct result, the function need not be called. > >> > >> That's a bit oversimplified, isn't it? > > > > Technically, the standard says an operand need not be evaluate, > > but you've asked people not to cite the Standard. I've also > > pointed you to F2018 Note 10.28 where it very clearly says that > > a function need not be evaluated with example nearly identical > > to the one in the PR. PURE vs IMPURE (or unmarked) function > > is a red herring. The standard makes no distinction. > > Look, Steve. This argument has been going in circles for weeks now. I > think we can stop it here. > I've already stated that I have no problem with the warning. As Thomas noted, gfortran should warn for both '.false. .and. check()' and 'check() .and. .false.' I am opposed to the change of TRUTH_AND_EXPR to TRUTH_ANDIF_EXPR, where you are special casing logical expressions that involve .and. and .or. In fact, I'll be submitting a bug report for a missed optimization. subroutine foo(x,y) subroutine foo(x,y) real x(10), y(10) real x(10), y(10) y = 0*sin(x) y = 0 end subroutine foo end subroutine foo .L2: pxor %xmm0, %xmm0 call sinf movq $0, 32(%rsi) pxor %xmm1, %xmm1 movups %xmm0, (%rsi) mulss %xmm1, %xmm0 movups %xmm0, 16(%rsi) movss %xmm0, 0(%rbp,%rbx) addq $4, %rbx cmpq $40, %rbx jne .L2 which I'm sure you'll just be thrilled with.
2018-06-28 16:41 GMT+02:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: >> > Technically, the standard says an operand need not be evaluate, >> > but you've asked people not to cite the Standard. I've also >> > pointed you to F2018 Note 10.28 where it very clearly says that >> > a function need not be evaluated with example nearly identical >> > to the one in the PR. PURE vs IMPURE (or unmarked) function >> > is a red herring. The standard makes no distinction. >> >> Look, Steve. This argument has been going in circles for weeks now. I >> think we can stop it here. >> > > I've already stated that I have no problem with the warning. As > Thomas noted, gfortran should warn for both '.false. .and. check()' > and 'check() .and. .false.' It doesn't really help the discussion if you just re-state other people's positions. It might help if you would actually tell use *why* you think both cases should be checked? gfortran's current implementation of .and. is intrinsically asymmetric and only optimizes away the second operand if possible. My motivation for the warning is mostly to signal compiler-dependent behavior, and I still haven't seen a compiler that treats the case 'check() .and. .false.' different from gfortran. Are you aware of one? > I am opposed to the change of TRUTH_AND_EXPR to TRUTH_ANDIF_EXPR, > where you are special casing logical expressions that involve > .and. and .or. It's the other way around. We currently have TRUTH_ANDIF_EXPR. And no one really wants to change it right now. Let's move on. > In fact, I'll be submitting a bug report for a missed optimization. > > subroutine foo(x,y) subroutine foo(x,y) > real x(10), y(10) real x(10), y(10) > y = 0*sin(x) y = 0 > end subroutine foo end subroutine foo > > .L2: pxor %xmm0, %xmm0 > call sinf movq $0, 32(%rsi) > pxor %xmm1, %xmm1 movups %xmm0, (%rsi) > mulss %xmm1, %xmm0 movups %xmm0, 16(%rsi) > movss %xmm0, 0(%rbp,%rbx) > addq $4, %rbx > cmpq $40, %rbx > jne .L2 > > which I'm sure you'll just be thrilled with. I can't say I'm totally thrilled with it, but, yes, I do agree it's a missed optimization. That probably comes as a surprise to you, since you are apparently trying to tease me in some way here (didn't work). After all, SIN is an elemental function, thus pure and without any side effects. The call can certainly be removed if the value is not needed. Please submit your bug report, but please don't put me in CC. Thanks, Janus
On Thu, Jun 28, 2018 at 05:52:43PM +0200, Janus Weil wrote: > 2018-06-28 16:41 GMT+02:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: > >> > Technically, the standard says an operand need not be evaluate, > >> > but you've asked people not to cite the Standard. I've also > >> > pointed you to F2018 Note 10.28 where it very clearly says that > >> > a function need not be evaluated with example nearly identical > >> > to the one in the PR. PURE vs IMPURE (or unmarked) function > >> > is a red herring. The standard makes no distinction. > >> > >> Look, Steve. This argument has been going in circles for weeks now. I > >> think we can stop it here. > >> > > > > I've already stated that I have no problem with the warning. As > > Thomas noted, gfortran should warn for both '.false. .and. check()' > > and 'check() .and. .false.' > > It doesn't really help the discussion if you just re-state other > people's positions. It might help if you would actually tell use *why* > you think both cases should be checked? > > gfortran's current implementation of .and. is intrinsically asymmetric > and only optimizes away the second operand if possible. My motivation > for the warning is mostly to signal compiler-dependent behavior, and I > still haven't seen a compiler that treats the case 'check() .and. > .false.' different from gfortran. Are you aware of one? Why I think it a warning should be emitted: symmetry!. You complained about the lack of symmetry in 'check() .and. .false.' and '.false. .and. check()'. For the record, I also think Thomas's original patch that actually switch the operands should be applied. > > In fact, I'll be submitting a bug report for a missed optimization. > > > > subroutine foo(x,y) subroutine foo(x,y) > > real x(10), y(10) real x(10), y(10) > > y = 0*sin(x) y = 0 > > end subroutine foo end subroutine foo > > > > .L2: pxor %xmm0, %xmm0 > > call sinf movq $0, 32(%rsi) > > pxor %xmm1, %xmm1 movups %xmm0, (%rsi) > > mulss %xmm1, %xmm0 movups %xmm0, 16(%rsi) > > movss %xmm0, 0(%rbp,%rbx) > > addq $4, %rbx > > cmpq $40, %rbx > > jne .L2 > > > > which I'm sure you'll just be thrilled with. > > I can't say I'm totally thrilled with it, but, yes, I do agree it's a > missed optimization. That probably comes as a surprise to you, since > you are apparently trying to tease me in some way here (didn't work). > After all, SIN is an elemental function, thus pure and without any > side effects. The call can certainly be removed if the value is not > needed. Please submit your bug report, but please don't put me in CC. Change sin(x) to my_function_with_side_effects() if like. It is a missed optimization regardless of the function's pureness. You continue to miss my point or conveniently ignore it. You want to special case the forced evaluation of the operands in two specific logical expressions; namely, .and. and .or. If you want to force evaluation of operands, then do it for all binary operators.
On Thu, Jun 28, 2018 at 07:41:30AM -0700, Steve Kargl wrote: > I am opposed to the change of TRUTH_AND_EXPR to TRUTH_ANDIF_EXPR, > where you are special casing logical expressions that involve > .and. and .or. I think using TRUTH_AND_EXPR is the right thing to do, and if fortran functions can be optimized away if their results aren't used, then let's add some new attribute like const or pure attributes that would allow the middle-end to optimize away calls to functions with that attribute if the lhs isn't needed. This attribute has been discussed recently in the thread about caching functions. The question is what exactly should this attribute allow, if just DCE if lhs is unused, or also CSE, if the function has been called with the same arguments earlier, if it can be optimized into copying the result of the earlier call. > In fact, I'll be submitting a bug report for a missed optimization. > > subroutine foo(x,y) subroutine foo(x,y) > real x(10), y(10) real x(10), y(10) > y = 0*sin(x) y = 0 > end subroutine foo end subroutine foo > > .L2: pxor %xmm0, %xmm0 > call sinf movq $0, 32(%rsi) > pxor %xmm1, %xmm1 movups %xmm0, (%rsi) > mulss %xmm1, %xmm0 movups %xmm0, 16(%rsi) > movss %xmm0, 0(%rbp,%rbx) > addq $4, %rbx > cmpq $40, %rbx > jne .L2 > > which I'm sure you'll just be thrilled with. For floating point, you generally need -ffast-math to be able to optimize such computations away, sinf already has const attribute (at least in C/C++), so with -ffast-math it is optimized. Jakub
On Jun 28 2018, Steve Kargl wrote: > >You continue to miss my point or conveniently ignore it. >You want to special case the forced evaluation of the >operands in two specific logical expressions; namely, .and. >and .or. If you want to force evaluation of operands, then >do it for all binary operators. It's not just binary operators. It also includes code like: IF (FRED()) CONTINUE and even: X = FRED() X = 0.0 There are a zillion other possible ways in which this can arise; I have certainly seen those optimised out, as well as the constructs being discussed. I have no idea how many are currently optimised out by gfortran, still less how many may be in the future. Janus would like to know of compilers that do this sort of thing - the place to look is the ones sold by the supercomputing companies, like Cray, IBM, Hitachi and Fujitsu. I don't know how many others still maintain their own compilers. I am retired and no longer have access to any of them. Having a warning for known confusing cases (like .AND.) but not others is fine. Lots of warnings are like that. Having an option that makes a couple of constructs in the language behave in a way not required by the standard, but not others with similar properties, merely adds a new gotcha. Regards, Nick Maclaren.
2018-06-28 18:22 GMT+02:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: > On Thu, Jun 28, 2018 at 05:52:43PM +0200, Janus Weil wrote: >> 2018-06-28 16:41 GMT+02:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: >> >> > Technically, the standard says an operand need not be evaluate, >> >> > but you've asked people not to cite the Standard. I've also >> >> > pointed you to F2018 Note 10.28 where it very clearly says that >> >> > a function need not be evaluated with example nearly identical >> >> > to the one in the PR. PURE vs IMPURE (or unmarked) function >> >> > is a red herring. The standard makes no distinction. >> >> >> >> Look, Steve. This argument has been going in circles for weeks now. I >> >> think we can stop it here. >> >> >> > >> > I've already stated that I have no problem with the warning. As >> > Thomas noted, gfortran should warn for both '.false. .and. check()' >> > and 'check() .and. .false.' >> >> It doesn't really help the discussion if you just re-state other >> people's positions. It might help if you would actually tell use *why* >> you think both cases should be checked? >> >> gfortran's current implementation of .and. is intrinsically asymmetric >> and only optimizes away the second operand if possible. My motivation >> for the warning is mostly to signal compiler-dependent behavior, and I >> still haven't seen a compiler that treats the case 'check() .and. >> .false.' different from gfortran. Are you aware of one? > > Why I think it a warning should be emitted: symmetry!. > > You complained about the lack of symmetry in 'check() .and. .false.' > and '.false. .and. check()'. well, my original complaint in PR85599 was that the second one is broken, and your reaction to that is to break the first one as well. >> > In fact, I'll be submitting a bug report for a missed optimization. >> > >> > subroutine foo(x,y) subroutine foo(x,y) >> > real x(10), y(10) real x(10), y(10) >> > y = 0*sin(x) y = 0 >> > end subroutine foo end subroutine foo >> > >> > .L2: pxor %xmm0, %xmm0 >> > call sinf movq $0, 32(%rsi) >> > pxor %xmm1, %xmm1 movups %xmm0, (%rsi) >> > mulss %xmm1, %xmm0 movups %xmm0, 16(%rsi) >> > movss %xmm0, 0(%rbp,%rbx) >> > addq $4, %rbx >> > cmpq $40, %rbx >> > jne .L2 >> > >> > which I'm sure you'll just be thrilled with. >> >> I can't say I'm totally thrilled with it, but, yes, I do agree it's a >> missed optimization. That probably comes as a surprise to you, since >> you are apparently trying to tease me in some way here (didn't work). >> After all, SIN is an elemental function, thus pure and without any >> side effects. The call can certainly be removed if the value is not >> needed. Please submit your bug report, but please don't put me in CC. > > Change sin(x) to my_function_with_side_effects() if like. It > is a missed optimization regardless of the function's pureness. Yes, if you're an orthodox believer in The One True Standard. I'd rather use my own brain cells now and then. In this case my brain just tells me that it's not a good idea to apply an optimization that can totally change the results of my code, and that it's not a good idea for a 'standard' to not define the semantics of an operator / expression, but instead leave it to the compiler how it should be evaluated. You insist that the standard does not forbid this optimization. The standard also does not forbid explicitly that you shoot yourself it the foot with a nuclear missile. So, you go ahead and shoot. Then someone comes along and asks why you did that, and you reply: "The standard did not forbid it." One thing that I always failed to comprehend is people's fixation on optimization. What's so great about your code running 0.1% faster if the second compiler you try gives you totally different results, with no hints whether it's your code that's broken, or the first compiler, or the second one, or the standard that both compilers tried to implement? With a ten-line code that might not be such a big problem, but above 100.000 loc or so it can quickly become a huge issue. Cheers, Janus
Am 28.06.2018 um 19:21 schrieb Janus Weil: > One thing that I always failed to comprehend is people's fixation on > optimization. What's so great about your code running 0.1% faster if > the second compiler you try gives you totally different results, with > no hints The warning added for my patch is supposed to be a hint :-)
On Thu, Jun 28, 2018 at 07:03:05PM +0200, Jakub Jelinek wrote: > > In fact, I'll be submitting a bug report for a missed optimization. > > > > subroutine foo(x,y) subroutine foo(x,y) > > real x(10), y(10) real x(10), y(10) > > y = 0*sin(x) y = 0 > > end subroutine foo end subroutine foo > > > > .L2: pxor %xmm0, %xmm0 > > call sinf movq $0, 32(%rsi) > > pxor %xmm1, %xmm1 movups %xmm0, (%rsi) > > mulss %xmm1, %xmm0 movups %xmm0, 16(%rsi) > > movss %xmm0, 0(%rbp,%rbx) > > addq $4, %rbx > > cmpq $40, %rbx > > jne .L2 > > > > which I'm sure you'll just be thrilled with. > > For floating point, you generally need -ffast-math to be able to > optimize such computations away, sinf already has const attribute (at least > in C/C++), so with -ffast-math it is optimized. Doesn't -ffast-math allow the violaton of the integrity of parentheses? That is, it allows more optimizations that violate the standard.
On Thu, Jun 28, 2018 at 07:21:21PM +0200, Janus Weil wrote: > 2018-06-28 18:22 GMT+02:00 Steve Kargl <sgk@troutmask.apl.washington.edu>: > > > > Why I think it a warning should be emitted: symmetry!. > > > > You complained about the lack of symmetry in 'check() .and. .false.' > > and '.false. .and. check()'. > > well, my original complaint in PR85599 was that the second one is > broken, and your reaction to that is to break the first one as well. > Rose colored glasses. You say 'break'. I say 'fix'. Fortunately, I'm done with this discussion. Do what you want.
On 06/28/2018 06:22 PM, Steve Kargl wrote: > You continue to miss my point or conveniently ignore it. > You want to special case the forced evaluation of the > operands in two specific logical expressions; namely, .and. > and .or. If you want to force evaluation of operands, then > do it for all binary operators. And - most interesting - that's how Fortran 77 formulated it (way before PURE/IMPURE functions entered the language): "6.6.1 Evaluation of Operands It is not necessary for a processor to evaluate all of the operands of an expression if the value of the expression can be determined otherwise. This principle is most often applicable to logical expressions, but it applies to all expressions. For example, in evaluating the logical expression X .GT. Y .OR. L(Z) where X, Y, and Z are real, and L is a logical function, the function reference L(Z) need not be evaluated if X is greater than Y. If a statement contains a function reference in a part of an expression that need not be evaluated, all entities that would have become defined in the execution of that reference become undefined at the completion of evaluation of the expression containing the function reference. In the example above, evaluation of the expression causes Z to become undefined if L defines its argument." Kind regards,
On Jun 28 2018, Toon Moene wrote: > >And - most interesting - that's how Fortran 77 formulated it (way before >PURE/IMPURE functions entered the language): > >"6.6.1 Evaluation of Operands > >It is not necessary for a processor to evaluate all of the operands of >an expression if the value of the expression can be determined >otherwise. ... Or even Fortran 66 :-) 6.4 Evaluation of Expressions. A part of an expression need be evaluated only if such action is necessary to establish the value of the expression. ... When two elements are combined by an operator, the order of evaluation of the elements is optional. ... Regards, Nick Maclaren.
On Thu, Jun 28, 2018 at 10:34:35AM -0700, Steve Kargl wrote: > On Thu, Jun 28, 2018 at 07:03:05PM +0200, Jakub Jelinek wrote: > > > In fact, I'll be submitting a bug report for a missed optimization. > > > > > > subroutine foo(x,y) subroutine foo(x,y) > > > real x(10), y(10) real x(10), y(10) > > > y = 0*sin(x) y = 0 > > > end subroutine foo end subroutine foo > > > > > > .L2: pxor %xmm0, %xmm0 > > > call sinf movq $0, 32(%rsi) > > > pxor %xmm1, %xmm1 movups %xmm0, (%rsi) > > > mulss %xmm1, %xmm0 movups %xmm0, 16(%rsi) > > > movss %xmm0, 0(%rbp,%rbx) > > > addq $4, %rbx > > > cmpq $40, %rbx > > > jne .L2 > > > > > > which I'm sure you'll just be thrilled with. > > > > For floating point, you generally need -ffast-math to be able to > > optimize such computations away, sinf already has const attribute (at least > > in C/C++), so with -ffast-math it is optimized. > > Doesn't -ffast-math allow the violaton of the integrity > of parentheses? That is, it allows more optimizations > that violate the standard. No, -ffast-math doesn't turn on -fno-protect-parens, only -Ofast does AFAIK. Furthermore, to optimize away the sinf, you just need a subset of -ffast-math, in particular -O2 -ffinite-math-only -fno-signed-zeros optimizes it to zero stores too. But without those 2 options it can't be done, as if sinf returns e.g. negative value -1.0, then 0 * -1.0 is -0.0, not 0.0, so optimizing it into 0.0 would be incorrect. Similarly, if the function would return Inf or -Inf or NaN (I know sinf should never return Inf or -Inf, but it can return NaN, e.g. sinf (__builtin_inff ()) is NaN, or sinf (__builtin_nanf ("")) too), then 0 * NaN is NaN, not 0. Jakub
> 2018-06-27 10:09 GMT+02:00 Janne Blomqvist <blomqvist.janne@gmail.com>: > > > > How about committing a patch changing to use TRUTH_{AND|OR}_EXPR, and then > > check benchmark results (polyhedron, spec etc.)? If performance worsens, we > > revert, if it improves, great, lets keep it? > > I would definitely support that. I cannot imagine that it will have a > large impact on benchmarks, but it's certainly a good idea to check. > (After all: ifort, which is usually perceived as being slightly ahead > of GCC in terms of performance, does not do this kind of > short-circuiting either.) Polyhedron benchmark Compiler options: -static -ffpe-summary=none -O3 -pipe -mtune=native -march=native -ffast-math -ftree-vectorize Benchmark Unpatched Patched Name (secs) (sec) --------- -------- -------- ac 8.06 8.08 0.3% aermod 20.17 20.88 3.5% air 3.57 3.58 0.3% capacita 42.00 42.66 1.6% channel 1.84 1.88 2.2% doduc 20.53 20.65 0.6% fatigue 4.32 4.38 1.4% gas_dyn 2.20 2.15 (2.3%) induct 6.19 6.20 0.2% linpk 7.23 7.79 7.8% mdbx 7.18 7.26 1.1% nf 8.12 8.15 0.4% protein 26.72 26.98 1.0% rnflow 35.69 35.51 (0.5%) test_fpu 5.93 6.01 1.4% tfft 1.82 1.90 4.4% 14 of 16 tests experience a slow down. As this is unscientific, anything within 1% to 2% may not be significant. However, linpk is 7.8% slower, tfft is 4.4% slower, and aermod is 3.5% slower. More importantly, with TRUTH_ANDIF_EXPR and TRUTH_ORIF_EXPR (the status quo) on x86_64-*-freebsd, I see === gfortran Summary === # of expected passes 47564 # of expected failures 104 # of unsupported tests 85 /safe/sgk/gcc/obj/gcc/gfortran version 9.0.0 20180626 (experimental) (GCC) while with TRUTH_AND_EXPR and TRUTH_OR_EXPR, I get === gfortran Summary === # of expected passes 47558 # of unexpected failures 6 # of expected failures 104 # of unsupported tests 85 FAIL: gfortran.dg/actual_pointer_function_1.f90 -O0 execution test FAIL: gfortran.dg/actual_pointer_function_1.f90 -O1 execution test FAIL: gfortran.dg/actual_pointer_function_1.f90 -O2 execution test FAIL: gfortran.dg/actual_pointer_function_1.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions execution test FAIL: gfortran.dg/actual_pointer_function_1.f90 -O3 -g execution test FAIL: gfortran.dg/actual_pointer_function_1.f90 -Os execution test Execution timeout is: 300 spawn [open ...] Program received signal SIGSEGV: Segmentation fault - invalid memory reference. Backtrace for this error: #0 0x7ffffffff1a2 in ??? #1 0x400c09 in ??? #2 0x400b91 in ??? #3 0x400c51 in ??? #4 0x400854 in _start at /usr/src/lib/csu/amd64/crt1.c:74 #5 0x200627fff in ???
On Thu, Jun 28, 2018 at 07:36:56PM -0700, Steve Kargl wrote: > === gfortran Summary === > > # of expected passes 47558 > # of unexpected failures 6 > # of expected failures 104 > # of unsupported tests 85 > > FAIL: gfortran.dg/actual_pointer_function_1.f90 -O0 execution test > FAIL: gfortran.dg/actual_pointer_function_1.f90 -O1 execution test > FAIL: gfortran.dg/actual_pointer_function_1.f90 -O2 execution test > FAIL: gfortran.dg/actual_pointer_function_1.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions execution test > FAIL: gfortran.dg/actual_pointer_function_1.f90 -O3 -g execution test > FAIL: gfortran.dg/actual_pointer_function_1.f90 -Os execution test > > Execution timeout is: 300 > spawn [open ...] > > Program received signal SIGSEGV: Segmentation fault - invalid memory reference. > > Backtrace for this error: > #0 0x7ffffffff1a2 in ??? > #1 0x400c09 in ??? > #2 0x400b91 in ??? > #3 0x400c51 in ??? > #4 0x400854 in _start > at /usr/src/lib/csu/amd64/crt1.c:74 > #5 0x200627fff in ??? If you have a test that is broken by the TRUTH_ANDIF_EXPR -> TRUTH_AND_EXPR change, then the test must be broken, because from the snippets that were posted, Fortran does not require (unlike C/C++) that the second operand is not evaluated if the first one evaluates to false for (and) or true (for or), it just allows it. So, the optimizing away of the function calls should be an optimization, and as such should be done only when optimizing. So for -O0 at least always use TRUTH_{AND,OR}_EXPR, so that people can actually make sure that their programs are valid Fortran and can also step into those functions when debugging. For -O1 and higher perhaps use temporarily the *IF_EXPR, or better, as I said in another mail, let's add an attribute that will optimize all the calls that can be optimized, not just one special case. Jakub
2018-06-29 9:28 GMT+02:00 Jakub Jelinek <jakub@redhat.com>: > On Thu, Jun 28, 2018 at 07:36:56PM -0700, Steve Kargl wrote: >> === gfortran Summary === >> >> # of expected passes 47558 >> # of unexpected failures 6 >> # of expected failures 104 >> # of unsupported tests 85 >> >> FAIL: gfortran.dg/actual_pointer_function_1.f90 -O0 execution test >> FAIL: gfortran.dg/actual_pointer_function_1.f90 -O1 execution test >> FAIL: gfortran.dg/actual_pointer_function_1.f90 -O2 execution test >> FAIL: gfortran.dg/actual_pointer_function_1.f90 -O3 -fomit-frame-pointer -funroll-loops -fpeel-loops -ftracer -finline-functions execution test >> FAIL: gfortran.dg/actual_pointer_function_1.f90 -O3 -g execution test >> FAIL: gfortran.dg/actual_pointer_function_1.f90 -Os execution test >> >> Execution timeout is: 300 >> spawn [open ...] >> >> Program received signal SIGSEGV: Segmentation fault - invalid memory reference. >> >> Backtrace for this error: >> #0 0x7ffffffff1a2 in ??? >> #1 0x400c09 in ??? >> #2 0x400b91 in ??? >> #3 0x400c51 in ??? >> #4 0x400854 in _start >> at /usr/src/lib/csu/amd64/crt1.c:74 >> #5 0x200627fff in ??? > > If you have a test that is broken by the TRUTH_ANDIF_EXPR -> TRUTH_AND_EXPR > change, then the test must be broken, because from the snippets that were > posted, Fortran does not require (unlike C/C++) that the second operand is > not evaluated if the first one evaluates to false for (and) or true (for > or), it just allows it. Exactly. > So, the optimizing away of the function calls should be an optimization, and > as such should be done only when optimizing. So for -O0 at least always use > TRUTH_{AND,OR}_EXPR, so that people can actually make sure that their > programs are valid Fortran and can also step into those functions when > debugging. For -O1 and higher perhaps use temporarily the *IF_EXPR, or > better, as I said in another mail, let's add an attribute that will optimize > all the calls that can be optimized, not just one special case. Thanks for the comments, Jakub. I fully agree. This is pretty much the sanest strategy I've heard so far in all of this monstrous thread and I definitely support it. Cheers, Janus
Index: fortran/dump-parse-tree.c =================================================================== --- fortran/dump-parse-tree.c (Revision 261388) +++ fortran/dump-parse-tree.c (Arbeitskopie) @@ -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: fortran/resolve.c =================================================================== --- fortran/resolve.c (Revision 261388) +++ fortran/resolve.c (Arbeitskopie) @@ -3807,7 +3807,43 @@ lookup_uop_fuzzy (const char *op, gfc_symtree *uop return gfc_closest_fuzzy_match (op, candidates); } +/* 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_Wsurprising, "Impure function %qs at %L " + "might not be evaluated", name, &f->where); + else + gfc_warning (OPT_Wsurprising, "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. */ @@ -3910,6 +3946,8 @@ resolve_operator (gfc_expr *e) case INTRINSIC_NEQV: if (op1->ts.type == BT_LOGICAL && op2->ts.type == BT_LOGICAL) { + bool dont_move = false; + e->ts.type = BT_LOGICAL; e->ts.kind = gfc_kind_max (op1, op2); if (op1->ts.kind < e->ts.kind) @@ -3916,6 +3954,53 @@ 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) + { + bool op1_f, op2_f; + + op1_f = false; + op2_f = false; + gfc_expr_walker (&op1, impure_function_callback, &op1_f); + gfc_expr_walker (&op2, impure_function_callback, &op2_f); + + /* Some people code which depends on the short-circuiting that + Fortran does not provide, such as + + if (associated(m) .and. m%t) then + + So, warn about this idiom. However, avoid breaking + it on purpose. */ + + if (op1->expr_type == EXPR_FUNCTION && op1->value.function.isym + && op1->value.function.isym->id == GFC_ISYM_ASSOCIATED) + { + gfc_expr *e = op1->value.function.actual->expr; + gfc_expr *en = op1->value.function.actual->next->expr; + if (en == NULL && gfc_check_dependency (e, op2, true)) + { + gfc_warning (OPT_Wsurprising, "%qs function call at %L does " + "not guard expression at %L", "ASSOCIATED", + &op1->where, &op2->where); + dont_move = true; + } + } + + /* A bit of optimization: Transfer if (f(x) .and. flag) + into if (flag .and. f(x)), to save evaluation of a + function. The middle end should be capable of doing + this with a TRUTH_AND_EXPR, but it currently does not do + so. See PR 85599. */ + + if (!dont_move && op1_f && !op2_f) + { + e->value.op.op1 = op2; + e->value.op.op2 = op1; + op1 = e->value.op.op1; + op2 = e->value.op.op2; + } + } + break; } Index: testsuite/gfortran.dg/alloc_comp_default_init_2.f90 =================================================================== --- testsuite/gfortran.dg/alloc_comp_default_init_2.f90 (Revision 261388) +++ testsuite/gfortran.dg/alloc_comp_default_init_2.f90 (Arbeitskopie) @@ -11,7 +11,8 @@ program testprog integer, save :: callnb = 0 type(t_type) :: this allocate ( this % chars ( 4)) - if (.not.recursivefunc (this) .or. (callnb .ne. 10)) STOP 1 + if (.not.recursivefunc (this)) STOP 1 + if (callnb .ne. 10) STOP 2 contains recursive function recursivefunc ( this ) result ( match ) type(t_type), intent(in) :: this
Hello world, the attached patch introduces the following changes: If a logical .and. or .or. expression contains a reference to a function which is impure and which also does not behave like a pure function (i.e. does not have the implicit_pure attribute set), it emits a warning with -Wsurprising that the function might not be evaluated. (-Wsurprising is enabled by -Wall). It special cases the idiom if (associated(m) .and. m%t) which people appear to use. And, if there is an expression like func() .and. flag , it reverses the test as an optimization. The middle end should be capable of doing this, but apparently it doesn't, so the front end might as well do this. What it does not do is one part of PR 57160, i.e. warn against if (a /= 0 .and. 1/a > 5) which people who are used to C might also like to write. There is already quite some discussion in the PRs, especially 85599, where not all people were of the same opinion. Let us see where the discussion here leads us. Regression-tested (which found one bug in the testsuite). OK for trunk? Regards Thomas 2018-06-11 Thomas Koenig <tkoenig@gcc.gnu.org> PR fortran/57160 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. Special-case if (associated(m) .and. m%t). If an .and. or .or. expression has a function or a non-function, exchange the operands. 2018-06-11 Thomas Koenig <tkoenig@gcc.gnu.org> PR fortran/57160 PR fortran/85599 * gfortran.dg/logical_evaluation_1.f90: New test. * gfortran.dg/alloc_comp_default_init_2.f90: Fix code which implicitly depends on short-circuiting.