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

[Janus Weil]

2018-07-23 23:05 GMT+02:00 Fritz Reese <fritzoreese@gmail.com>:
> On Mon, Jul 23, 2018 at 1:11 PM Janus Weil <janus@gcc.gnu.org> wrote:
>> 2018-07-23 9:40 GMT+02:00 Adam Hirst <adam@aphirst.karoo.co.uk>:
>> > 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?
>>
>
> 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).


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


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


> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr57160_v2.diff
Type: text/x-patch
Size: 3839 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20180724/5f398c0f/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: short_circuiting_2.f90
Type: text/x-fortran
Size: 475 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20180724/5f398c0f/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: short_circuiting_3.f90
Type: text/x-fortran
Size: 471 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/fortran/attachments/20180724/5f398c0f/attachment-0002.bin>