[Patch, Fortran] PR 57160: short-circuit IF only with -ffrontend-optimize

[Janus Weil]

Hi all,

here is a follow-up patch to my recent commit for PR 85599, also
dealing with the short-circuiting of logical operators. In the course
of the extensive discussion of that PR it became clear that the
Fortran standard allows the short-circuiting of .AND. and .OR.
operators, but does not mandate it.

gfortran currently does short-circuiting, and after my patch for PR
85599 warns about cases where this might remove an impure function
call (which potentially can change results).

Now, this PR (57160) is about code which relies on the
short-circuiting behavior. Since short-circuiting is not guaranteed by
the standard, such code is invalid. Generating a warning or an error
at compile-time is a bit harder here, though, since there are multiple
variations of such a situation, e.g.:
* ASSOCIATED(p) .AND. p%T
* ALLOCATED(a) .AND. a%T
* i<ubound(x) .AND. x(i)
* ...

The suggestion in the PR was to do short-circuiting only with
optimization flags, but inhibit it with -O0, so that the faulty code
will run into a segfault (or runtime error) at least when
optimizations are disabled, and the problem can be identified.

I find this suggestion very reasonable. It makes it possible to detect
invalid code at -O0, while keeping good performance for valid code at
-O{1,2,3}. Also it is technically very simple to implement, and it
immediately identified a faulty test case that has lived in the
testsuite for eleven years without being detected.

The attached patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?

Cheers,
Janus


2018-07-20 Janus Weil <***@gcc.gnu.org>

PR fortran/57160
* trans-expr.c (gfc_conv_expr_op): Use short-circuiting operators only
with -ffrontend-optimize. Without that flag, make sure that both
operands are evaluated.


2018-07-20 Janus Weil <***@gcc.gnu.org>

PR fortran/57160
* gfortran.dg/actual_pointer_function_1.f90: Fix invalid test case.

[Adam Hirst]

Post by Janus Weil
Hi all,
here is a follow-up patch to my recent commit for PR 85599, also
dealing with the short-circuiting of logical operators. In the course
of the extensive discussion of that PR it became clear that the
Fortran standard allows the short-circuiting of .AND. and .OR.
operators, but does not mandate it.
gfortran currently does short-circuiting, and after my patch for PR
85599 warns about cases where this might remove an impure function
call (which potentially can change results).
Now, this PR (57160) is about code which relies on the
short-circuiting behavior. Since short-circuiting is not guaranteed by
the standard, such code is invalid. Generating a warning or an error
at compile-time is a bit harder here, though, since there are multiple
* ASSOCIATED(p) .AND. p%T
* ALLOCATED(a) .AND. a%T
* i<ubound(x) .AND. x(i)
* ...
The suggestion in the PR was to do short-circuiting only with
optimization flags, but inhibit it with -O0, so that the faulty code
will run into a segfault (or runtime error) at least when
optimizations are disabled, and the problem can be identified.
I find this suggestion very reasonable. It makes it possible to detect
invalid code at -O0, while keeping good performance for valid code at
-O{1,2,3}. Also it is technically very simple to implement, and it
immediately identified a faulty test case that has lived in the
testsuite for eleven years without being detected.

One thing I'm not seeing in the original discussion was whether or not
this should also count for -Og, which certainly in my experience is
(paired with -g) a common debug setting.

I would err towards -Og here being paired with -O0, but I could see it
being argued both ways - either way, I thought it might at least be
worth making explicit?

Post by Janus Weil
The attached patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?
Cheers,
Janus
PR fortran/57160
* trans-expr.c (gfc_conv_expr_op): Use short-circuiting operators only
with -ffrontend-optimize. Without that flag, make sure that both
operands are evaluated.
PR fortran/57160
* gfortran.dg/actual_pointer_function_1.f90: Fix invalid test case.

[Janus Weil]

Hi Adam,

Post by Adam Hirst

Post by Janus Weil
Now, this PR (57160) is about code which relies on the
short-circuiting behavior. Since short-circuiting is not guaranteed by
the standard, such code is invalid. Generating a warning or an error
at compile-time is a bit harder here, though, since there are multiple
* ASSOCIATED(p) .AND. p%T
* ALLOCATED(a) .AND. a%T
* i<ubound(x) .AND. x(i)
* ...
The suggestion in the PR was to do short-circuiting only with
optimization flags, but inhibit it with -O0, so that the faulty code
will run into a segfault (or runtime error) at least when
optimizations are disabled, and the problem can be identified.
I find this suggestion very reasonable. It makes it possible to detect
invalid code at -O0, while keeping good performance for valid code at
-O{1,2,3}. Also it is technically very simple to implement, and it
immediately identified a faulty test case that has lived in the
testsuite for eleven years without being detected.

One thing I'm not seeing in the original discussion was whether or not
this should also count for -Og

good point!

Post by Adam Hirst
which certainly in my experience is
(paired with -g) a common debug setting.

Agreed. I actually use it myself.

Post by Adam Hirst
I would err towards -Og here being paired with -O0, but I could see it
being argued both ways - either way, I thought it might at least be
worth making explicit?

Well, yes, the current documentation for -ffrontend-optimize is not
horribly explicit, but it does say: " Enabled by default by any -O
option." Technically that includes -Og, I guess.

Phenomenologically, it seems that -Og indeed behaves like -O{1,2,3} in
this respect. If one wanted to change that, one would probably do
this:

Index: gcc/fortran/options.c
===================================================================
--- gcc/fortran/options.c (revision 262908)
+++ gcc/fortran/options.c (working copy)
@@ -417,7 +417,7 @@
specified it directly. */

if (flag_frontend_optimize == -1)
- flag_frontend_optimize = optimize;
+ flag_frontend_optimize = optimize && !optimize_debug;

/* Same for front end loop interchange. */


