public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Edelsohn <dje.gcc@gmail.com>
To: Hongtao Liu <crazylht@gmail.com>
Cc: Segher Boessenkool <segher@kernel.crashing.org>,
	Richard Biener <richard.guenther@gmail.com>,
	 Michael Meissner <meissner@linux.ibm.com>,
	Peter Bergner <bergner@linux.ibm.com>,
	 Bill Schmidt <wschmidt@linux.ibm.com>,
	GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Fix SFmode subreg of DImode and TImode
Date: Fri, 10 Sep 2021 10:08:27 -0400	[thread overview]
Message-ID: <CAGWvnym5W8Htaai7MgaJqPqowUFG4jhxO7-BLwewmCFxjqpQvQ@mail.gmail.com> (raw)
In-Reply-To: <CAMZc-byhGF7QVWs5DJGV9PydiKqrpuX4zFHuj+VqPFchpQCoFA@mail.gmail.com>

On Thu, Sep 9, 2021 at 11:03 PM Hongtao Liu <crazylht@gmail.com> wrote:
>
> On Fri, Sep 10, 2021 at 7:49 AM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
> >
> > On Thu, Sep 09, 2021 at 08:16:16AM +0200, Richard Biener wrote:
> > > > I think we should (longer term) get rid of the overloaded meanings and
> > > > uses of subregs.  One fairly simple thing is to make a new rtx code
> > > > "bit_cast" (or is there a nice short more traditional name for it?)
> > >
> > > But subreg _is_ bit_cast.
> >
> > It is not.  (subreg:M (reg:N) O) for O>0, little-endian, is not a
> > bit_cast.  It is taking a part of a register, or a single register from
> > a multi-register thing.  Paradoxicals are not bit-casts either.
> >
> > Subregs from or to (but not both) integer modes are generally bit_cast,
> > yeah.
> >
> > > What is odd to me is that a "disallowed" subreg
> > > like (subreg:SF (reg:TI ..) 0) magically becomes valid (in terms of
> > > validate_subreg) if you rewrite it as (subreg:SF (subreg:SI (reg:TI ..) 0) 0).
> > > Of course that's nested and invalid but just push the inner subreg to a
> > > new pseudo and the thing becomes valid.
> >
> > Bingo.
> >
> > And many targets have strange rules for bit-strings in which modes can
> > be used as bit-strings in which other modes, and at what offsets in
> > which registers.  Now perhaps none of that is optimal (I bet it isn't),
> > but changing this without a transition plan simply does not work.
> >
> > > > But that is not the core problem we had here.  The behaviour of the
> > > > generic parts of the compiler was changed, without testing if that
> > > > works on other targets but x86.  That is an understandable mistake, it
> > > > takes some experience to know where the morasses are.  But this change
> > > > should have been accompanied by testcases exercising the changed code.
> > > > We would have clearly seen there are issues then, simply by watching
> > > > gcc-testresults@ (and/or maintainers are on top of the test results
> > > > anyway).  Also, if there were testcases for this, we could have some
> > > > confidence that a change in this area is robust.
> > >
> > > Well, that only works if some maintainers that are familiar enough
> > > with all this chime in ;)
> >
> > Not really.  It works always.  And it works way better than the
> > pandemonium we now have with broken targets left and right.
> >
> > With testcases anyone can see if any specific target is broken here.
> >
> > > It's stage1 so it's understandable that some
> > > people (like me ...) are tyring to help people making progress even
> > > if that involves trying to decipher 30 years of GCC history in this
> > > area (without much success in the end as we see) ;)
> >
> > Yeah :-)  And my thanks to you and everyone involved for tackling this
> > problematic part of GCC, which has been neglected and patched over for
> > way too long.  But from that same history it follows that anything you
> > do not super carefully (with testing everywhere) will cause some serious
> Frankly, testing everywhere is too heavy a burden for developers,
> after all, everyone has a limited variety of machines, and may not be
> familiar with using  other targets' simulators.

Hongtao,

This is why the GNU Toolchain community sponsors the GCC Compile Farm.
Not every architecture is provided, but a good subset.  And there are
a few, powerful Linux on Power systems in the cluster, such as gcc135.

And the comments in the code warned of port-specific problems.  Even
if you cannot test the patch yourself, you should have publicly
requested that other port maintainers test the patch to shake out
problems.

The Tree-SSA passes are mostly target-independent, but RTL is
target-dependent, which can elicit widely different behavior in the
RTL passes.  When you make changes to RTL passes in the common parts
of the compiler, you must consider the impact on all targets.  GCC
explicitly supports a wide variety of architectures, ABIs and OSes.
All of the developers strive to ensure that changes don't adversely
affect any targets.

Thanks, David

  parent reply	other threads:[~2021-09-10 14:08 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-07  7:12 Michael Meissner
2021-09-07 23:07 ` Segher Boessenkool
2021-09-08  6:42   ` Richard Biener
2021-09-08 17:08     ` Segher Boessenkool
2021-09-08 17:50       ` David Edelsohn
2021-09-08 18:39       ` Richard Biener
2021-09-08 19:16         ` Segher Boessenkool
2021-09-09  1:23           ` Hongtao Liu
2021-09-09  1:31           ` Hongtao Liu
2021-09-09  6:16           ` Richard Biener
2021-09-09 22:53             ` Michael Meissner
2021-09-09 23:48             ` Segher Boessenkool
2021-09-10  3:09               ` Hongtao Liu
2021-09-10 10:54                 ` Richard Biener
2021-09-10 11:25                   ` Hongtao Liu
2021-09-10 12:34                     ` Hongtao Liu
2021-09-10 14:08                 ` David Edelsohn [this message]
2021-09-10 14:14                   ` Hongtao Liu
2021-09-10 15:32                 ` Segher Boessenkool
2021-09-10 10:53               ` Richard Biener
2021-09-10 15:05                 ` Segher Boessenkool
2021-09-13  9:03                   ` Richard Biener
2021-09-08 15:31   ` Michael Meissner
2021-09-09 21:59 ` Jim Wilson

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=CAGWvnym5W8Htaai7MgaJqPqowUFG4jhxO7-BLwewmCFxjqpQvQ@mail.gmail.com \
    --to=dje.gcc@gmail.com \
    --cc=bergner@linux.ibm.com \
    --cc=crazylht@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=meissner@linux.ibm.com \
    --cc=richard.guenther@gmail.com \
    --cc=segher@kernel.crashing.org \
    --cc=wschmidt@linux.ibm.com \
    /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).