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

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

Hi Segher, David,

David Edelsohn <dje.gcc@gmail.com> writes:

> On Tue, Jun 13, 2023 at 2:16 PM Segher Boessenkool
> <segher@kernel.crashing.org> wrote:
>>
>> Hi!
>>
>> On Tue, Jun 13, 2023 at 10:15:49AM +0800, Jiufu Guo wrote:
>> > David Edelsohn <dje.gcc@gmail.com> writes:
>> > >
>> > > This definitely seems to be a better solution.
>> > >
>> > > The TARGET_CONST_ANCHOR change should not be part of this patch.  Also
>> > > there is no ChangeLog for the patch.
>> >
>> > Thanks a lot for your quick review!! And sorry for the sending this patch
>> > in a hurry.  I would update the patch accordingly.
>>
>> > > This generally looks correct and consistent with other ports. I want
>> > > to give Segher a chance to double check it, if he wishes.
>>
>> The documentation is very clear that the only thing for which you can
>> have BLKmode is "mem".  Not unspec, only "mem".
>>
>> Let's not do this.  The existing code has clear and obvious semantics,
>> which is documented as well -- there is no reason to make it worse in
>> every respect.

Thanks for all your insight comments!

Yeap, while "unspec:BLK" is very widely used already on various ports.
And it seems a few place is using BLKmode without strictly align with
the document :( It would not be very good thing, but maybe no better
solutions.

For existing code "set (mem/c:BLK (reg/f:DI 1 1) (const_int 0 [0])"
Since it is a set, the operand set_src should be valid for
the mode of the set_dest. While set_src is 'const_int 0'.
And this 'set' may be mis-readed as 'a memory is zeroed' or
'no-op to a mem'. Using unspec here would just say this is an special
operation instead a normal 'const_int 0'.

BR,
Jeff (Jiufu Guo)

>
> Segher,
>
> Unfortunately, GCC now is inconsistent and this response is incorrect.
> The documentation is out of date or was ignored and the "facts on the
> ground" contradict your review.
>
> Yes, (const_int 0) is supposed to be a general no-op and BLKmode only
> is supposed to be used for MEM, but other major targets (arm, aarch64,
> riscv, s390) all use unspec:BLK and specifically UNSPEC_TIE.  rs6000
> is the only port that does not follow this convention.  The middle-end
> has adapted to the behavior of all of the other targets, whether that
> conformed to the documentation or not.  The rs6000 port needs to be
> fixed and Jiufu's approach is the correct one, consistent with all
> other targets for stack tie.  If the documentation differs, the
> documentation needs to be updated, not a different approach for the
> rs6000 port.  Jiufu's patch is correct.
>
> Thanks, David