public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Fritz Reese <fritzoreese@gmail.com>
To: Janus Weil <janus@gcc.gnu.org>
Cc: adam@aphirst.karoo.co.uk, fortran <fortran@gcc.gnu.org>
Subject: Re: [Patch, Fortran] PR 57160: short-circuit IF only with -ffrontend-optimize
Date: Mon, 23 Jul 2018 21:06:00 -0000	[thread overview]
Message-ID: <CAE4aFAnb7eMS0YmhZixTenZhGUDXyFdPnAei=FgB1DmFugnNAw@mail.gmail.com> (raw)
In-Reply-To: <CAKwh3qhGmq-o1c9rHzNfiHnqV-Gbp2_iFZopPd7KE1BeBWobdQ@mail.gmail.com>

On Mon, Jul 23, 2018 at 1:11 PM Janus Weil <janus@gcc.gnu.org> wrote:
>
> Hi Adam,
>
> 2018-07-23 9:40 GMT+02:00 Adam Hirst <adam@aphirst.karoo.co.uk>:
[...]
> > One thing I'm not seeing in the original discussion was whether or not
> > this should also count for -Og
>
> good point!
>
>
> > which certainly in my experience is
> > (paired with -g) a common debug setting.
>
> Agreed. I actually use it myself.
>
>
> > 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.

> 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

  reply	other threads:[~2018-07-23 21:06 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-20 21:38 Janus Weil
2018-07-23  7:40 ` Adam Hirst
2018-07-23 17:11   ` Janus Weil
2018-07-23 21:06     ` Fritz Reese [this message]
2018-07-24 18:46       ` Janus Weil
2018-07-24 19:49         ` Janus Weil
2018-07-24 19:50         ` Thomas Koenig
2018-07-24 20:14           ` Janus Weil
2018-07-25 20:05             ` Janus Weil
2018-07-25 20:59               ` Nicolas Koenig
2018-07-25 21:01                 ` Thomas Koenig
2018-07-25 21:31                 ` Janus Weil
2018-07-24  9:12 Dominique d'Humières
2018-07-24 13:46 ` Janus Weil
2018-07-24 15:42   ` Janne Blomqvist
2018-07-24 16:18     ` Janus Weil
2018-07-27 16:42 Dominique d'Humières
2018-07-28  8:03 ` Janus Weil
2018-08-01 20:46   ` Janus Weil
2018-08-06 20:59     ` Janus Weil
2018-08-07 10:11       ` Dominique d'Humières
2018-08-07 17:14         ` Janus Weil
2018-08-07 22:21           ` Thomas König
2018-08-08  0:17             ` William Clodius
2018-08-08  5:41             ` Janus Weil
2018-08-08  8:35           ` Manfred Schwarb
2018-08-08 11:23             ` Janus Weil
2018-08-08 11:29               ` Arjen Markus
2018-08-08 18:21                 ` Janus Weil
2018-08-08 20:48                   ` Janus Weil
2018-08-09  1:05                     ` Jerry DeLisle
2018-08-09 21:14                       ` Janus Weil
2018-08-10  0:38                         ` Jerry DeLisle
2018-08-10 14:14                           ` Janus Weil
2018-08-08 19:50             ` Janus Weil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAE4aFAnb7eMS0YmhZixTenZhGUDXyFdPnAei=FgB1DmFugnNAw@mail.gmail.com' \
    --to=fritzoreese@gmail.com \
    --cc=adam@aphirst.karoo.co.uk \
    --cc=fortran@gcc.gnu.org \
    --cc=janus@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).