I tend to agree with you that this might be a good idea, but I also
don't have a strong opinion here. (Alternatively one could leave
-ffrontend-optimize as is, and just couple the short-circuiting
behavior to "flag_frontend_optimize && !optimize_debug", but that
seems less attractive to me.) Maybe others can comment?

Cheers,
Janus

[Fritz Reese]

Post by Janus Weil
Hi Adam,

[...]

Post by Janus Weil

Post by Adam Hirst
One thing I'm not seeing in the original discussion was whether or not
this should also count for -Og

good point!

Post by Adam Hirst
which certainly in my experience is
(paired with -g) a common debug setting.

Agreed. I actually use it myself.

Post by Adam Hirst
I would err towards -Og here being paired with -O0, but I could see it
being argued both ways - either way, I thought it might at least be
worth making explicit?

Well, yes, the current documentation for -ffrontend-optimize is not
horribly explicit, but it does say: " Enabled by default by any -O
option." Technically that includes -Og, I guess.
Phenomenologically, it seems that -Og indeed behaves like -O{1,2,3} in
this respect. If one wanted to change that, one would probably do
Index: gcc/fortran/options.c
===================================================================
--- gcc/fortran/options.c (revision 262908)
+++ gcc/fortran/options.c (working copy)
@@ -417,7 +417,7 @@
specified it directly. */
if (flag_frontend_optimize == -1)
- flag_frontend_optimize = optimize;
+ flag_frontend_optimize = optimize && !optimize_debug;
/* Same for front end loop interchange. */
I tend to agree with you that this might be a good idea, but I also
don't have a strong opinion here. (Alternatively one could leave
-ffrontend-optimize as is, and just couple the short-circuiting
behavior to "flag_frontend_optimize && !optimize_debug", but that
seems less attractive to me.) Maybe others can comment?

IMO it makes sense to omit frontend optimizations with -Og since one
probably expects -g/-Og to provide the most faithful reproduction of
the code (least optimized). I would be OK including this in the patch.

Post by Janus Weil
The attached patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?

I would recommend updating invoke.texi to include a comment regarding
the effect of -ffrontend-optimize on short-circuiting. If you include
the above regarding -Og, you should also clarify "Enabled by default
by any -O option" (e.g. "Enabled by any -O option except -O0 and
-Og").

Normally I like to see testcase(s) enforcing the new behavior as well,
unless there is a good reason not to. (That way any future changes to
short-circuiting or -ffrontend-optimize should at least snag on the
testcase and cause special consideration.)

Otherwise looks OK.

Fritz

[Janus Weil]

Post by Fritz Reese

Post by Janus Weil

Post by Adam Hirst
I would err towards -Og here being paired with -O0, but I could see it
being argued both ways - either way, I thought it might at least be
worth making explicit?

Well, yes, the current documentation for -ffrontend-optimize is not
horribly explicit, but it does say: " Enabled by default by any -O
option." Technically that includes -Og, I guess.
Phenomenologically, it seems that -Og indeed behaves like -O{1,2,3} in
this respect. If one wanted to change that, one would probably do
Index: gcc/fortran/options.c
===================================================================
--- gcc/fortran/options.c (revision 262908)
+++ gcc/fortran/options.c (working copy)
@@ -417,7 +417,7 @@
specified it directly. */
if (flag_frontend_optimize == -1)
- flag_frontend_optimize = optimize;
+ flag_frontend_optimize = optimize && !optimize_debug;
/* Same for front end loop interchange. */
I tend to agree with you that this might be a good idea, but I also
don't have a strong opinion here. (Alternatively one could leave
-ffrontend-optimize as is, and just couple the short-circuiting
behavior to "flag_frontend_optimize && !optimize_debug", but that
seems less attractive to me.) Maybe others can comment?

IMO it makes sense to omit frontend optimizations with -Og since one
probably expects -g/-Og to provide the most faithful reproduction of
the code (least optimized). I would be OK including this in the patch.

Good, since we all seem to agree on that, I'm including it in the
patch (new version in the attachment).

Post by Fritz Reese

Post by Janus Weil
The attached patch regtests cleanly on x86_64-linux-gnu. Ok for trunk?

I would recommend updating invoke.texi to include a comment regarding
the effect of -ffrontend-optimize on short-circuiting. If you include
the above regarding -Og, you should also clarify "Enabled by default
by any -O option" (e.g. "Enabled by any -O option except -O0 and
-Og").

Done.

Post by Fritz Reese
Normally I like to see testcase(s) enforcing the new behavior as well,
unless there is a good reason not to. (That way any future changes to
short-circuiting or -ffrontend-optimize should at least snag on the
testcase and cause special consideration.)

I somehow thought that the change to actual_pointer_function_1 would
be enough, but of course this does not verify the general behavior wrt
to short-circuiting. I'm attaching two test cases in this direction
now.

Post by Fritz Reese
Otherwise looks OK.

Thanks for the review. The new patch also disables the warnings from
PR85599 if -ffrontend-optimize is not given, as noted by Dominique.

The attached is what I'd like to commit (and is regtesting now), but
I'll wait for further comments of course.

Cheers,
Janus

[Janus Weil]

Post by Janus Weil

Post by Fritz Reese
Otherwise looks OK.

Thanks for the review. The new patch also disables the warnings from
PR85599 if -ffrontend-optimize is not given, as noted by Dominique.
The attached is what I'd like to commit (and is regtesting now), but
I'll wait for further comments of course.

Regtesting showed one failure in inline_matmul_23, which is fixed by
adding -ffrontend-optimize. Updated patch attached.

Cheers,
Janus

[Thomas Koenig]

Post by Janus Weil
Good, since we all seem to agree on that, I'm including it in the
patch (new version in the attachment).

Sorry for chiming in so late, but I don't (and Dominique doesn't
either).

