| Message ID | 20231025020008.2256911-1-guojiufu@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] rs6000: update num_insns_constant for 2 insns | expand |
Hi, on 2023/10/25 10:00, Jiufu Guo wrote: > Hi, > > Trunk gcc supports more constants to be built via two instructions: e.g. > "li/lis; xori/xoris/rldicl/rldicr/rldic". > And then num_insns_constant should also be updated. > Thanks for updating this. > Bootstrap & regtest pass ppc64{,le}. > Is this ok for trunk? > > BR, > Jeff (Jiufu Guo) > > gcc/ChangeLog: > > * config/rs6000/rs6000.cc (can_be_built_by_lilis_and_rldicX): New > function. > (num_insns_constant_gpr): Update to return 2 for more cases. > (rs6000_emit_set_long_const): Update to use > can_be_built_by_lilis_and_rldicX. > > --- > gcc/config/rs6000/rs6000.cc | 64 ++++++++++++++++++++++++------------- > 1 file changed, 41 insertions(+), 23 deletions(-) > > diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc > index cc24dd5301e..b23ff3d7917 100644 > --- a/gcc/config/rs6000/rs6000.cc > +++ b/gcc/config/rs6000/rs6000.cc > @@ -6032,6 +6032,9 @@ direct_return (void) > return 0; > } > > +static bool > +can_be_built_by_lilis_and_rldicX (HOST_WIDE_INT, int *, HOST_WIDE_INT *); > + > /* Helper for num_insns_constant. Calculate number of instructions to > load VALUE to a single gpr using combinations of addi, addis, ori, > oris, sldi and rldimi instructions. */ > @@ -6044,35 +6047,41 @@ num_insns_constant_gpr (HOST_WIDE_INT value) > return 1; > > /* constant loadable with addis */ > - else if ((value & 0xffff) == 0 > - && (value >> 31 == -1 || value >> 31 == 0)) > + if ((value & 0xffff) == 0 && (value >> 31 == -1 || value >> 31 == 0)) > return 1; > > /* PADDI can support up to 34 bit signed integers. */ > - else if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (value)) > + if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (value)) > return 1; > > - else if (TARGET_POWERPC64) > - { > - HOST_WIDE_INT low = sext_hwi (value, 32); > - HOST_WIDE_INT high = value >> 31; > + if (!TARGET_POWERPC64) > + return 2; > > - if (high == 0 || high == -1) > - return 2; > + HOST_WIDE_INT low = sext_hwi (value, 32); > + HOST_WIDE_INT high = value >> 31; > > - high >>= 1; > + if (high == 0 || high == -1) > + return 2; > > - if (low == 0 || low == high) > - return num_insns_constant_gpr (high) + 1; > - else if (high == 0) > - return num_insns_constant_gpr (low) + 1; > - else > - return (num_insns_constant_gpr (high) > - + num_insns_constant_gpr (low) + 1); > - } > + high >>= 1; > > - else > + HOST_WIDE_INT ud2 = (low >> 16) & 0xffff; > + HOST_WIDE_INT ud1 = low & 0xffff; > + if (high == -1 && ((!(ud2 & 0x8000) && ud1 == 0) || (ud1 & 0x8000))) > + return 2; > + if (high == 0 && (ud1 == 0 || (!(ud1 & 0x8000)))) > return 2; I was thinking that instead of enumerating all the cases in function rs6000_emit_set_long_const, if we can add one optional argument like "int* num_insns=nullptr" to function rs6000_emit_set_long_const, and when it's not nullptr, not emitting anything but update the count in rs6000_emit_set_long_const. It helps people remember to update num_insns when updating rs6000_emit_set_long_const in future, it's also more clear on how the number comes from. Does it sound good to you? BR, Kewen > + > + int shift; > + HOST_WIDE_INT mask; > + if (can_be_built_by_lilis_and_rldicX (value, &shift, &mask)) > + return 2; > + > + if (low == 0 || low == high) > + return num_insns_constant_gpr (high) + 1; > + if (high == 0) > + return num_insns_constant_gpr (low) + 1; > + return (num_insns_constant_gpr (high) + num_insns_constant_gpr (low) + 1); > } > > /* Helper for num_insns_constant. Allow constants formed by the > @@ -10492,6 +10501,18 @@ can_be_built_by_li_and_rldic (HOST_WIDE_INT c, int *shift, HOST_WIDE_INT *mask) > return false; > } > > +/* Combine the above checking functions for li/lis;rldicX. */ > + > +static bool > +can_be_built_by_lilis_and_rldicX (HOST_WIDE_INT c, int *shift, > + HOST_WIDE_INT *mask) > +{ > + return (can_be_built_by_li_lis_and_rotldi (c, shift, mask) > + || can_be_built_by_li_lis_and_rldicl (c, shift, mask) > + || can_be_built_by_li_lis_and_rldicr (c, shift, mask) > + || can_be_built_by_li_and_rldic (c, shift, mask)); > +} > + > /* Subroutine of rs6000_emit_set_const, handling PowerPC64 DImode. > Output insns to set DEST equal to the constant C as a series of > lis, ori and shl instructions. */ > @@ -10538,10 +10559,7 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c) > emit_move_insn (dest, gen_rtx_XOR (DImode, temp, > GEN_INT ((ud2 ^ 0xffff) << 16))); > } > - else if (can_be_built_by_li_lis_and_rotldi (c, &shift, &mask) > - || can_be_built_by_li_lis_and_rldicl (c, &shift, &mask) > - || can_be_built_by_li_lis_and_rldicr (c, &shift, &mask) > - || can_be_built_by_li_and_rldic (c, &shift, &mask)) > + else if (can_be_built_by_lilis_and_rldicX (c, &shift, &mask)) > { > temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode); > unsigned HOST_WIDE_INT imm = (c | ~mask);
Hi, "Kewen.Lin" <linkw@linux.ibm.com> writes: > Hi, > > on 2023/10/25 10:00, Jiufu Guo wrote: >> Hi, >> >> Trunk gcc supports more constants to be built via two instructions: e.g. >> "li/lis; xori/xoris/rldicl/rldicr/rldic". >> And then num_insns_constant should also be updated. >> > > Thanks for updating this. > >> Bootstrap & regtest pass ppc64{,le}. >> Is this ok for trunk? >> >> BR, >> Jeff (Jiufu Guo) >> >> gcc/ChangeLog: >> >> * config/rs6000/rs6000.cc (can_be_built_by_lilis_and_rldicX): New >> function. >> (num_insns_constant_gpr): Update to return 2 for more cases. >> (rs6000_emit_set_long_const): Update to use >> can_be_built_by_lilis_and_rldicX. >> >> --- >> gcc/config/rs6000/rs6000.cc | 64 ++++++++++++++++++++++++------------- >> 1 file changed, 41 insertions(+), 23 deletions(-) >> >> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc >> index cc24dd5301e..b23ff3d7917 100644 >> --- a/gcc/config/rs6000/rs6000.cc >> +++ b/gcc/config/rs6000/rs6000.cc >> @@ -6032,6 +6032,9 @@ direct_return (void) >> return 0; >> } >> >> +static bool >> +can_be_built_by_lilis_and_rldicX (HOST_WIDE_INT, int *, HOST_WIDE_INT *); >> + >> /* Helper for num_insns_constant. Calculate number of instructions to >> load VALUE to a single gpr using combinations of addi, addis, ori, >> oris, sldi and rldimi instructions. */ >> @@ -6044,35 +6047,41 @@ num_insns_constant_gpr (HOST_WIDE_INT value) >> return 1; >> >> /* constant loadable with addis */ >> - else if ((value & 0xffff) == 0 >> - && (value >> 31 == -1 || value >> 31 == 0)) >> + if ((value & 0xffff) == 0 && (value >> 31 == -1 || value >> 31 == 0)) >> return 1; >> >> /* PADDI can support up to 34 bit signed integers. */ >> - else if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (value)) >> + if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (value)) >> return 1; >> >> - else if (TARGET_POWERPC64) >> - { >> - HOST_WIDE_INT low = sext_hwi (value, 32); >> - HOST_WIDE_INT high = value >> 31; >> + if (!TARGET_POWERPC64) >> + return 2; >> >> - if (high == 0 || high == -1) >> - return 2; >> + HOST_WIDE_INT low = sext_hwi (value, 32); >> + HOST_WIDE_INT high = value >> 31; >> >> - high >>= 1; >> + if (high == 0 || high == -1) >> + return 2; >> >> - if (low == 0 || low == high) >> - return num_insns_constant_gpr (high) + 1; >> - else if (high == 0) >> - return num_insns_constant_gpr (low) + 1; >> - else >> - return (num_insns_constant_gpr (high) >> - + num_insns_constant_gpr (low) + 1); >> - } >> + high >>= 1; >> >> - else >> + HOST_WIDE_INT ud2 = (low >> 16) & 0xffff; >> + HOST_WIDE_INT ud1 = low & 0xffff; >> + if (high == -1 && ((!(ud2 & 0x8000) && ud1 == 0) || (ud1 & 0x8000))) >> + return 2; >> + if (high == 0 && (ud1 == 0 || (!(ud1 & 0x8000)))) >> return 2; > > I was thinking that instead of enumerating all the cases in function > rs6000_emit_set_long_const, if we can add one optional argument like > "int* num_insns=nullptr" to function rs6000_emit_set_long_const, and > when it's not nullptr, not emitting anything but update the count in > rs6000_emit_set_long_const. It helps people remember to update > num_insns when updating rs6000_emit_set_long_const in future, it's > also more clear on how the number comes from. > > Does it sound good to you? Thanks for your advice! Yes, "rs6000_emit_set_long_const" and "num_insns_constant_gpr" should be aligned. Using a same logic (same code place) would make sense. BR, Jeff (Jiufu Guo) > > BR, > Kewen > >> + >> + int shift; >> + HOST_WIDE_INT mask; >> + if (can_be_built_by_lilis_and_rldicX (value, &shift, &mask)) >> + return 2; >> + >> + if (low == 0 || low == high) >> + return num_insns_constant_gpr (high) + 1; >> + if (high == 0) >> + return num_insns_constant_gpr (low) + 1; >> + return (num_insns_constant_gpr (high) + num_insns_constant_gpr (low) + 1); >> } >> >> /* Helper for num_insns_constant. Allow constants formed by the >> @@ -10492,6 +10501,18 @@ can_be_built_by_li_and_rldic (HOST_WIDE_INT c, int *shift, HOST_WIDE_INT *mask) >> return false; >> } >> >> +/* Combine the above checking functions for li/lis;rldicX. */ >> + >> +static bool >> +can_be_built_by_lilis_and_rldicX (HOST_WIDE_INT c, int *shift, >> + HOST_WIDE_INT *mask) >> +{ >> + return (can_be_built_by_li_lis_and_rotldi (c, shift, mask) >> + || can_be_built_by_li_lis_and_rldicl (c, shift, mask) >> + || can_be_built_by_li_lis_and_rldicr (c, shift, mask) >> + || can_be_built_by_li_and_rldic (c, shift, mask)); >> +} >> + >> /* Subroutine of rs6000_emit_set_const, handling PowerPC64 DImode. >> Output insns to set DEST equal to the constant C as a series of >> lis, ori and shl instructions. */ >> @@ -10538,10 +10559,7 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c) >> emit_move_insn (dest, gen_rtx_XOR (DImode, temp, >> GEN_INT ((ud2 ^ 0xffff) << 16))); >> } >> - else if (can_be_built_by_li_lis_and_rotldi (c, &shift, &mask) >> - || can_be_built_by_li_lis_and_rldicl (c, &shift, &mask) >> - || can_be_built_by_li_lis_and_rldicr (c, &shift, &mask) >> - || can_be_built_by_li_and_rldic (c, &shift, &mask)) >> + else if (can_be_built_by_lilis_and_rldicX (c, &shift, &mask)) >> { >> temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode); >> unsigned HOST_WIDE_INT imm = (c | ~mask);
diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc index cc24dd5301e..b23ff3d7917 100644 --- a/gcc/config/rs6000/rs6000.cc +++ b/gcc/config/rs6000/rs6000.cc @@ -6032,6 +6032,9 @@ direct_return (void) return 0; } +static bool +can_be_built_by_lilis_and_rldicX (HOST_WIDE_INT, int *, HOST_WIDE_INT *); + /* Helper for num_insns_constant. Calculate number of instructions to load VALUE to a single gpr using combinations of addi, addis, ori, oris, sldi and rldimi instructions. */ @@ -6044,35 +6047,41 @@ num_insns_constant_gpr (HOST_WIDE_INT value) return 1; /* constant loadable with addis */ - else if ((value & 0xffff) == 0 - && (value >> 31 == -1 || value >> 31 == 0)) + if ((value & 0xffff) == 0 && (value >> 31 == -1 || value >> 31 == 0)) return 1; /* PADDI can support up to 34 bit signed integers. */ - else if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (value)) + if (TARGET_PREFIXED && SIGNED_INTEGER_34BIT_P (value)) return 1; - else if (TARGET_POWERPC64) - { - HOST_WIDE_INT low = sext_hwi (value, 32); - HOST_WIDE_INT high = value >> 31; + if (!TARGET_POWERPC64) + return 2; - if (high == 0 || high == -1) - return 2; + HOST_WIDE_INT low = sext_hwi (value, 32); + HOST_WIDE_INT high = value >> 31; - high >>= 1; + if (high == 0 || high == -1) + return 2; - if (low == 0 || low == high) - return num_insns_constant_gpr (high) + 1; - else if (high == 0) - return num_insns_constant_gpr (low) + 1; - else - return (num_insns_constant_gpr (high) - + num_insns_constant_gpr (low) + 1); - } + high >>= 1; - else + HOST_WIDE_INT ud2 = (low >> 16) & 0xffff; + HOST_WIDE_INT ud1 = low & 0xffff; + if (high == -1 && ((!(ud2 & 0x8000) && ud1 == 0) || (ud1 & 0x8000))) + return 2; + if (high == 0 && (ud1 == 0 || (!(ud1 & 0x8000)))) return 2; + + int shift; + HOST_WIDE_INT mask; + if (can_be_built_by_lilis_and_rldicX (value, &shift, &mask)) + return 2; + + if (low == 0 || low == high) + return num_insns_constant_gpr (high) + 1; + if (high == 0) + return num_insns_constant_gpr (low) + 1; + return (num_insns_constant_gpr (high) + num_insns_constant_gpr (low) + 1); } /* Helper for num_insns_constant. Allow constants formed by the @@ -10492,6 +10501,18 @@ can_be_built_by_li_and_rldic (HOST_WIDE_INT c, int *shift, HOST_WIDE_INT *mask) return false; } +/* Combine the above checking functions for li/lis;rldicX. */ + +static bool +can_be_built_by_lilis_and_rldicX (HOST_WIDE_INT c, int *shift, + HOST_WIDE_INT *mask) +{ + return (can_be_built_by_li_lis_and_rotldi (c, shift, mask) + || can_be_built_by_li_lis_and_rldicl (c, shift, mask) + || can_be_built_by_li_lis_and_rldicr (c, shift, mask) + || can_be_built_by_li_and_rldic (c, shift, mask)); +} + /* Subroutine of rs6000_emit_set_const, handling PowerPC64 DImode. Output insns to set DEST equal to the constant C as a series of lis, ori and shl instructions. */ @@ -10538,10 +10559,7 @@ rs6000_emit_set_long_const (rtx dest, HOST_WIDE_INT c) emit_move_insn (dest, gen_rtx_XOR (DImode, temp, GEN_INT ((ud2 ^ 0xffff) << 16))); } - else if (can_be_built_by_li_lis_and_rotldi (c, &shift, &mask) - || can_be_built_by_li_lis_and_rldicl (c, &shift, &mask) - || can_be_built_by_li_lis_and_rldicr (c, &shift, &mask) - || can_be_built_by_li_and_rldic (c, &shift, &mask)) + else if (can_be_built_by_lilis_and_rldicX (c, &shift, &mask)) { temp = !can_create_pseudo_p () ? dest : gen_reg_rtx (DImode); unsigned HOST_WIDE_INT imm = (c | ~mask);
Hi, Trunk gcc supports more constants to be built via two instructions: e.g. "li/lis; xori/xoris/rldicl/rldicr/rldic". And then num_insns_constant should also be updated. Bootstrap & regtest pass ppc64{,le}. Is this ok for trunk? BR, Jeff (Jiufu Guo) gcc/ChangeLog: * config/rs6000/rs6000.cc (can_be_built_by_lilis_and_rldicX): New function. (num_insns_constant_gpr): Update to return 2 for more cases. (rs6000_emit_set_long_const): Update to use can_be_built_by_lilis_and_rldicX. --- gcc/config/rs6000/rs6000.cc | 64 ++++++++++++++++++++++++------------- 1 file changed, 41 insertions(+), 23 deletions(-)