| Message ID | Zy5rZozlNONZl5TV@cowardly-lion.the-meissners.org |
|---|---|
| State | New |
| Headers | show |
| Series | Separate PowerPC archiecture bits from ISA flags that use command line options | expand |
On 11/8/24 1:49 PM, Michael Meissner wrote: > As part of the architecture flags patches, this patch changes the use of > TARGET_POPCNTB to TARGET_POWER5. The POPCNTB instruction was added in ISA 2.02 > (power5). I like what this patch and the other related clean up patches are doing, namely changing the TARGET_<MNEMONIC> macros to TARGET_<CPU> which makes much more sense. However, the way you ordered the patch series, this cleanup patch depends on the main patches that change us to using architecture flags, rather than the isa flags that require explicit machine options. I'd prefer (and I think Segher will too) that these cleanup patches be done *before* your main patches that change us to using architecture flags. That way they're independent of the main patches so if we had to revert those patches, then these cleanup patches would not have to be reverted too. So I'm speaking of patches 4/11, 5/11. 7/11 and 8/11. I don't see a 6/11. Did you forget to email that? Was that for changing TARGET_FOO to TARGET_POWER6? If so, then that should be handled like patches 4 thru 8. Peter
On Thu, Nov 14, 2024 at 06:26:11PM -0600, Peter Bergner wrote: > On 11/8/24 1:49 PM, Michael Meissner wrote: > > As part of the architecture flags patches, this patch changes the use of > > TARGET_POPCNTB to TARGET_POWER5. The POPCNTB instruction was added in ISA 2.02 > > (power5). > > I like what this patch and the other related clean up patches are doing, > namely changing the TARGET_<MNEMONIC> macros to TARGET_<CPU> which makes > much more sense. However, the way you ordered the patch series, this > cleanup patch depends on the main patches that change us to using > architecture flags, rather than the isa flags that require explicit > machine options. > > I'd prefer (and I think Segher will too) that these cleanup patches be > done *before* your main patches that change us to using architecture > flags. That way they're independent of the main patches so if we had > to revert those patches, then these cleanup patches would not have to > be reverted too. > > So I'm speaking of patches 4/11, 5/11. 7/11 and 8/11. I don't see a > 6/11. Did you forget to email that? Was that for changing TARGET_FOO > to TARGET_POWER6? If so, then that should be handled like patches > 4 thru 8. Yes in the V2 version of the patches, I forgot to post patch #6. I posted it in the V3 patches, that also included the fix if you did not specify a default CPU on a LE system. I have reformulated the patches, and I will be posting them shortly. I will be splitting them into 4 groups. The first patch set will provide TARGET_POWER{5,5X,6,7,8,9,10,11} based on the current ISA bits. It just adds the define and then changes most of the uses. The second path does not allow -mvsx to bump up the cpu to power7. The third patch set after the TARGET_PATCH<x> set is applied adds the arch masks, and removes the 3 switches used to set the arch bits, but are not documented (-mpower8-internal, -mpower10, and -mpower11). The fourth patch after the 3 above patch sets are applied adds the -mcpu=future support.
On Thu, Nov 14, 2024 at 06:26:11PM -0600, Peter Bergner wrote: > On 11/8/24 1:49 PM, Michael Meissner wrote: > > As part of the architecture flags patches, this patch changes the use of > > TARGET_POPCNTB to TARGET_POWER5. The POPCNTB instruction was added in ISA 2.02 > > (power5). > > I like what this patch and the other related clean up patches are doing, > namely changing the TARGET_<MNEMONIC> macros to TARGET_<CPU> which makes > much more sense. However, the way you ordered the patch series, this > cleanup patch depends on the main patches that change us to using > architecture flags, rather than the isa flags that require explicit > machine options. > > I'd prefer (and I think Segher will too) that these cleanup patches be > done *before* your main patches that change us to using architecture > flags. That way they're independent of the main patches so if we had > to revert those patches, then these cleanup patches would not have to > be reverted too. > > So I'm speaking of patches 4/11, 5/11. 7/11 and 8/11. I don't see a > 6/11. Did you forget to email that? Was that for changing TARGET_FOO > to TARGET_POWER6? If so, then that should be handled like patches > 4 thru 8. See the 4 patch sets: Add more user friendly TARGET_ names for PowerPC https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669067.html Add support for -mcpu=future in the PowerPC https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669099.html Do not allow -mvsx to boost the cpu to power7 https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669106.html Separte PowerPC ISA bits from architecture bits set by -mcpu=<xxx> https://gcc.gnu.org/pipermail/gcc-patches/2024-November/669108.html
diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc index 9bdbae1ecf9..98a0545030c 100644 --- a/gcc/config/rs6000/rs6000-builtin.cc +++ b/gcc/config/rs6000/rs6000-builtin.cc @@ -155,7 +155,7 @@ rs6000_builtin_is_supported (enum rs6000_gen_builtins fncode) case ENB_ALWAYS: return true; case ENB_P5: - return TARGET_POPCNTB; + return TARGET_POWER5; case ENB_P6: return TARGET_CMPB; case ENB_P6_64: diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h index 7ad8baca177..4500724d895 100644 --- a/gcc/config/rs6000/rs6000.h +++ b/gcc/config/rs6000/rs6000.h @@ -547,9 +547,7 @@ extern int rs6000_vector_align[]; #define TARGET_EXTRA_BUILTINS (TARGET_POWERPC64 \ || TARGET_PPC_GPOPT /* 970/power4 */ \ - || TARGET_POPCNTB /* ISA 2.02 */ \ - || TARGET_CMPB /* ISA 2.05 */ \ - || TARGET_POPCNTD /* ISA 2.06 */ \ + || TARGET_POWER5 /* ISA 2.02 & above */ \ || TARGET_ALTIVEC \ || TARGET_VSX \ || TARGET_HARD_FLOAT) @@ -563,9 +561,9 @@ extern int rs6000_vector_align[]; #define TARGET_FRES (TARGET_HARD_FLOAT && TARGET_PPC_GFXOPT) #define TARGET_FRE (TARGET_HARD_FLOAT \ - && (TARGET_POPCNTB || VECTOR_UNIT_VSX_P (DFmode))) + && (TARGET_POWER5 || VECTOR_UNIT_VSX_P (DFmode))) -#define TARGET_FRSQRTES (TARGET_HARD_FLOAT && TARGET_POPCNTB \ +#define TARGET_FRSQRTES (TARGET_HARD_FLOAT && TARGET_POWER5 \ && TARGET_PPC_GFXOPT) #define TARGET_FRSQRTE (TARGET_HARD_FLOAT \ diff --git a/gcc/config/rs6000/rs6000.md b/gcc/config/rs6000/rs6000.md index 8eda2f7bb0d..10d13bf812d 100644 --- a/gcc/config/rs6000/rs6000.md +++ b/gcc/config/rs6000/rs6000.md @@ -379,7 +379,7 @@ (define_attr "enabled" "" (const_int 1) (and (eq_attr "isa" "p5") - (match_test "TARGET_POPCNTB")) + (match_test "TARGET_POWER5")) (const_int 1) (and (eq_attr "isa" "p6") @@ -2510,7 +2510,7 @@ (define_expand "ffs<mode>2" (define_expand "popcount<mode>2" [(set (match_operand:GPR 0 "gpc_reg_operand") (popcount:GPR (match_operand:GPR 1 "gpc_reg_operand")))] - "TARGET_POPCNTB || TARGET_POPCNTD" + "TARGET_POWER5" { rs6000_emit_popcount (operands[0], operands[1]); DONE; @@ -2520,7 +2520,7 @@ (define_insn "popcntb<mode>2" [(set (match_operand:GPR 0 "gpc_reg_operand" "=r") (unspec:GPR [(match_operand:GPR 1 "gpc_reg_operand" "r")] UNSPEC_POPCNTB))] - "TARGET_POPCNTB" + "TARGET_POWER5" "popcntb %0,%1" [(set_attr "type" "popcnt")]) @@ -2535,7 +2535,7 @@ (define_insn "popcntd<mode>2" (define_expand "parity<mode>2" [(set (match_operand:GPR 0 "gpc_reg_operand") (parity:GPR (match_operand:GPR 1 "gpc_reg_operand")))] - "TARGET_POPCNTB" + "TARGET_POWER5" { rs6000_emit_parity (operands[0], operands[1]); DONE; @@ -2544,7 +2544,7 @@ (define_expand "parity<mode>2" (define_insn "parity<mode>2_cmpb" [(set (match_operand:GPR 0 "gpc_reg_operand" "=r") (unspec:GPR [(match_operand:GPR 1 "gpc_reg_operand" "r")] UNSPEC_PARITY))] - "TARGET_CMPB && TARGET_POPCNTB" + "TARGET_CMPB" "prty<wd> %0,%1" [(set_attr "type" "popcnt")])