Re: [PATCH, rs6000] Split TARGET_POWER8 from TARGET_DIRECT_MOVE [PR101865] (2/2)

[will schmidt <will_schmidt@vnet.ibm.com>]

Ping.

On Mon, 2022-09-19 at 11:13 -0500, will schmidt wrote:
> [PATCH, rs6000] Split TARGET_POWER8 from TARGET_DIRECT_MOVE [PR101865]
> 
> Hi,
>   The _ARCH_PWR8 define is conditional on TARGET_DIRECT_MOVE,
> and can be disabled by dependent options when it should not be.
> This manifests in the issue seen in PR101865 where -mno-vsx
> mistakenly disables _ARCH_PWR8.
> 
> This change replaces the relevant TARGET_DIRECT_MOVE references
> with a TARGET_POWER8 entry so that the direct_move and power8
> features can be enabled or disabled independently.
> 
> This is done via the OPTION_MASK definitions, so this
> means that some references to the OPTION_MASK_DIRECT_MOVE
> option are now replaced with OPTION_MASK_POWER8.
> 
> The existing (and rather lengthy) commentary for DIRECT_MOVE remains
> in place in rs6000-c.cc:rs6000_target_modify_macros().  The
> if-defined logic there will now set a __DIRECT_MOVE__ define when
> TARGET_DIRECT_MOVE is set, this serves as a placeholder for debug
> purposes, but is otherwise unused.  This can be removed in a
> subsequent patch, or in an update of this patch, depending on feedback.
> 
> This regests cleanly (power8,power9,power10), and resolves
> PR 101865 as represented in the tests from (1/2).
> 
> OK for trunk?
> Thanks,
> -Will
> 
> 
> gcc/
> 	PR Target/101865
> 	* config/rs6000/rs6000-builtin.cc
> 	(rs6000_builtin_is_supported): Replace TARGET_DIRECT_MOVE
> 	usage with TARGET_POWER8.
> 	* config/rs6000/rs6000-c.cc (rs6000_target_modify_macros):
> 	Add __DIRECT_MOVE__ define.  Replace _ARCH_PWR8_ define
> 	conditional with OPTION_MASK_POWER8.
> 	* config/rs6000/rs6000-cpus.def (ISA_2_7_MASKS_SERVER):
> 	Add OPTION_MASK_POWER8 entry.
> 	(POWERPC_MASKS): Same.
> 	* config/rs6000/rs6000.cc (rs6000_option_override_internal):
> 	Replace OPTION_MASK_DIRECT_MOVE usage with OPTION_MASK_POWER8.
> 	(rs6000_opt_masks): Add "power8" entry for new OPTION_MASK_POWER8.
> 	* config/rs6000/rs6000.opt (-mpower8): Add entry for POWER8.
> 	* config/rs6000/vsx.md (vsx_extract_<mode>): Replace
> 	TARGET_DIRECT_MOVE usage with TARGET_POWER8.
> 	(define_peephole2): Same.
> 
> diff --git a/gcc/config/rs6000/rs6000-builtin.cc b/gcc/config/rs6000/rs6000-builtin.cc
> index 3ce729c1e6de..91a0f39bd796 100644
> --- a/gcc/config/rs6000/rs6000-builtin.cc
> +++ b/gcc/config/rs6000/rs6000-builtin.cc
> @@ -163,11 +163,11 @@ rs6000_builtin_is_supported (enum rs6000_gen_builtins fncode)
>      case ENB_P7:
>        return TARGET_POPCNTD;
>      case ENB_P7_64:
>        return TARGET_POPCNTD && TARGET_POWERPC64;
>      case ENB_P8:
> -      return TARGET_DIRECT_MOVE;
> +      return TARGET_POWER8;
>      case ENB_P8V:
>        return TARGET_P8_VECTOR;
>      case ENB_P9:
>        return TARGET_MODULO;
>      case ENB_P9_64:
> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
> index ca9cc42028f7..41d51b039061 100644
> --- a/gcc/config/rs6000/rs6000-c.cc
> +++ b/gcc/config/rs6000/rs6000-c.cc
> @@ -439,11 +439,13 @@ rs6000_target_modify_macros (bool define_p, HOST_WIDE_INT flags)
>       turned off in any of the following conditions:
>       1. TARGET_HARD_FLOAT, TARGET_ALTIVEC, or TARGET_VSX is explicitly
>  	disabled and OPTION_MASK_DIRECT_MOVE was not explicitly
>  	enabled.
>       2. TARGET_VSX is off.  */
> -  if ((flags & OPTION_MASK_DIRECT_MOVE) != 0)
> +  if ((OPTION_MASK_DIRECT_MOVE) != 0)
> +    rs6000_define_or_undefine_macro (define_p, "__DIRECT_MOVE__");
> +  if ((flags & OPTION_MASK_POWER8) != 0)
>      rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR8");
>    if ((flags & OPTION_MASK_MODULO) != 0)
>      rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR9");
>    if ((flags & OPTION_MASK_POWER10) != 0)
>      rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR10");
> diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def
> index c3825bcccd84..c873f6d58989 100644
> --- a/gcc/config/rs6000/rs6000-cpus.def
> +++ b/gcc/config/rs6000/rs6000-cpus.def
> @@ -48,10 +48,11 @@
>     system.  */
>  #define ISA_2_7_MASKS_SERVER	(ISA_2_6_MASKS_SERVER			\
>  				 | OPTION_MASK_P8_VECTOR		\
>  				 | OPTION_MASK_CRYPTO			\
>  				 | OPTION_MASK_DIRECT_MOVE		\
> +				 | OPTION_MASK_POWER8			\
>  				 | OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
>  				 | OPTION_MASK_QUAD_MEMORY		\
>  				 | OPTION_MASK_QUAD_MEMORY_ATOMIC)
> 
>  /* ISA masks setting fusion options.  */
> @@ -124,10 +125,11 @@
>  #define POWERPC_MASKS		(OPTION_MASK_ALTIVEC			\
>  				 | OPTION_MASK_CMPB			\
>  				 | OPTION_MASK_CRYPTO			\
>  				 | OPTION_MASK_DFP			\
>  				 | OPTION_MASK_DIRECT_MOVE		\
> +				 | OPTION_MASK_POWER8			\
>  				 | OPTION_MASK_DLMZB			\
>  				 | OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
>  				 | OPTION_MASK_FLOAT128_HW		\
>  				 | OPTION_MASK_FLOAT128_KEYWORD		\
>  				 | OPTION_MASK_FPRND			\
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index fcca062a8709..ed423b9e1837 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -3766,15 +3766,14 @@ rs6000_option_override_internal (bool global_init_p)
>        if ((rs6000_isa_flags_explicit & OPTION_MASK_MULTIPLE) != 0)
>  	warning (0, "%qs is not supported on little endian systems",
>  		 "-mmultiple");
>      }
> 
> -  /* If little-endian, default to -mstrict-align on older processors.
> -     Testing for direct_move matches power8 and later.  */
> +  /* If little-endian, default to -mstrict-align on older processors.  */
>    if (!BYTES_BIG_ENDIAN
>        && !(processor_target_table[tune_index].target_enable
> -	   & OPTION_MASK_DIRECT_MOVE))
> +	   & OPTION_MASK_POWER8))
>      rs6000_isa_flags |= ~rs6000_isa_flags_explicit & OPTION_MASK_STRICT_ALIGN;
> 
>    /* Add some warnings for VSX.  */
>    if (TARGET_VSX)
>      {
> @@ -3857,11 +3856,11 @@ rs6000_option_override_internal (bool global_init_p)
>  	error ("%qs incompatible with explicitly disabled options",
>  	       "-mpower9-minmax");
>        else
>  	rs6000_isa_flags |= ISA_3_0_MASKS_SERVER;
>      }
> -  else if (TARGET_P8_VECTOR || TARGET_DIRECT_MOVE || TARGET_CRYPTO)
> +  else if (TARGET_P8_VECTOR || TARGET_POWER8 || TARGET_CRYPTO)
>      rs6000_isa_flags |= (ISA_2_7_MASKS_SERVER & ~ignore_masks);
>    else if (TARGET_VSX)
>      rs6000_isa_flags |= (ISA_2_6_MASKS_SERVER & ~ignore_masks);
>    else if (TARGET_POPCNTD)
>      rs6000_isa_flags |= (ISA_2_6_MASKS_EMBEDDED & ~ignore_masks);
> @@ -24046,10 +24045,11 @@ static struct rs6000_opt_mask const rs6000_opt_masks[] =
>    { "block-ops-vector-pair",	OPTION_MASK_BLOCK_OPS_VECTOR_PAIR,
>  								false, true  },
>    { "cmpb",			OPTION_MASK_CMPB,		false, true  },
>    { "crypto",			OPTION_MASK_CRYPTO,		false, true  },
>    { "direct-move",		OPTION_MASK_DIRECT_MOVE,	false, true  },
> +  { "power8",			OPTION_MASK_POWER8,		false, true  },
>    { "dlmzb",			OPTION_MASK_DLMZB,		false, true  },
>    { "efficient-unaligned-vsx",	OPTION_MASK_EFFICIENT_UNALIGNED_VSX,
>  								false, true  },
>    { "float128",			OPTION_MASK_FLOAT128_KEYWORD,	false, true  },
>    { "float128-hardware",	OPTION_MASK_FLOAT128_HW,	false, true  },
> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
> index b63a5d443af6..53964387da6d 100644
> --- a/gcc/config/rs6000/rs6000.opt
> +++ b/gcc/config/rs6000/rs6000.opt
> @@ -490,10 +490,15 @@ mcrypto
>  Target Mask(CRYPTO) Var(rs6000_isa_flags)
>  Use ISA 2.07 Category:Vector.AES and Category:Vector.SHA2 instructions.
> 
>  mdirect-move
>  Target Undocumented Mask(DIRECT_MOVE) Var(rs6000_isa_flags) WarnRemoved
> +Enable direct move (ISA 2.07).
> +
> +mpower8
> +Target Mask(POWER8) Var(rs6000_isa_flags)
> +Use instructions added in ISA 2.07 (power8).
> 
>  mhtm
>  Target Mask(HTM) Var(rs6000_isa_flags)
>  Use ISA 2.07 transactional memory (HTM) instructions.
> 
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index e226a93bbe55..be4fb902049d 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -3407,11 +3407,11 @@ (define_insn "vsx_extract_<mode>"
>    if (element == VECTOR_ELEMENT_SCALAR_64BIT)
>      {
>        if (op0_regno == op1_regno)
>  	return ASM_COMMENT_START " vec_extract to same register";
> 
> -      else if (INT_REGNO_P (op0_regno) && TARGET_DIRECT_MOVE
> +      else if (INT_REGNO_P (op0_regno) && TARGET_POWER8
>  	       && TARGET_POWERPC64)
>  	return "mfvsrd %0,%x1";
> 
>        else if (FP_REGNO_P (op0_regno) && FP_REGNO_P (op1_regno))
>  	return "fmr %0,%1";
> @@ -6204,11 +6204,11 @@ (define_peephole2
> 
>     ;; MTVSRD
>     (set (match_operand:SF SFBOOL_MTVSR_D "vsx_register_operand")
>  	(unspec:SF [(match_dup SFBOOL_SHL_D)] UNSPEC_P8V_MTVSRD))]
> 
> -  "TARGET_POWERPC64 && TARGET_DIRECT_MOVE
> +  "TARGET_POWERPC64 && TARGET_POWER8
>     /* The REG_P (xxx) tests prevents SUBREG's, which allows us to use REGNO
>        to compare registers, when the mode is different.  */
>     && REG_P (operands[SFBOOL_MFVSR_D]) && REG_P (operands[SFBOOL_BOOL_D])
>     && REG_P (operands[SFBOOL_BOOL_A1]) && REG_P (operands[SFBOOL_SHL_D])
>     && REG_P (operands[SFBOOL_SHL_A])   && REG_P (operands[SFBOOL_MTVSR_D])
>