[fortran] Handling of .and. and .or. expressions

Message ID e2698932-14bb-e7e8-c9dd-6b59be1e39db@netcologne.de
State New
Headers show
Series
  • [fortran] Handling of .and. and .or. expressions
Related show

Commit Message

Thomas Koenig June 11, 2018, 7:22 p.m.
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.

Comments

Janus Weil June 13, 2018, 12:20 p.m. | #1
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.
Thomas Koenig June 13, 2018, 8:39 p.m. | #2
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
Steve Kargl June 13, 2018, 8:42 p.m. | #3
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.
Janne Blomqvist June 14, 2018, 8:05 a.m. | #4
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.
Janus Weil June 14, 2018, 8:38 a.m. | #5
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
Janne Blomqvist June 14, 2018, 9:41 a.m. | #6
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.
Janus Weil June 14, 2018, 9:44 a.m. | #7
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
Jakub Jelinek June 14, 2018, 9:55 a.m. | #8
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
Janus Weil June 14, 2018, 10:14 a.m. | #9
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
Janne Blomqvist June 14, 2018, 10:40 a.m. | #10
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.
Steve Kargl June 14, 2018, 2:47 p.m. | #11
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.
Janus Weil June 15, 2018, 9:22 a.m. | #12
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
Janus Weil June 15, 2018, 10:26 a.m. | #13
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
Thomas Koenig June 15, 2018, 5:06 p.m. | #14
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
Thomas Koenig June 15, 2018, 5:10 p.m. | #15
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...
Thomas Koenig June 15, 2018, 5:13 p.m. | #16
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.
Janne Blomqvist June 15, 2018, 5:27 p.m. | #17
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.
Janne Blomqvist June 15, 2018, 5:49 p.m. | #18
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)
Steve Kargl June 15, 2018, 6:38 p.m. | #19
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.
Janus Weil June 16, 2018, 10:35 a.m. | #20
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
Janus Weil June 16, 2018, 10:53 a.m. | #21
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
Janus Weil June 16, 2018, 11:09 a.m. | #22
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
Thomas Koenig June 16, 2018, 11:20 a.m. | #23
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).
Steve Kargl June 16, 2018, 4:38 p.m. | #24
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
Janus Weil June 16, 2018, 7:21 p.m. | #25
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
Steve Kargl June 16, 2018, 7:43 p.m. | #26
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".
Steve Kargl June 16, 2018, 8 p.m. | #27
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);
Jakub Jelinek June 16, 2018, 8:40 p.m. | #28
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
Janus Weil June 16, 2018, 8:42 p.m. | #29
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
Thomas Koenig June 16, 2018, 9:14 p.m. | #30
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;
 	}
Steve Kargl June 16, 2018, 9:20 p.m. | #31
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.
Steve Kargl June 16, 2018, 9:22 p.m. | #32
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
Janus Weil June 16, 2018, 10:38 p.m. | #33
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
Janus Weil June 25, 2018, 7:16 p.m. | #34
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
Thomas Koenig June 26, 2018, 9:12 p.m. | #35
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
Jakub Jelinek June 26, 2018, 9:16 p.m. | #36
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
Janus Weil June 27, 2018, 5:52 a.m. | #37
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
Jakub Jelinek June 27, 2018, 6:15 a.m. | #38
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
Janus Weil June 27, 2018, 7:35 a.m. | #39
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
Jakub Jelinek June 27, 2018, 7:42 a.m. | #40
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
Janus Weil June 27, 2018, 7:52 a.m. | #41
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
Jakub Jelinek June 27, 2018, 8:02 a.m. | #42
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
Janne Blomqvist June 27, 2018, 8:09 a.m. | #43
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.
Janus Weil June 27, 2018, 9:21 a.m. | #44
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
N.M. Maclaren June 27, 2018, 1:43 p.m. | #45
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.
Janus Weil June 27, 2018, 2:26 p.m. | #46
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
Steve Kargl June 27, 2018, 2:33 p.m. | #47
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.
Thomas Koenig June 27, 2018, 4:17 p.m. | #48
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
Steve Kargl June 27, 2018, 4:46 p.m. | #49
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
N.M. Maclaren June 27, 2018, 5:26 p.m. | #50
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.
Janne Blomqvist June 27, 2018, 7:05 p.m. | #51
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.
Janne Blomqvist June 27, 2018, 7:15 p.m. | #52
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.
Thomas Koenig June 27, 2018, 7:34 p.m. | #53
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
N.M. Maclaren June 27, 2018, 7:57 p.m. | #54
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.
Janus Weil June 27, 2018, 8:46 p.m. | #55
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
Steve Kargl June 27, 2018, 9:47 p.m. | #56
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.
Janus Weil June 28, 2018, 5:58 a.m. | #57
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
Thomas Koenig June 28, 2018, 6:40 a.m. | #58
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
Janne Blomqvist June 28, 2018, 6:42 a.m. | #59
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)
Janne Blomqvist June 28, 2018, 7:09 a.m. | #60
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.
Janus Weil June 28, 2018, 7:21 a.m. | #61
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
N.M. Maclaren June 28, 2018, 11:05 a.m. | #62
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.
Janus Weil June 28, 2018, 11:37 a.m. | #63
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
N.M. Maclaren June 28, 2018, 11:59 a.m. | #64
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.
Janus Weil June 28, 2018, 1:27 p.m. | #65
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
N.M. Maclaren June 28, 2018, 2:28 p.m. | #66
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.
Steve Kargl June 28, 2018, 2:41 p.m. | #67
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.
Janus Weil June 28, 2018, 3:52 p.m. | #68
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
Steve Kargl June 28, 2018, 4:22 p.m. | #69
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.
Jakub Jelinek June 28, 2018, 5:03 p.m. | #70
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
N.M. Maclaren June 28, 2018, 5:17 p.m. | #71
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.
Janus Weil June 28, 2018, 5:21 p.m. | #72
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
Thomas Koenig June 28, 2018, 5:34 p.m. | #73
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 :-)
Steve Kargl June 28, 2018, 5:34 p.m. | #74
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.
Steve Kargl June 28, 2018, 5:37 p.m. | #75
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.
Toon Moene June 28, 2018, 5:55 p.m. | #76
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,
N.M. Maclaren June 28, 2018, 6:50 p.m. | #77
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.
Jakub Jelinek June 28, 2018, 7:29 p.m. | #78
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
Steve Kargl June 29, 2018, 2:36 a.m. | #79
> 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 ???
Jakub Jelinek June 29, 2018, 7:28 a.m. | #80
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
Janus Weil June 29, 2018, 8:58 a.m. | #81
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

Patch

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