Hi!

On Wed, Jun 14, 2023 at 12:06:29PM +0800, Jiufu Guo wrote:
> Segher Boessenkool <seg...@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

Reply via email to