Message ID | 20231025020008.2256911-2-guojiufu@linux.ibm.com |
---|---|
State | New |
Headers | show |
Series | [1/3] rs6000: update num_insns_constant for 2 insns | expand |
on 2023/10/25 10:00, 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. > > 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/config/rs6000/rs6000.cc | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index b23ff3d7917..4690384cdbe 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -10530,7 +10530,11 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c) > ud3 = (c >> 32) & 0xffff; > ud4 = (c >> 48) & 0xffff; > > - if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000)) > + if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (c)) > + { > + emit_move_insn (dest, GEN_INT (c)); > + } Nit: unexpected formatting, no {} needed. Is there any test case justifying this change? I think only one "li" or "lis" beats "pli" since the latter is a prefixed insn, it puts more burdens on insn decoding. BR, Kewen > + else if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000)) > || (ud4 == 0 && ud3 == 0 && ud2 == 0 && ! (ud1 & 0x8000))) > emit_move_insn (dest, GEN_INT (sext_hwi (ud1, 16))); >
Hi, "Kewen.Lin" <linkw@linux.ibm.com> writes: > on 2023/10/25 10:00, 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. >> >> 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/config/rs6000/rs6000.cc | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc >> index b23ff3d7917..4690384cdbe 100644 >> --- a/gcc/config/rs6000/rs6000.cc >> +++ b/gcc/config/rs6000/rs6000.cc >> @@ -10530,7 +10530,11 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c) >> ud3 = (c >> 32) & 0xffff; >> ud4 = (c >> 48) & 0xffff; >> >> - if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000)) >> + if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (c)) >> + { >> + emit_move_insn (dest, GEN_INT (c)); >> + } > > Nit: unexpected formatting, no {} needed. > > Is there any test case justifying this change? Great catch! pr93012.c could cover this implicitly, but it only be changed in the [PATCH 3/3]. I would add a new case for this in this patch. > I think only one "li" or "lis" > beats "pli" since the latter is a prefixed insn, it puts more burdens on insn > decoding. Yes! Good news is "emit_move_insn (dest, GEN_INT (c));" is able to support "li, lis and pli". The "mov" would match/generate the best one. Thanks for your quick review and very helpful comments! BR, Jeff (Jiufu Guo) > > BR, > Kewen > >> + else if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000)) >> || (ud4 == 0 && ud3 == 0 && ud2 == 0 && ! (ud1 & 0x8000))) >> emit_move_insn (dest, GEN_INT (sext_hwi (ud1, 16))); >>
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index b23ff3d7917..4690384cdbe 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -10530,7 +10530,11 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c) ud3 = (c >> 32) & 0xffff; ud4 = (c >> 48) & 0xffff; - if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000)) + if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (c)) + { + emit_move_insn (dest, GEN_INT (c)); + } + else if ((ud4 == 0xffff && ud3 == 0xffff && ud2 == 0xffff && (ud1 & 0x8000)) || (ud4 == 0 && ud3 == 0 && ud2 == 0 && ! (ud1 & 0x8000))) emit_move_insn (dest, GEN_INT (sext_hwi (ud1, 16)));