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