Hi Segher, David,
David Edelsohn <dje....@gmail.com> writes: > On Tue, Jun 13, 2023 at 2:16 PM Segher Boessenkool > <seg...@kernel.crashing.org> wrote: >> >> Hi! >> >> On Tue, Jun 13, 2023 at 10:15:49AM +0800, Jiufu Guo wrote: >> > David Edelsohn <dje....@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