If you want to enforce either short-circuit evaluation or
forced evaluation, please use a dedicated option.

I don't think it is a good idea to have

if (allocated(x) .and. any(x>0))

crash on -O0 and not crash on -O.

-O should not change the semantics of a program in
such a way.

Regards

Thomas

[Janus Weil]

Post by Thomas Koenig

Post by Janus Weil
Good, since we all seem to agree on that, I'm including it in the
patch (new version in the attachment).

Sorry for chiming in so late, but I don't (and Dominique doesn't
either).

Dominique has not commented on the question whether -Og should imply
-ffrontend-optimization, therefore I don't know his opinion on that
matter.

Post by Thomas Koenig
If you want to enforce either short-circuit evaluation or
forced evaluation, please use a dedicated option.

I don't want to enforce any of that. gfortran is enforcing
short-circuiting right now (with any option), and I want to change
that.

Post by Thomas Koenig
I don't think it is a good idea to have
if (allocated(x) .and. any(x>0))
crash on -O0 and not crash on -O.

I actually do. The code is invalid. Crashing it is good, because it
draws the user's attention to that fact.

You can find lots of invalid programs that change their behavior with
different optimization flags.

Post by Thomas Koenig
-O should not change the semantics of a program in
such a way.

You should thank the authors of the Fortran standard for not fixing
the semantics of and/or operands.

Note that -O will not change the results of valid code (except if
impure functions are involved, and those trigger a warning since
recently).

Cheers,
Janus

[Janus Weil]

Post by Janus Weil

Post by Thomas Koenig
If you want to enforce either short-circuit evaluation or
forced evaluation, please use a dedicated option.

I don't want to enforce any of that. gfortran is enforcing
short-circuiting right now (with any option), and I want to change
that.

To expand on that point: I really don't see why a dedicated option is
needed. Short-circuiting of logical expressions is certainly an
"optimization" (which is allowed but not required by the Fortran
standard), and it is done in the Fortran front end, thus enabling it
with -ffrontend-optimize makes perfect sense to me. Actually I don't
really care which one of the plenty optimization flags of GCC turns it
on, but I do think that, as all other optimizations, it should
certainly be disabled at -O0.

In fact the the idea put forward by Joost in PR57160 (which I'm
implementing here) coincides with the policy proposed by Jakub in

Post by Janus Weil
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.

Post by Thomas Koenig
I don't think it is a good idea to have
if (allocated(x) .and. any(x>0))
crash on -O0 and not crash on -O.

I actually do. The code is invalid. Crashing it is good, because it
draws the user's attention to that fact.

What would be even better than a hard crash (segfault) would be a
proper runtime error that gives more information about the nature of
the problem. -fcheck=all may already catch some of the situations
mentioned in the beginning of this thread, but it clearly needs
improvement/extension, which could be tackled as a follow-up.

If there is further constructive criticism regarding v3 of the patch,
as posted at https://gcc.gnu.org/ml/fortran/2018-07/msg00103.html,
I'll be happy to deal with that. If not, I'd like to commit it by the
weekend.

Cheers,
Janus

[Nicolas Koenig]

HI Janus,

Post by Janus Weil
To expand on that point: I really don't see why a dedicated option is
needed. Short-circuiting of logical expressions is certainly an
"optimization" (which is allowed but not required by the Fortran
standard),

If you do mean optimizations, why the quotes?

And if you think this is an optimization, why did you object to
my patch which actually used the leeway given by the
Fortran standard?

And if you don't think this is an optimization, coupling this to
an -O option is not the right way.

Regards

Thomas

[Thomas Koenig]

Nicolas Koenig did not actually write...

Post by Nicolas Koenig
HI Janus,

Oops - sorry for that. I had selected the wrong mail account for
this, that was actually me writing, not Nicolas.

Regards

Thomas

[Janus Weil]

Hi Thomas,

Post by Nicolas Koenig

Post by Janus Weil
To expand on that point: I really don't see why a dedicated option is
needed. Short-circuiting of logical expressions is certainly an
"optimization" (which is allowed but not required by the Fortran
standard),

If you do mean optimizations, why the quotes?
And if you think this is an optimization, why did you object to
my patch which actually used the leeway given by the
Fortran standard?
And if you don't think this is an optimization, coupling this to
an -O option is not the right way.

first off: I clearly think that this is an optimization (I used the
quotes merely as an emphasis).

But in any case it's not about what I think, but about what the
standard says. And the standard says things like:

"It is not necessary for a processor to evaluate all of the operands
of an expression, or to evaluate entirely each
operand, if the value of the expression can be determined otherwise."

To me that sounds like omitting the evaluation of an operand (aka
"short-circuiting") is something that a compiler is allowed to do, but
not required. The main motivation for omitting the evaluation is
typically performance. So, according to my interpretation of the
standard, short-circuiting is what you would call an "optimization".

Note that this in contrast to the C/C++ case, where short-circuiting
is part of the semantics of the && and || operators, thus it's
required (not optional) and therefore not an optimization.


One of the reasons why I objected to your earlier patch was precisely
the missing possibility to disable the optimization. If this
possibility is provided, I'd actually be willing to discuss further
optimizations (provided they can guarantee for all valid code that the
results are not modified) ...

Cheers,
Janus

