Re: [PATCH] rs6000: Replace OPTION_MASK_DIRECT_MOVE with OPTION_MASK_P8_VECTOR [PR101865]

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Peter,

on 2024/4/6 06:28, Peter Bergner wrote:
> This is a cleanup patch in preparation to fixing the real bug in PR101865.
> TARGET_DIRECT_MOVE is redundant with TARGET_P8_VECTOR, so alias it to that.
> Also replace all usages of OPTION_MASK_DIRECT_MOVE with OPTION_MASK_P8_VECTOR
> and delete the now dead mask.
> 
> This passed bootstrap and retesting on powerpc64le-linux with no regressions.
> Ok for trunk?
> 
> Eventually we'll want to backport this along with the follow-on patch that
> actually fixes PR101865.
> 
> Peter
> 
> 
> gcc/
> 	PR target/101865
> 	* config/rs6000/rs6000.h (TARGET_DIRECT_MOVE): Define.
> 	* config/rs6000/rs6000.cc (rs6000_option_override_internal): Replace
> 	OPTION_MASK_DIRECT_MOVE with OPTION_MASK_P8_VECTOR.  Delete redundant
> 	OPTION_MASK_DIRECT_MOVE usage.  Delete TARGET_DIRECT_MOVE dead code.
> 	(rs6000_opt_masks): Neuter the "direct-move" option.
> 	* config/rs6000/rs6000-c.cc (rs6000_target_modify_macros): Replace
> 	OPTION_MASK_DIRECT_MOVE with OPTION_MASK_P8_VECTOR.  Delete useless
> 	comment.
> 	* config/rs6000/rs6000-cpus.def (ISA_2_7_MASKS_SERVER): Delete
> 	OPTION_MASK_DIRECT_MOVE.
> 	(OTHER_VSX_VECTOR_MASKS): Likewise.
> 	(POWERPC_MASKS): Likewise.
> 	* config/rs6000/rs6000.opt (mno-direct-move): New.
> 	(mdirect-move): Remove Mask and Var.
> 
> 
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 68bc45d65ba..77d045c9f6e 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -471,6 +471,8 @@ extern int rs6000_vector_align[];
>  #define TARGET_EXTSWSLI	(TARGET_MODULO && TARGET_POWERPC64)
>  #define TARGET_MADDLD	TARGET_MODULO
>  
> +/* TARGET_DIRECT_MOVE is redundant to TARGET_P8_VECTOR, so alias it to that.  */
> +#define TARGET_DIRECT_MOVE	TARGET_P8_VECTOR
>  #define TARGET_XSCVDPSPN	(TARGET_DIRECT_MOVE || TARGET_P8_VECTOR)
>  #define TARGET_XSCVSPDPN	(TARGET_DIRECT_MOVE || TARGET_P8_VECTOR)
>  #define TARGET_VADDUQM		(TARGET_P8_VECTOR && TARGET_POWERPC64)
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index 6ba9df4f02e..c241371147c 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -3811,7 +3811,7 @@ rs6000_option_override_internal (bool global_init_p)
>       Testing for direct_move matches power8 and later.  */
>    if (!BYTES_BIG_ENDIAN
>        && !(processor_target_table[tune_index].target_enable
> -	   & OPTION_MASK_DIRECT_MOVE))
> +	   & OPTION_MASK_P8_VECTOR))
>      rs6000_isa_flags |= ~rs6000_isa_flags_explicit & OPTION_MASK_STRICT_ALIGN;
>  
>    /* Add some warnings for VSX.  */
> @@ -3853,8 +3853,7 @@ rs6000_option_override_internal (bool global_init_p)
>        && (rs6000_isa_flags_explicit & (OPTION_MASK_SOFT_FLOAT
>  				       | OPTION_MASK_ALTIVEC
>  				       | OPTION_MASK_VSX)) != 0)
> -    rs6000_isa_flags &= ~((OPTION_MASK_P8_VECTOR | OPTION_MASK_CRYPTO
> -			   | OPTION_MASK_DIRECT_MOVE)
> +    rs6000_isa_flags &= ~((OPTION_MASK_P8_VECTOR | OPTION_MASK_CRYPTO)
>  		         & ~rs6000_isa_flags_explicit);
>  
>    if (TARGET_DEBUG_REG || TARGET_DEBUG_TARGET)
> @@ -3939,13 +3938,6 @@ rs6000_option_override_internal (bool global_init_p)
>        rs6000_isa_flags &= ~OPTION_MASK_FPRND;
>      }
>  
> -  if (TARGET_DIRECT_MOVE && !TARGET_VSX)
> -    {
> -      if (rs6000_isa_flags_explicit & OPTION_MASK_DIRECT_MOVE)
> -	error ("%qs requires %qs", "-mdirect-move", "-mvsx");
> -      rs6000_isa_flags &= ~OPTION_MASK_DIRECT_MOVE;
> -    }
> -
>    if (TARGET_P8_VECTOR && !TARGET_ALTIVEC)
>      rs6000_isa_flags &= ~OPTION_MASK_P8_VECTOR;
>  
> @@ -24429,7 +24421,7 @@ static struct rs6000_opt_mask const rs6000_opt_masks[] =
>  								false, true  },
>    { "cmpb",			OPTION_MASK_CMPB,		false, true  },
>    { "crypto",			OPTION_MASK_CRYPTO,		false, true  },
> -  { "direct-move",		OPTION_MASK_DIRECT_MOVE,	false, true  },
> +  { "direct-move",		0,				false, true  },
>    { "dlmzb",			OPTION_MASK_DLMZB,		false, true  },
>    { "efficient-unaligned-vsx",	OPTION_MASK_EFFICIENT_UNALIGNED_VSX,
>  								false, true  },
> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
> index ce0b14a8d37..647f20de7f2 100644
> --- a/gcc/config/rs6000/rs6000-c.cc
> +++ b/gcc/config/rs6000/rs6000-c.cc
> @@ -429,19 +429,7 @@ rs6000_target_modify_macros (bool define_p, HOST_WIDE_INT flags)
>      rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR6");
>    if ((flags & OPTION_MASK_POPCNTD) != 0)
>      rs6000_define_or_undefine_macro (define_p, "_ARCH_PWR7");
> -  /* Note that the OPTION_MASK_DIRECT_MOVE flag is automatically
> -     turned on in the following condition:
> -     1. TARGET_P8_VECTOR is enabled and OPTION_MASK_DIRECT_MOVE is not
> -        explicitly disabled.
> -        Hereafter, the OPTION_MASK_DIRECT_MOVE flag is considered to
> -        have been turned on explicitly.
> -     Note that the OPTION_MASK_DIRECT_MOVE flag is automatically
> -     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 ((flags & OPTION_MASK_P8_VECTOR) != 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");
> diff --git a/gcc/config/rs6000/rs6000-cpus.def b/gcc/config/rs6000/rs6000-cpus.def
> index 28249600318..45dd5a85901 100644
> --- a/gcc/config/rs6000/rs6000-cpus.def
> +++ b/gcc/config/rs6000/rs6000-cpus.def
> @@ -49,7 +49,6 @@
>  #define ISA_2_7_MASKS_SERVER	(ISA_2_6_MASKS_SERVER			\
>  				 | OPTION_MASK_P8_VECTOR		\
>  				 | OPTION_MASK_CRYPTO			\
> -				 | OPTION_MASK_DIRECT_MOVE		\
>  				 | OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
>  				 | OPTION_MASK_QUAD_MEMORY		\
>  				 | OPTION_MASK_QUAD_MEMORY_ATOMIC)
> @@ -90,7 +89,6 @@
>  #define OTHER_VSX_VECTOR_MASKS	(OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
>  				 | OPTION_MASK_FLOAT128_KEYWORD		\
>  				 | OPTION_MASK_P8_VECTOR		\
> -				 | OPTION_MASK_DIRECT_MOVE		\
>  				 | OPTION_MASK_CRYPTO			\
>  				 | OPTION_MASK_P9_VECTOR		\
>  				 | OPTION_MASK_FLOAT128_HW		\
> @@ -118,7 +116,6 @@
>  				 | OPTION_MASK_CMPB			\
>  				 | OPTION_MASK_CRYPTO			\
>  				 | OPTION_MASK_DFP			\
> -				 | OPTION_MASK_DIRECT_MOVE		\
>  				 | OPTION_MASK_DLMZB			\
>  				 | OPTION_MASK_EFFICIENT_UNALIGNED_VSX	\
>  				 | OPTION_MASK_FLOAT128_HW		\
> diff --git a/gcc/config/rs6000/rs6000.opt b/gcc/config/rs6000/rs6000.opt
> index 83197681b66..62131e45bdf 100644
> --- a/gcc/config/rs6000/rs6000.opt
> +++ b/gcc/config/rs6000/rs6000.opt
> @@ -490,8 +490,11 @@ mcrypto
>  Target Mask(CRYPTO) Var(rs6000_isa_flags)
>  Use ISA 2.07 Category:Vector.AES and Category:Vector.SHA2 instructions.
>  
> +mno-direct-move
> +Target Undocumented WarnRemoved
> +
>  mdirect-move
> -Target Undocumented Mask(DIRECT_MOVE) Var(rs6000_isa_flags) WarnRemoved
> +Target Undocumented WarnRemoved

When reviewing my previous patch to "neuter option -mpower{8,9}-vector",
Segher mentioned that we don't need to keep such option warning all the
time and can drop it like in a release later as users should be aware of
this information then, I agreed and considering that patch disabling
-m[no-]direct-move was r8-7845-g57f108f5a1e1b2, I think we can just remove
m[no-]direct-move here?  What do you think?

OK for trunk with this tweaked if you agree that, and also ok for active
release branches after burn-in time, thanks!

BR,
Kewen