public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Jiufu Guo <guojiufu@linux.ibm.com>
Cc: gcc-patches@gcc.gnu.org, dje.gcc@gmail.com, linkw@gcc.gnu.org,
	bergner@linux.ibm.com, rguenther@suse.de,
	richard.sandiford@arm.com, jeffreyalaw@gmail.com
Subject: Re: [PATCH] rs6000: replace '(const_int 0)' to 'unspec:BLK [(const_int 0)]' for stack_tie
Date: Wed, 14 Jun 2023 10:15:33 -0500	[thread overview]
Message-ID: <20230614151533.GW19790@gate.crashing.org> (raw)
In-Reply-To: <7no7liadru.fsf@ltcden2-lp1.aus.stglabs.ibm.com>

Hi!

On Wed, Jun 14, 2023 at 12:06:29PM +0800, Jiufu Guo wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> I'm also thinking about other solutions:
> 1. "set (mem/c:BLK (reg/f:DI 1 1) (const_int 0 [0])"
>   This is the existing pattern.  It may be read as an action
>   to clean an unknown-size memory block.

Including a size zero memory block, yes.  BLKmode was originally to do
things like bcopy (before modern names like memcpy were more usually
used), and those very much need size zero as well.

> 2. "set (mem/c:BLK (reg/f:DI 1 1) unspec:blk (const_int 0 [0])
> UNSPEC_TIE".
>   Current patch is using this one.

What would be the semantics of that?  Just the same as the current stuff
I'd say, or less?  It cannot be more!

> 3. "set (mem/c:DI (reg/f:DI 1 1) unspec:DI (const_int 0 [0])
> UNSPEC_TIE".
>    This avoids using BLK on unspec, but using DI.

And is incorrect because of that.

> 4. "set (mem/c:BLK (reg/f:DI 1 1) unspec (const_int 0 [0])
> UNSPEC_TIE"
>    There is still a mode for the unspec.

It has VOIDmode here, which is incorrect.

> > On Tue, Jun 13, 2023 at 08:23:35PM +0800, Jiufu Guo wrote:
> >> +	  && XINT (SET_SRC (set), 1) == UNSPEC_TIE
> >> +	  && XVECEXP (SET_SRC (set), 0, 0) == const0_rtx);
> >
> > This makes it required that the operand of an UNSPEC_TIE unspec is a
> > const_int 0.  This should be documented somewhere.  Ideally you would
> > want no operand at all here, but every unspec has an operand.
> 
> Right!  Since checked UNSPEC_TIE arleady, we may not need to check
> the inner operand. Like " && XINT (SET_SRC (set), 1) == UNSPEC_TIE);".

Yes.  But we should write down somewhere (in a comment near the unspec
constant def for example) what the operand is -- so, "operand is usually
(const_int 0) because we have to put *something* there" or such.  The
clumsiness of this is enough for me to prefer some other solution
already ;-)


Segher

  parent reply	other threads:[~2023-06-14 15:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-13 12:23 Jiufu Guo
2023-06-13 12:48 ` Xi Ruoyao
2023-06-14  1:55   ` Jiufu Guo
2023-06-14  9:18     ` Xi Ruoyao
2023-06-14 15:05       ` Segher Boessenkool
2023-06-15  7:59         ` Jiufu Guo
2023-06-13 18:33 ` Segher Boessenkool
2023-06-14  4:06   ` Jiufu Guo
2023-06-14  7:59     ` Richard Biener
2023-06-14  9:04       ` Richard Sandiford
2023-06-14  9:22         ` Richard Biener
2023-06-14  9:43           ` Richard Sandiford
2023-06-14  9:52             ` Richard Biener
2023-06-14 10:02               ` Richard Sandiford
2023-06-14 16:08               ` Segher Boessenkool
2023-06-14 16:32           ` Segher Boessenkool
2023-06-14  9:29         ` Jiufu Guo
2023-06-14 16:38         ` Segher Boessenkool
2023-06-14  9:26       ` Jiufu Guo
2023-06-14 15:45         ` Segher Boessenkool
2023-06-14 15:38       ` Segher Boessenkool
2023-06-14 16:25         ` Richard Biener
2023-06-14 17:03           ` Segher Boessenkool
2023-06-14 15:15     ` Segher Boessenkool [this message]
2023-06-15  7:00       ` Jiufu Guo
2023-06-15 16:30         ` Segher Boessenkool
2023-06-16  2:24           ` Jiufu Guo
  -- strict thread matches above, loose matches on Subject: below --
2023-06-12 13:19 Jiufu Guo
2023-06-13  0:24 ` David Edelsohn
2023-06-13  2:15   ` Jiufu Guo
2023-06-13 18:14     ` Segher Boessenkool
2023-06-13 18:59       ` David Edelsohn
2023-06-14  3:00         ` Jiufu Guo

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=20230614151533.GW19790@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=bergner@linux.ibm.com \
    --cc=dje.gcc@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=guojiufu@linux.ibm.com \
    --cc=jeffreyalaw@gmail.com \
    --cc=linkw@gcc.gnu.org \
    --cc=rguenther@suse.de \
    --cc=richard.sandiford@arm.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).