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: Thomas Koenig <tkoenig@netcologne.de>,
	fortran <fortran@gcc.gnu.org>,
		gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [Patch, Fortran] PR 85599: warn about short-circuiting of logical expressions for non-pure functions
Date: Tue, 17 Jul 2018 15:19:00 -0000	[thread overview]
Message-ID: <CAE4aFAkq1nzzqB4E6g0_RoWYGtQg56XYNfLCSvafKr80cHDHDg@mail.gmail.com> (raw)
In-Reply-To: <CAKwh3qi_N7=+NTctsTTo7saNnNKZh4BYX2EJ=xcgSKzCRccYxQ@mail.gmail.com>

On Tue, Jul 17, 2018 at 10:32 AM Janus Weil <janus@gcc.gnu.org> wrote:
>
> 2018-07-17 9:52 GMT+02:00 Janus Weil <janus@gcc.gnu.org>:
> >> 2018-07-16 21:50 GMT+02:00 Thomas Koenig <tkoenig@netcologne.de>:
> >>> What I would suggest is to enable -Wfunction-eliminiation with
> >>> -Wextra and also use that for your new warning.
> >>
> >> Thanks for the comments. Makes sense. Updated patch attached.
> >
> >
> > Huh, after actually trying -Wfunction-elimination, I'm not so sure any
> > more if it really makes sense to throw the new warnings in there,
> > mainly for two reasons:
[...]
> > In other words: Does it make sense to tone down
> > -Wfunction-elimination, by only warning about impure functions?
>
> Here is an update of the patch which does that. Regtesting now ...
>
> Cheers,
> Janus

Would not this break the testcase function_optimize_5.f90 :

  write (unit=line, fmt='(4F7.2)') matmul(a,b)  & ! { dg-warning
"Removing call to function 'matmul'" }
       & + matmul(a,b)
  z = sin(x) + 2.0 + sin(x)  ! { dg-warning "Removing call to function 'sin'" }
  print *,z
  x = ext_func(a) + 23 + ext_func(a)
  print *,d,x
  z = element(x) + element(x) ! { dg-warning "Removing call to
function 'element'" }
  print *,z
  i = mypure(x) - mypure(x) ! { dg-warning "Removing call to function
'mypure'" }

The docs for -Wfunction-elimination would read:

> Warn if any calls to functions are eliminated by the optimizations
> enabled by the @option{-ffrontend-optimize} option.
> This option is implied by @option{-Wextra}.

However, with your patch, it should probably read something like "warn
if any calls to impure functions are eliminated..." Possibly with an
explicit remark indicating that pure functions are not warned.

Additionally:

$ ./contrib/check_GNU_style.sh pr85599_v6.diff

Lines should not exceed 80 characters.
33:+  if (e->expr_type == EXPR_FUNCTION && !gfc_pure_function (e,
&name) && !gfc_implicit_pure_function (e))
36:+         gfc_warning (OPT_Wfunction_elimination, "Removing call to
impure function %qs at %L", name, &(e->where));
38:+         gfc_warning (OPT_Wfunction_elimination, "Removing call to
impure function at %L", &(e->where));
201:+      if (f != last && !gfc_pure_function (f, &name) &&
!gfc_implicit_pure_function (f))

Blocks of 8 spaces should be replaced with tabs.
36:+         gfc_warning (OPT_Wfunction_elimination, "Removing call to
impure function %qs at %L", name, &(e->where));
38:+         gfc_warning (OPT_Wfunction_elimination, "Removing call to
impure function at %L", &(e->where));
230:+             with impure function as second operand.  */

Dot, space, space, new sentence.
79:+expression, if they do not contribute to the final result. For logical
82:+result of the expression can be established without them. However, since not

Have you considered using levels for the flag, such that the original
behavior of -Wfunction-elimination would be retained with e.g.
-Wfunction-elimination=2 whereas one could use
-Wfunction-elimination=1 (or similar) to omit warnings about impure
functions?

- Fritz

  reply	other threads:[~2018-07-17 15:19 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-11 21:06 Janus Weil
2018-07-12 19:43 ` Janus Weil
2018-07-13  8:03   ` Janus Weil
2018-07-15 20:39     ` Janus Weil
2018-07-15 20:57       ` Thomas Koenig
2018-07-16  8:07         ` Janus Weil
2018-07-16 19:51           ` Thomas Koenig
2018-07-17  5:08             ` Janus Weil
2018-07-17  7:52               ` Janus Weil
2018-07-17 14:32                 ` Janus Weil
2018-07-17 15:19                   ` Fritz Reese [this message]
2018-07-17 17:19                     ` Janus Weil
2018-07-17 17:34                       ` Thomas Koenig
2018-07-17 18:36                         ` Janus Weil
2018-07-17 18:55                           ` Fritz Reese
2018-07-17 19:21                             ` Janus Weil
2018-07-18 18:43                               ` Janus Weil
2018-07-12 11:17 Dominique d'Humières
2018-07-12 14:12 ` Janus Weil
2018-07-12 14:35   ` Dominique d'Humières
2018-07-12 14:55     ` Janus Weil
2018-07-12 19:53       ` Thomas Koenig
2018-07-12 20:03         ` 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=CAE4aFAkq1nzzqB4E6g0_RoWYGtQg56XYNfLCSvafKr80cHDHDg@mail.gmail.com \
    --to=fritzoreese@gmail.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=janus@gcc.gnu.org \
    --cc=tkoenig@netcologne.de \
    /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).