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

[Janus Weil <janus at gcc dot gnu dot org>]

2018-07-24 11:12 GMT+02:00 Dominique d'Humières <dominiq@lps.ens.fr>:
> Hi Janus,
>
>> 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)
>> * …
>>
>
> Aren’t you confusing portability with validity?

I don' think so.


> 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.


>> 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.


> Please leave the default behavior (and test) as they are now.

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


> 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?!?


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

That's a valid point.

Cheers,
Janus