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 :-) )
> 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)?
> >> +/* 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 again,
Segher
