Re: [PATCH] rs6000: replace '(const_int 0)' to 'unspec:BLK [(const_int 0)]' for stack_tie

[Jiufu Guo <guojiufu@linux.ibm.com>]

Hi,

Segher Boessenkool <segher@kernel.crashing.org> writes:

> 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.h

The size is possible to be zero.  No asm code needs to
be generated for "set 'const_int 0' to zero size memory"".
stack_tie does not generate any real code.  It seems ok :)

While, it may not be zero size mem.  This may be a concern.
This is one reason that I would like to have an unspec_tie.

Another reason is unspec:blk is used but various ports :) 

>
>> 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!

The semantic that I trying to achieve is "this is a special
insn, not only a normal set to unknown size mem".

As you explained before on 'unspec:DI', the unspec would
just decorate the set_src part: something DI value with
machine-specific operation.

But, since 'tie_operand' is checked for this insn.
If 'tie_operand' checks UNPSEC_TIE, then the insn
with UNPSEC_TIE is 'a special insn'.  Or interpret
the semantic of this insn as: this insn stack_ite
indicates "set/operate a zero size block".

Does this make sense?

>
>> 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 ;-)

Thanks a lot for your comments!

BR,
Jeff (Jiufu Guo)

>
>
> Segher