Re: [PATCH 1/2] Using pli(paddi) and rotate to build 64bit constants

[Segher Boessenkool <segher@kernel.crashing.org>]

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