Hi,
Segher Boessenkool <seg...@kernel.crashing.org> writes:
> Hi!
>
> On Fri, Sep 02, 2022 at 02:56:21PM +0800, Jiufu Guo wrote:
>> >> + /* pli 9,high32 + sldi 9,32 + paddi 9,9,low32. */
>> >> + else
>> >> + {
>> >
>> > The comment goes here, in the block it refers to. Comments for a block
>> > are the first thing *in* the block.
>> OK, great! I like the format you sugguested here :-)
>
> It's the normal GCC style, not my invention :-)
>
>> >> + emit_move_insn (copy_rtx (dest), GEN_INT ((ud4 << 16) | ud3));
>> >> +
>> >> + emit_move_insn (copy_rtx (dest),
>> >> + gen_rtx_ASHIFT (DImode, copy_rtx (dest),
>> >> + GEN_INT (32)));
>> >> +
>> >> + bool can_use_paddi = REGNO (dest) != FIRST_GPR_REGNO;
>> >
>> > There should be a test that we so the right thing (or *a* right thing,
>> > anyway; a working thing; but hopefully a reasonably fast thing) for
>> > !can_use_paddi.
>> To catch this test point, we need let the splitter run after RA,
>> and register 0 happen to be the dest of an assignment.
>
> Or force the testcase to use r0 some other way. Well, "forcing" cannot
> be done, but we can probably encourage it (via a local register asm for
> example, or by tying the var to the output of an asm that is hard reg 0,
> or perhaps there are other ways as well :-) )
Specify register is very helpful here! Both below two cases could help
register 0 to be selected for a variable. Great! Thanks!
code1
-------
void __attribute__ ((noinline)) foo (unsigned long long *a)
{
register long long d asm("r0") = 0x1245abcef9240dec;
long long n;
asm("cntlzd %0, %1" : "=r"(n) : "r"(d));
*a = n;
}
code2
-----------
register long long d asm ("r0");
void __attribute__ ((noinline)) foo ()
{
d = 0x2351847027482577ULL;
}
>
>> I will add this test case in patch.
>> Is this ok? Any sugguestions?
>
> Sounds useful yes. Maybe describe the expected output in words as well
> (in the testcase, not in email)?
OK. Will update the patch accordingly.
>
>> >> +/* 3 insns for each constant: pli+sldi+paddi or pli+pli+rldimi.
>> >> + And 3 additional insns: std+std+blr: 9 insns totally. */
>> >> +/* { dg-final { scan-assembler-times {(?n)^\s+[a-z]} 9 } } */
>> >
>> > Also test the expected insns separately please? The std's (just with
>> > \mstd so it will catch all variations as well), the blr, the pli's and
>> > the rldimi etc.?
>> The reason of using "(?n)^\s+[a-z]" is to keep this test case pass no
>> matter the splitter running before or after RA.
>
> Ah. Some short comment in the testcase please?
Thanks. We can check individual instructions to check better asm
pli+pli+rldimi is generated, since the splitter could run in split1 pass.
BR,
Jeff(Jiufu)
>
> Thanks again,
>
>
> Segher
