Hi Jeff, Thanks for the patch, some comments on nits are inline.
on 2022/9/1 11:24, Jiufu Guo wrote: > Hi, > > As mentioned in PR106550, since pli could support 34bits immediate, we could > use less instructions(3insn would be ok) to build 64bits constant with pli. > > For example, for constant 0x020805006106003, we could generate it with: > asm code1: > pli 9,101736451 (0x6106003) > sldi 9,9,32 > paddi 9,9, 2130000 (0x0208050) > > or asm code2: > pli 10, 2130000 > pli 9, 101736451 > rldimi 9, 10, 32, 0 > > Testing with simple cases as below, run them a lot of times: > f1.c > long __attribute__ ((noinline)) foo (long *arg,long *,long*) > { > *arg = 0x2351847027482577; > } > 5insns: base > pli+sldi+paddi: similar -0.08% > pli+pli+rldimi: faster +0.66% > > f2.c > long __attribute__ ((noinline)) foo (long *arg, long *arg2, long *arg3) > { > *arg = 0x2351847027482577; > *arg2 = 0x3257845024384680; > *arg3 = 0x1245abcef9240dec; > } > 5nisns: base > pli+sldi+paddi: faster +1.35% > pli+pli+rldimi: faster +5.49% > > f2.c would be more meaningful. Because 'sched passes' are effective for > f2.c, but 'scheds' do less thing for f1.c. > > Compare with previous patch: > https://gcc.gnu.org/pipermail/gcc-patches/2022-August/599525.html > This one updates code slightly and extracts changes on rs6000.md to a > seperate patch. > > This patch pass boostrap and regtest on ppc64le(includes p10). > Is it ok for trunk? > > BR, > Jeff(Jiufu) > > > PR target/106550 > > gcc/ChangeLog: > > * config/rs6000/rs6000.cc (rs6000_emit_set_long_const): Add 'pli' for > constant building. > > gcc/testsuite/ChangeLog: > > * gcc.target/powerpc/pr106550.c: New test. > > --- > gcc/config/rs6000/rs6000.cc | 39 +++++++++++++++++++++ > gcc/testsuite/gcc.target/powerpc/pr106550.c | 14 ++++++++ > 2 files changed, 53 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/powerpc/pr106550.c > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index df491bee2ea..1ccb2ff30a1 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -10181,6 +10181,45 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT > c) > gen_rtx_IOR (DImode, copy_rtx (temp), > GEN_INT (ud1))); > } > + else if (TARGET_PREFIXED) > + { > + /* pli 9,high32 + pli 10,low32 + rldimi 9,10,32,0. */ > + if (can_create_pseudo_p ()) > + { > + temp = gen_reg_rtx (DImode); > + rtx temp1 = gen_reg_rtx (DImode); > + emit_move_insn (copy_rtx (temp), GEN_INT ((ud4 << 16) | ud3)); > + emit_move_insn (copy_rtx (temp1), GEN_INT ((ud2 << 16) | ud1)); > + Nit: copy_rtx here seems not necessary, as both temp and temp1 are with CODE REG. The function copy_rtx returns the given rtx for code REG. > + emit_insn (gen_rotldi3_insert_3 (dest, temp, GEN_INT (32), temp1, > + GEN_INT (0xffffffff))); > + } > + > + /* pli 9,high32 + sldi 9,32 + paddi 9,9,low32. */ > + else > + { > + 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; > + The REGNO usage has asserted dest is with CODE REG, if it's always true I don't see why we need copy_rtx around. Or do I miss something? > + /* Use paddi for the low32 bits. */ > + if (ud2 != 0 && ud1 != 0 && can_use_paddi) > + emit_move_insn (dest, gen_rtx_PLUS (DImode, copy_rtx (dest), > + GEN_INT ((ud2 << 16) | ud1))); > + /* Use oris, ori for low32 bits. */ > + if (ud2 != 0 && (ud1 == 0 || !can_use_paddi)) > + emit_move_insn (ud1 != 0 ? copy_rtx (dest) : dest, > + gen_rtx_IOR (DImode, copy_rtx (dest), > + GEN_INT (ud2 << 16))); > + if (ud1 != 0 && (ud2 == 0 || !can_use_paddi)) > + emit_move_insn (dest, gen_rtx_IOR (DImode, copy_rtx (dest), > + GEN_INT (ud1))); > + } > + } > else > { > temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode); > diff --git a/gcc/testsuite/gcc.target/powerpc/pr106550.c > b/gcc/testsuite/gcc.target/powerpc/pr106550.c > new file mode 100644 > index 00000000000..c6f4116bb9a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/powerpc/pr106550.c > @@ -0,0 +1,14 @@ > +/* PR target/106550 */ > +/* { dg-options "-O2 -std=c99 -mdejagnu-cpu=power10" } */ > + Need to check power10_ok, like: /* { dg-require-effective-target power10_ok } */ Nit: -std=c99 is not needed? BR, Kewen