[Dominique d'Humières]

Hi Janus,

Post by Janus Weil
gfortran currently does short-circuiting, and after my patch for PR
85599 warns about cases where this might remove an impure function
call (which potentially can change results).
Now, this PR (57160) is about code which relies on the
short-circuiting behavior. Since short-circuiting is not guaranteed by
the standard, such code is invalid. Generating a warning or an error
at compile-time is a bit harder here, though, since there are multiple
* ASSOCIATED(p) .AND. p%T
* ALLOCATED(a) .AND. a%T
* i<ubound(x) .AND. x(i)
* …

Aren’t you confusing portability with validity?
The above codes are indeed invalid without short-circuit evaluation,
but I did not find anything in the standard saying such codes are
invalid with short-circuit evaluation.

Post by Janus Weil
The suggestion in the PR was to do short-circuiting only with
optimization flags, but inhibit it with -O0, so that the faulty code
will run into a segfault (or runtime error) at least when
optimizations are disabled, and the problem can be identified.

This PR has nothing to do with optimization and I think
it is a very bad idea to (ab)use any optimization option.

Please leave the default behavior (and test) as they are now.
If you want non short-circuit evaluation, introduce an option for it.

Note that the warning introduce for pr85599 should be disabled
for non short-circuit evaluations.

TIA

Dominique

[Janus Weil]

Post by Dominique d'Humières
Hi Janus,

Post by Janus Weil
gfortran currently does short-circuiting, and after my patch for PR
85599 warns about cases where this might remove an impure function
call (which potentially can change results).
Now, this PR (57160) is about code which relies on the
short-circuiting behavior. Since short-circuiting is not guaranteed by
the standard, such code is invalid. Generating a warning or an error
at compile-time is a bit harder here, though, since there are multiple
* ASSOCIATED(p) .AND. p%T
* ALLOCATED(a) .AND. a%T
* i<ubound(x) .AND. x(i)
* …

Aren’t you confusing portability with validity?

I don' think so.

Post by Dominique d'Humières
The above codes are indeed invalid without short-circuit evaluation,
but I did not find anything in the standard saying such codes are
invalid with short-circuit evaluation.

If they're not valid without short-circuiting, then they're not valid
at all, because the Fortran standard does not require a processor to
do short-circuiting.

Post by Dominique d'Humières

Post by Janus Weil
The suggestion in the PR was to do short-circuiting only with
optimization flags, but inhibit it with -O0, so that the faulty code
will run into a segfault (or runtime error) at least when
optimizations are disabled, and the problem can be identified.

This PR has nothing to do with optimization and I think
it is a very bad idea to (ab)use any optimization option.

The PR is definitely related to optimization, because the fact that
the invalid test case works at all is only due to an optional
optimization called short-circuiting.

Don't get me wrong. I think it would be great if the Fortran standard
would *require* short-circuiting for and/or operators (or had separate
operators which do that). That would allow to write code like
"ASSOCIATED(p) .AND. p%T" (which would actually be useful).
Unfortunately that's not the case, therefore such code is plain
invalid.

Post by Dominique d'Humières
Please leave the default behavior (and test) as they are now.

You seriously prefer to keep an invalid test case in the testsuite?

Post by Dominique d'Humières
If you want non short-circuit evaluation, introduce an option for it.

Your argument could easily be reversed: If you want short-circuiting,
go introduce an option for it.

I'm sure we'll not get anywhere this way, and I do think that Joost's
suggestion to avoid short-circuiting with -O0 (and possibly -Og) is
very reasonable: People who want maximum performance still get that
with -O3, and at the same time they can still check their codes for
errors with -O0.

What is your problem?!?

Post by Dominique d'Humières
Note that the warning introduce for pr85599 should be disabled
for non short-circuit evaluations.

That's a valid point.

Cheers,
Janus

[Janne Blomqvist]

Post by Janus Weil

Post by Dominique d'Humières
If you want non short-circuit evaluation, introduce an option for it.

Your argument could easily be reversed: If you want short-circuiting,
go introduce an option for it.
I'm sure we'll not get anywhere this way, and I do think that Joost's
suggestion to avoid short-circuiting with -O0 (and possibly -Og) is
very reasonable: People who want maximum performance still get that
with -O3, and at the same time they can still check their codes for
errors with -O0.

I agree with Dominique, that this would be better as a separate option.
Optimization bugs that pop up at different optimization levels are hard
enough for users to figure out, without the frontend generating different
code to begin with depending on the optimization level. Also, with a
separate option it would be easy to check how it affects performance at
different optimization levels.

What about making it a -fcheck=short-circuit-logicals (or however you want
to spell it?) option, that also would be enabled with -fcheck=all?

[Janus Weil]

Post by Janne Blomqvist
Optimization bugs that pop up at different optimization levels are hard
enough for users to figure out

Right, and they're impossible to detect if there is no way to disable
the optimization, which is what this PR is about.

Post by Janne Blomqvist
without the frontend generating different
code to begin with depending on the optimization level.

In the end it doesn't make much of a difference whether the
optimizations are done in the front or the middle end. The user knows
nothing about this, and he doesn't need to.

The problematic point here is that short-circuiting is an optimization
that is enabled already at -O0.

Post by Janne Blomqvist
Also, with a
separate option it would be easy to check how it affects performance at
different optimization levels.

For the case at hand, the short-circuiting is an absolutely valid
optimization. There is no reason why you wouldn't wanna do it (with -O
flags).

Post by Janne Blomqvist
What about making it a -fcheck=short-circuit-logicals (or however you want
to spell it?) option, that also would be enabled with -fcheck=all?

What would such a flag even do? The actually invalid operation in the
test case is a null-pointer access, which could be caught by
-fcheck=pointer if we disable the optimization that removes it (i.e.
short-circuiting).

However, it also seems like -fcheck=pointer could use some
enhancement, since it does not even seem to catch simple cases like
this:

integer, pointer :: p => null()
print *,p
end

Cheers,
Janus

