Hi,
"Kewen.Lin" <li...@linux.ibm.com> writes: > Hi, > > on 2023/11/15 11:02, Jiufu Guo wrote: >> Hi, >> >> For constants with 16bit values, 'li or lis' can be used to generate >> the value. For 34bit constant, 'pli' is ok to generate the value. >> For example: 0x6666666666666666ULL, "pli 3,1717986918; rldimi 3,3,32,0" >> can be used. > > Since now if emit_move_insn with a 34bit constant, it's already adopting > pli. So it's not obvious to the readers why we want this change, I think > you should probably state the reason here explicitly, like in function > rs6000_emit_set_long_const it's possible to recursively call itself without > invoking emit_move_insn, then it can result in sub-optimal constant build ... > And for the testing I prefer to have a dedicated test case for it, like > extracting function msk66 from pr93012.c and checking its generated assembly > has pli but not lis and ori on Power10 and up. I would update the message to make it clear. Thanks so much for your suggestions! BR, Jeff (Jiufu Guo) > > The others look good to me. Thanks! > > BR, > Kewen > >> >> Compare with previous: >> https://gcc.gnu.org/pipermail/gcc-patches/2023-October/634196.html >> This verion updates a testcase to cover this functionality. >> >> Bootstrap®test pass on ppc64{,le}. >> Is this ok for trunk? >> >> BR, >> Jeff (Jiufu Guo) >> >> gcc/ChangeLog: >> >> * config/rs6000/rs6000.cc (rs6000_emit_set_long_const): Add code to use >> pli for 34bit constant. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.target/powerpc/pr93012.c: Update to check pli. >> >> --- >> gcc/config/rs6000/rs6000.cc | 9 +++++++++ >> gcc/testsuite/gcc.target/powerpc/pr93012.c | 1 + >> 2 files changed, 10 insertions(+) >> >> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc >> index ba40dd6eee4..b277c52687b 100644 >> --- a/gcc/config/rs6000/rs6000.cc >> +++ b/gcc/config/rs6000/rs6000.cc >> @@ -10504,6 +10504,15 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT >> c, int *num_insns) >> return; >> \ >> } >> >> + if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (c)) >> + { >> + /* li/lis/pli */ >> + ADJUST_INSN_NUM_AND_RET (1); >> + >> + emit_move_insn (dest, GEN_INT (c)); >> + return; >> + } >> + >> if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000)) >> || (ud4 == 0 && ud3 == 0 && ud2 == 0 && !(ud1 & 0x8000))) >> { >> diff --git a/gcc/testsuite/gcc.target/powerpc/pr93012.c >> b/gcc/testsuite/gcc.target/powerpc/pr93012.c >> index 4f764d0576f..a07ff764bbf 100644 >> --- a/gcc/testsuite/gcc.target/powerpc/pr93012.c >> +++ b/gcc/testsuite/gcc.target/powerpc/pr93012.c >> @@ -10,4 +10,5 @@ unsigned long long mskh1() { return 0xffff9234ffff9234ULL; >> } >> unsigned long long mskl1() { return 0x2bcdffff2bcdffffULL; } >> unsigned long long mskse() { return 0xffff1234ffff1234ULL; } >> >> +/* { dg-final { scan-assembler-times {\mpli\M} 4 { target has_arch_pwr10 }} >> } */ >> /* { dg-final { scan-assembler-times {\mrldimi\M} 7 } } */
