public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Thomas Koenig <tkoenig@netcologne.de>
Cc: "fortran@gcc.gnu.org" <fortran@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [patch, fortran] Fix common subexpression elimination with IEEE rounding (PR108329)
Date: Mon, 9 Jan 2023 13:59:57 +0100	[thread overview]
Message-ID: <CAFiYyc1Ed_=N+xuv2n=Dh0nARbm2YPwdOf_PTpos2LneRx10Yw@mail.gmail.com> (raw)
In-Reply-To: <a13d9941-c8f2-8b92-c47e-85709581cf87@netcologne.de>

On Sun, Jan 8, 2023 at 5:21 PM Thomas Koenig <tkoenig@netcologne.de> wrote:
>
> Hi Richard,
>
> >> Am 08.01.2023 um 14:31 schrieb Paul Richard Thomas via Fortran <fortran@gcc.gnu.org>:
> >>
> >> Hi Thomas,
> >>
> >> Following your off-line explanation that the seemingly empty looking
> >> assembly line forces an effective reload from memory, all is now clear.
> >
> > It’s not a full fix (for register vars) and it’s ‚superior‘ to the call itself only because asm handling is implemented in a rather stupid way in the Alias oracle.  So I don’t think this is a „fix“ at all.
>
> There are no register variables in Fortran, this is Fortran FE only,
> and it is a fix in the sense that correct code is no longer miscompiled.

It's a quite big hammer and the fact that it "works" is just luck and
the fact that the memory barrier implied by the ieee_set_rouding_mode
does not is because by-reference passed arguments are marked by
the frontend so they can be CSEd since memory barriers may not
affect them.

As said, the fact that this "works" is just because we're lazy on GIMPLE:

/* If the statement STMT may clobber the memory reference REF return true,
   otherwise return false.  */

bool
stmt_may_clobber_ref_p_1 (gimple *stmt, ao_ref *ref, bool tbaa_p)
{
...
  else if (gimple_code (stmt) == GIMPLE_ASM)
    return true;

> There's a FIXME in the code pointing to the relevant PR precisely
> because I think that this is less than elegant (as do you, obviously).
> Do you have other suggestions how to implement this?  If PR 34678
> is solved, this would probably provide a mechanism that we could
> simply re-use.

There is no reliable way to get this correct at the moment and if there
were good and easy ways to get this working they'd be implemented already.

Richard.

> Best regards
>
>         Thomas

  reply	other threads:[~2023-01-09 13:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-07 15:46 Thomas Koenig
2023-01-08 13:31 ` Paul Richard Thomas
2023-01-08 15:53   ` Richard Biener
2023-01-08 16:21     ` Thomas Koenig
2023-01-09 12:59       ` Richard Biener [this message]
2023-01-09 15:27         ` Thomas Koenig

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='CAFiYyc1Ed_=N+xuv2n=Dh0nARbm2YPwdOf_PTpos2LneRx10Yw@mail.gmail.com' \
    --to=richard.guenther@gmail.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@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).