[Dominique d'Humières]

Post by Janus Weil
If there is further constructive criticism regarding v3 of the patch,
as posted at https://gcc.gnu.org/ml/fortran/2018-07/msg00103.html,
I'll be happy to deal with that. If not, I'd like to commit it by the
weekend.

This patch breaks backward compatibility and make the situation more confusing that it is presently.

Hence I am definitively against this patch.

Now I have wasted more than enough of my time on this nonsense and I won’t post anything else on this thread.

Dominique

[Janus Weil]

Post by Dominique d'Humières

Post by Janus Weil
If there is further constructive criticism regarding v3 of the patch,
as posted at https://gcc.gnu.org/ml/fortran/2018-07/msg00103.html,
I'll be happy to deal with that. If not, I'd like to commit it by the
weekend.

This patch breaks backward compatibility

Not for valid Fortran code. The patch only affects code that is not
guaranteed to work by the Fortran standard, like "if (associated(a)
.and. a>0)".

"Breaking" such code is precisely the purpose of the patch. The idea
is to discourage users from writing such code, because it is not valid
Fortran code (and therefore not portable). Are you trying to argue
that such code should work with gfortran?

Post by Dominique d'Humières
Hence I am definitively against this patch.

If you are against it, do you have *any* other solution for the problem at hand?

Post by Dominique d'Humières
Now I have wasted more than enough of my time on this nonsense and I won’t post anything else on this thread.

Say what ...? You know, your behavior is pissing me off like hell, dude.

What you're saying here is that you disapprove of the patch, but
you're not willing to discuss the reasons. That is not constructive at
all. I'm trying to make a contribution to improving gfortran, and you
are just brute-force blocking that effort due some very emotional
personal opinion, and with zero objective arguments. I don't think
that's how things should work, and it greatly reduces my motivation to
make any further contributions here.

In fact it's me who's wasting his time, not you. You are just blaring
out three-sentence replies with words like "nonsense" that don't
contribute anything to this discussion except bad vibrations. That's
not helping anyone, and it's certainly not helping to improve
gfortran.

Cheers,
Janus

[Janus Weil]

Post by Janus Weil

Post by Dominique d'Humières

Post by Janus Weil
If there is further constructive criticism regarding v3 of the patch,
as posted at https://gcc.gnu.org/ml/fortran/2018-07/msg00103.html,
I'll be happy to deal with that. If not, I'd like to commit it by the
weekend.

This patch breaks backward compatibility

Not for valid Fortran code. The patch only affects code that is not
guaranteed to work by the Fortran standard, like "if (associated(a)
.and. a>0)".
"Breaking" such code is precisely the purpose of the patch. The idea
is to discourage users from writing such code, because it is not valid
Fortran code (and therefore not portable). Are you trying to argue
that such code should work with gfortran?

Post by Dominique d'Humières
Hence I am definitively against this patch.

If you are against it, do you have *any* other solution for the problem at hand?

Ping!

We have a problem here which deserves a solution. So, Dominique, if
you don't like my solution, how do you propose to solve PR 57160?

Thanks,
Janus

[Janus Weil]

Post by Janus Weil

Post by Janus Weil

Post by Dominique d'Humières

Post by Janus Weil
If there is further constructive criticism regarding v3 of the patch,
as posted at https://gcc.gnu.org/ml/fortran/2018-07/msg00103.html,
I'll be happy to deal with that. If not, I'd like to commit it by the
weekend.

This patch breaks backward compatibility

Not for valid Fortran code. The patch only affects code that is not
guaranteed to work by the Fortran standard, like "if (associated(a)
.and. a>0)".
"Breaking" such code is precisely the purpose of the patch. The idea
is to discourage users from writing such code, because it is not valid
Fortran code (and therefore not portable). Are you trying to argue
that such code should work with gfortran?

Post by Dominique d'Humières
Hence I am definitively against this patch.

If you are against it, do you have *any* other solution for the problem at hand?

Ping!
We have a problem here which deserves a solution. So, Dominique, if
you don't like my solution, how do you propose to solve PR 57160?

ping**2

It's so easy to scream "bullshit", and so hard to find a good
solution. Still, we need one ...

I suggest the naysayers get active. I'm out.

Cheers,
Janus

[Dominique d'Humières]

Post by Janus Weil
It's so easy to scream "bullshit", and so hard to find a good
solution. Still, we need one …

Sorry if I have not been clear enough. I see only two
sensible solutions:

(1) The statu quo, closing the PR as WONTFIX
(2) Introduce a new option, for instance
-fshort-circuit (default) for the short-circuit evaluation
-fno-short-circuit to force the evaluation of both expressions.

Dominique

[Janus Weil]

Post by Dominique d'Humières

Post by Janus Weil
It's so easy to scream "bullshit", and so hard to find a good
solution. Still, we need one …

Sorry if I have not been clear enough. I see only two
(1) The statu quo, closing the PR as WONTFIX

How is that even an option?!? gfortran accepts invalid code. What's
wrong with fixing that?

If your aim is to keep "the status quo" (read: a compiler with
gazillions of bugs), why don't we just shut down bugzilla altogether?

Post by Dominique d'Humières
(2) Introduce a new option, for instance
-fshort-circuit (default) for the short-circuit evaluation
-fno-short-circuit to force the evaluation of both expressions.

For starters, no one will find a bug in his code if it requires
-fsome-obscure-option-that-nobody-knows.

Also I could ask the innocent question why -fshort-circuit should be
the default, but I guess I won't bother to go there.

I still find it deeply unsatisfying that gfortran is incompatible with
other compilers on a very fundamental level. And I find it very
disturbing that no one even cares. Anyway, I'm kinda through with this
whole issue. I guess I'll just stop caring as well.

Goodbye ...

[Thomas König]

Hi Janus,

Post by Janus Weil
How is that even an option?!? gfortran accepts invalid code. What's
wrong with fixing that?

Just a general remark, I am on a business trip in the US at the moment.

There are errors which the compiler is required to diagnose, when a constraint is violated. And there are errors which the compiler is not required to catch, those marked with „shall“ in the standard.

Issuing an error or a warning for those is always a balancing act because if the possibility of false positives.

Regards, Thomas

[William Clodius]

The confusion of false negative is also a concern.

Post by Dominique d'Humières
Hi Janus,

Post by Janus Weil
How is that even an option?!? gfortran accepts invalid code. What's
wrong with fixing that?

Just a general remark, I am on a business trip in the US at the moment.
There are errors which the compiler is required to diagnose, when a constraint is violated. And there are errors which the compiler is not required to catch, those marked with „shall“ in the standard.
Issuing an error or a warning for those is always a balancing act because if the possibility of false positives.
Regards, Thomas

[Janus Weil]

Hi Thomas,

Post by Thomas König

Post by Janus Weil
How is that even an option?!? gfortran accepts invalid code. What's
wrong with fixing that?

Just a general remark, I am on a business trip in the US at the moment.
There are errors which the compiler is required to diagnose, when a constraint is violated. And there are errors which the compiler is not required to catch, those marked with „shall“ in the standard.
Issuing an error or a warning for those is always a balancing act because if the possibility of false positives.

sure, I'm aware of that general fact.

However, it's certainly a good idea to diagnose everything you can, no
matter if the standard requires you to do that or not. Having
undetectably invalid code does not increase the appeal of the compiler
(or the language in general). Just like having compiler-dependent
code. All of that is a major turn-off (at least to me).

I'll also note that my approach to fixing PR 57160 had zero false positives.

Cheers,
Janus

[Manfred Schwarb]

Hi Janus, hi Dominique,

I did not raise my voice so far, as it is not so terribly relevant I guess,
nevertheless...

Post by Janus Weil

Post by Dominique d'Humières

Post by Janus Weil
It's so easy to scream "bullshit", and so hard to find a good
solution. Still, we need one …

Sorry if I have not been clear enough. I see only two
(1) The statu quo, closing the PR as WONTFIX

How is that even an option?!? gfortran accepts invalid code. What's
wrong with fixing that?
If your aim is to keep "the status quo" (read: a compiler with
gazillions of bugs), why don't we just shut down bugzilla altogether?

Post by Dominique d'Humières
(2) Introduce a new option, for instance
-fshort-circuit (default) for the short-circuit evaluation
-fno-short-circuit to force the evaluation of both expressions.

For starters, no one will find a bug in his code if it requires
-fsome-obscure-option-that-nobody-knows.
Also I could ask the innocent question why -fshort-circuit should be
the default, but I guess I won't bother to go there.

I think both of you are right to some degree. I think it is a (good) custom
in GCC to couple major optimizations with its own option flag. And to activate
it for some -Ox levels.
Because IMHO it is just this, a bog ordinary optimization.
And usually, optimizations which obfuscate the control flow and degrade debug-ability
are deactivated at level -O0.
Why can't this be handled just like any other optimization?

Cheers,
Manfred


PS: left-to-right short-circuiting is very handy, indeed. But I think it would be
wrong to nail down some behavior in gfortran which is not mandated by the standards.
I rather would prefer to aggressively implement some explicitly non-standard .ANDELSE.
(or similar) syntax and not to wait for the committees. FORTRAN has a long tradition
of compilers driving progress and standards to follow, after all...

Post by Janus Weil
I still find it deeply unsatisfying that gfortran is incompatible with
other compilers on a very fundamental level. And I find it very
disturbing that no one even cares. Anyway, I'm kinda through with this
whole issue. I guess I'll just stop caring as well.
Goodbye ...

[Janus Weil]

Oh, look, a reasonable person! Where have you been hiding all this
time? And is there more of your kind in that place? ;D

Post by Manfred Schwarb
Hi Janus, hi Dominique,
I did not raise my voice so far, as it is not so terribly relevant I guess,
nevertheless...

Post by Janus Weil

Post by Dominique d'Humières

Post by Janus Weil
It's so easy to scream "bullshit", and so hard to find a good
solution. Still, we need one …

Sorry if I have not been clear enough. I see only two
(1) The statu quo, closing the PR as WONTFIX

How is that even an option?!? gfortran accepts invalid code. What's
wrong with fixing that?
If your aim is to keep "the status quo" (read: a compiler with
gazillions of bugs), why don't we just shut down bugzilla altogether?

Post by Dominique d'Humières
(2) Introduce a new option, for instance
-fshort-circuit (default) for the short-circuit evaluation
-fno-short-circuit to force the evaluation of both expressions.

For starters, no one will find a bug in his code if it requires
-fsome-obscure-option-that-nobody-knows.
Also I could ask the innocent question why -fshort-circuit should be
the default, but I guess I won't bother to go there.

I think both of you are right to some degree. I think it is a (good) custom
in GCC to couple major optimizations with its own option flag. And to activate
it for some -Ox levels.
Because IMHO it is just this, a bog ordinary optimization.
And usually, optimizations which obfuscate the control flow and degrade debug-ability
are deactivated at level -O0.
Why can't this be handled just like any other optimization?
Cheers,
Manfred
PS: left-to-right short-circuiting is very handy, indeed. But I think it would be
wrong to nail down some behavior in gfortran which is not mandated by the standards.
I rather would prefer to aggressively implement some explicitly non-standard .ANDELSE.
(or similar) syntax and not to wait for the committees. FORTRAN has a long tradition
of compilers driving progress and standards to follow, after all...

Post by Janus Weil
I still find it deeply unsatisfying that gfortran is incompatible with
other compilers on a very fundamental level. And I find it very
disturbing that no one even cares. Anyway, I'm kinda through with this
whole issue. I guess I'll just stop caring as well.
Goodbye ...

[Arjen Markus]

FWIW, I second Manfred's opinion. I have no strong preference to such
short-circuiting or against it, but I feel that putting the programmer
in control is the better option.

Regards,

Arjen

Post by Janus Weil
Oh, look, a reasonable person! Where have you been hiding all this
time? And is there more of your kind in that place? ;D

Post by Manfred Schwarb
Hi Janus, hi Dominique,
I did not raise my voice so far, as it is not so terribly relevant I guess,
nevertheless...

Post by Janus Weil

Post by Dominique d'Humières

Post by Janus Weil
It's so easy to scream "bullshit", and so hard to find a good
solution. Still, we need one …

Sorry if I have not been clear enough. I see only two
(1) The statu quo, closing the PR as WONTFIX

How is that even an option?!? gfortran accepts invalid code. What's
wrong with fixing that?
If your aim is to keep "the status quo" (read: a compiler with
gazillions of bugs), why don't we just shut down bugzilla altogether?

Post by Dominique d'Humières
(2) Introduce a new option, for instance
-fshort-circuit (default) for the short-circuit evaluation
-fno-short-circuit to force the evaluation of both expressions.

For starters, no one will find a bug in his code if it requires
-fsome-obscure-option-that-nobody-knows.
Also I could ask the innocent question why -fshort-circuit should be
the default, but I guess I won't bother to go there.

I think both of you are right to some degree. I think it is a (good) custom
in GCC to couple major optimizations with its own option flag. And to activate
it for some -Ox levels.
Because IMHO it is just this, a bog ordinary optimization.
And usually, optimizations which obfuscate the control flow and degrade debug-ability
are deactivated at level -O0.
Why can't this be handled just like any other optimization?
Cheers,
Manfred
PS: left-to-right short-circuiting is very handy, indeed. But I think it would be
wrong to nail down some behavior in gfortran which is not mandated by the standards.
I rather would prefer to aggressively implement some explicitly non-standard .ANDELSE.
(or similar) syntax and not to wait for the committees. FORTRAN has a long tradition
of compilers driving progress and standards to follow, after all...

Post by Janus Weil
I still find it deeply unsatisfying that gfortran is incompatible with
other compilers on a very fundamental level. And I find it very
disturbing that no one even cares. Anyway, I'm kinda through with this
whole issue. I guess I'll just stop caring as well.
Goodbye ...

[Janus Weil]

Hi all,

since there seems to be significant support for this after all, I just
rebased the last patch version and pushed it to github (I sincerely
hope gcc will officially make the switch to git very soon):

https://github.com/janusw/gcc/commit/35aae880ff2b3d4a9f6a9c821376fb510202961c

This is still the most reasonable approach I can see. And I'd still be
grateful for (technical) comments on the patch.

What's the procedure in GCC for dealing with such controversial
patches? The procedure for simple cases is: The patch author asks for
review, an authorized reviewer/maintainer approves it or asks for
improvements. What happens if that fails? Is there any rule on how
decisions are to be made in the non-trivial case?

Cheers,
Janus

Post by Arjen Markus
FWIW, I second Manfred's opinion. I have no strong preference to such
short-circuiting or against it, but I feel that putting the programmer
in control is the better option.
Regards,
Arjen

Post by Janus Weil
Oh, look, a reasonable person! Where have you been hiding all this
time? And is there more of your kind in that place? ;D

Post by Manfred Schwarb
Hi Janus, hi Dominique,
I did not raise my voice so far, as it is not so terribly relevant I guess,
nevertheless...

Post by Janus Weil

Post by Dominique d'Humières

Post by Janus Weil
It's so easy to scream "bullshit", and so hard to find a good
solution. Still, we need one …

Sorry if I have not been clear enough. I see only two
(1) The statu quo, closing the PR as WONTFIX

How is that even an option?!? gfortran accepts invalid code. What's
wrong with fixing that?
If your aim is to keep "the status quo" (read: a compiler with
gazillions of bugs), why don't we just shut down bugzilla altogether?

Post by Dominique d'Humières
(2) Introduce a new option, for instance
-fshort-circuit (default) for the short-circuit evaluation
-fno-short-circuit to force the evaluation of both expressions.

For starters, no one will find a bug in his code if it requires
-fsome-obscure-option-that-nobody-knows.
Also I could ask the innocent question why -fshort-circuit should be
the default, but I guess I won't bother to go there.

I think both of you are right to some degree. I think it is a (good) custom
in GCC to couple major optimizations with its own option flag. And to activate
it for some -Ox levels.
Because IMHO it is just this, a bog ordinary optimization.
And usually, optimizations which obfuscate the control flow and degrade debug-ability
are deactivated at level -O0.
Why can't this be handled just like any other optimization?
Cheers,
Manfred
PS: left-to-right short-circuiting is very handy, indeed. But I think it would be
wrong to nail down some behavior in gfortran which is not mandated by the standards.
I rather would prefer to aggressively implement some explicitly non-standard .ANDELSE.
(or similar) syntax and not to wait for the committees. FORTRAN has a long tradition
of compilers driving progress and standards to follow, after all...

Post by Janus Weil
I still find it deeply unsatisfying that gfortran is incompatible with
other compilers on a very fundamental level. And I find it very
disturbing that no one even cares. Anyway, I'm kinda through with this
whole issue. I guess I'll just stop caring as well.
Goodbye ...

[Janus Weil]

Post by Janus Weil
Hi all,
since there seems to be significant support for this after all, I just
rebased the last patch version and pushed it to github (I sincerely
https://github.com/janusw/gcc/commit/35aae880ff2b3d4a9f6a9c821376fb510202961c
This is still the most reasonable approach I can see. And I'd still be
grateful for (technical) comments on the patch.
What's the procedure in GCC for dealing with such controversial
patches? The procedure for simple cases is: The patch author asks for
review, an authorized reviewer/maintainer approves it or asks for
improvements. What happens if that fails? Is there any rule on how
decisions are to be made in the non-trivial case?

Re-reading the thread, I noticed that I haven't actually gotten a
clear "yes" or "no" from anyone with review privileges for the Fortran
front-end.

So, I'll just continue with the usual procedure: The patch above
regtests cleanly on x86_64-linux-gnu and provides a clear improvement
regarding the standard-conformance of the GNU Fortan compiler. Ok to
commit to trunk?

Cheers,
Janus

[Jerry DeLisle]

Post by Janus Weil

Post by Janus Weil
Hi all,
since there seems to be significant support for this after all, I just
rebased the last patch version and pushed it to github (I sincerely
https://github.com/janusw/gcc/commit/35aae880ff2b3d4a9f6a9c821376fb510202961c
This is still the most reasonable approach I can see. And I'd still be
grateful for (technical) comments on the patch.
What's the procedure in GCC for dealing with such controversial
patches? The procedure for simple cases is: The patch author asks for
review, an authorized reviewer/maintainer approves it or asks for
improvements. What happens if that fails? Is there any rule on how
decisions are to be made in the non-trivial case?

Re-reading the thread, I noticed that I haven't actually gotten a
clear "yes" or "no" from anyone with review privileges for the Fortran
front-end.
So, I'll just continue with the usual procedure: The patch above
regtests cleanly on x86_64-linux-gnu and provides a clear improvement
regarding the standard-conformance of the GNU Fortan compiler. Ok to
commit to trunk?
Cheers,
Janus

Yes, OK for trunk.

Thanks for the effort.

Jerry

[Janus Weil]

Post by Jerry DeLisle

Post by Janus Weil

Post by Janus Weil
Hi all,
since there seems to be significant support for this after all, I just
rebased the last patch version and pushed it to github (I sincerely
https://github.com/janusw/gcc/commit/35aae880ff2b3d4a9f6a9c821376fb510202961c
This is still the most reasonable approach I can see. And I'd still be
grateful for (technical) comments on the patch.
What's the procedure in GCC for dealing with such controversial
patches? The procedure for simple cases is: The patch author asks for
review, an authorized reviewer/maintainer approves it or asks for
improvements. What happens if that fails? Is there any rule on how
decisions are to be made in the non-trivial case?

Re-reading the thread, I noticed that I haven't actually gotten a
clear "yes" or "no" from anyone with review privileges for the Fortran
front-end.
So, I'll just continue with the usual procedure: The patch above
regtests cleanly on x86_64-linux-gnu and provides a clear improvement
regarding the standard-conformance of the GNU Fortan compiler. Ok to
commit to trunk?

Yes, OK for trunk.

Thanks, Jerry. I'll wait until Saturday to allow for further comments ...

Cheers,
Janus

[Jerry DeLisle]

Post by Janus Weil

Post by Jerry DeLisle

Post by Janus Weil

Post by Janus Weil
Hi all,
since there seems to be significant support for this after all, I just
rebased the last patch version and pushed it to github (I sincerely
https://github.com/janusw/gcc/commit/35aae880ff2b3d4a9f6a9c821376fb510202961c
This is still the most reasonable approach I can see. And I'd still be
grateful for (technical) comments on the patch.
What's the procedure in GCC for dealing with such controversial
patches? The procedure for simple cases is: The patch author asks for
review, an authorized reviewer/maintainer approves it or asks for
improvements. What happens if that fails? Is there any rule on how
decisions are to be made in the non-trivial case?

Re-reading the thread, I noticed that I haven't actually gotten a
clear "yes" or "no" from anyone with review privileges for the Fortran
front-end.
So, I'll just continue with the usual procedure: The patch above
regtests cleanly on x86_64-linux-gnu and provides a clear improvement
regarding the standard-conformance of the GNU Fortan compiler. Ok to
commit to trunk?

Yes, OK for trunk.

Thanks, Jerry. I'll wait until Saturday to allow for further comments ...
Cheers,
Janus

I dont see any need to wait, this topic has been beat to death, Of
course, your choice.

Jerry

[Janus Weil]

Am Fr., 10. Aug. 2018 um 02:38 Uhr schrieb Jerry DeLisle

Post by Jerry DeLisle

Post by Janus Weil

Post by Jerry DeLisle

Post by Janus Weil

Post by Janus Weil
Hi all,
since there seems to be significant support for this after all, I just
rebased the last patch version and pushed it to github (I sincerely
https://github.com/janusw/gcc/commit/35aae880ff2b3d4a9f6a9c821376fb510202961c
This is still the most reasonable approach I can see. And I'd still be
grateful for (technical) comments on the patch.
What's the procedure in GCC for dealing with such controversial
patches? The procedure for simple cases is: The patch author asks for
review, an authorized reviewer/maintainer approves it or asks for
improvements. What happens if that fails? Is there any rule on how
decisions are to be made in the non-trivial case?

Re-reading the thread, I noticed that I haven't actually gotten a
clear "yes" or "no" from anyone with review privileges for the Fortran
front-end.
So, I'll just continue with the usual procedure: The patch above
regtests cleanly on x86_64-linux-gnu and provides a clear improvement
regarding the standard-conformance of the GNU Fortan compiler. Ok to
commit to trunk?

Yes, OK for trunk.

Thanks, Jerry. I'll wait until Saturday to allow for further comments ...

I dont see any need to wait, this topic has been beat to death, Of
course, your choice.

Alright then. Committed to trunk as r263471.

Cheers,
Janus

[Janus Weil]

Post by Manfred Schwarb
PS: left-to-right short-circuiting is very handy, indeed. But I think it would be
wrong to nail down some behavior in gfortran which is not mandated by the standards.
I rather would prefer to aggressively implement some explicitly non-standard .ANDELSE.
(or similar) syntax and not to wait for the committees. FORTRAN has a long tradition
of compilers driving progress and standards to follow, after all...

Absolutely agree. I just opened the following PR:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86893

Cheers,
Janus