[rs6000] Correct definition of macro of fixed point efficient unaligned

  Hi,
  The patch corrects the definition of
TARGET_EFFICIENT_OVERLAPPING_UNALIGNED and change its name to a
comprehensible name.

  Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no
regressions. Is this OK for trunk?

Thanks
Gui Haochen

ChangeLog
rs6000: Correct definition of macro of fixed point efficient unaligned

Marco TARGET_EFFICIENT_OVERLAPPING_UNALIGNED is used in rs6000-string.cc to
guard whether a platform is efficient on fixed point unaligned load/store.
It's originally defined by TARGET_EFFICIENT_UNALIGNED_VSX which is enabled
from P8 and can be disabled by mno-vsx option. So the definition is wrong.
This patch corrects the problem and define it by "!STRICT_ALIGNMENT" which
is true on P7 BE and P8 above.

gcc/
	* config/rs6000/rs6000.h (TARGET_EFFICIENT_OVERLAPPING_UNALIGNED):
	Rename to...
	(TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT): ...this, set it to
	!STRICT_ALIGNMENT.
	* config/rs6000/rs6000-string.cc (select_block_compare_mode):
	Replace TARGET_EFFICIENT_OVERLAPPING_UNALIGNED with
	TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT.
	(select_block_compare_mode): Likewise.
	(expand_block_compare_gpr): Likewise.
	(expand_block_compare): Likewise.
	(expand_strncmp_gpr_sequence): Likewise.

gcc/testsuite/
	* gcc.target/powerpc/target_efficient_unaligned_fixedpoint-1.c: New.
	* gcc.target/powerpc/target_efficient_unaligned_fixedpoint-2.c: New.

patch.diff
  

Hi,

on 2023/12/11 09:49, HAO CHEN GUI wrote:
> Hi,
>   The patch corrects the definition of
> TARGET_EFFICIENT_OVERLAPPING_UNALIGNED and change its name to a
> comprehensible name.
> 
>   Bootstrapped and tested on x86 and powerpc64-linux BE and LE with no
> regressions. Is this OK for trunk?
> 
> Thanks
> Gui Haochen
> 
> ChangeLog
> rs6000: Correct definition of macro of fixed point efficient unaligned
> 
> Marco TARGET_EFFICIENT_OVERLAPPING_UNALIGNED is used in rs6000-string.cc to
> guard whether a platform is efficient on fixed point unaligned load/store.
> It's originally defined by TARGET_EFFICIENT_UNALIGNED_VSX which is enabled
> from P8 and can be disabled by mno-vsx option. So the definition is wrong.
> This patch corrects the problem and define it by "!STRICT_ALIGNMENT" which
> is true on P7 BE and P8 above.

I agree it's unexpected that the current TARGET_EFFICIENT_OVERLAPPING_UNALIGNED
is mapped as TARGET_EFFICIENT_UNALIGNED_VSX for scalar int access, but there is
one function rs6000_slow_unaligned_access (for hook slow_unaligned_access), I
expect it can well fit into these places, can we just adopt it?  IMHO it helps
the long term maintenance. 

> 
> gcc/
> 	* config/rs6000/rs6000.h (TARGET_EFFICIENT_OVERLAPPING_UNALIGNED):
> 	Rename to...
> 	(TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT): ...this, set it to
> 	!STRICT_ALIGNMENT.
> 	* config/rs6000/rs6000-string.cc (select_block_compare_mode):
> 	Replace TARGET_EFFICIENT_OVERLAPPING_UNALIGNED with
> 	TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT.
> 	(select_block_compare_mode): Likewise.
> 	(expand_block_compare_gpr): Likewise.
> 	(expand_block_compare): Likewise.
> 	(expand_strncmp_gpr_sequence): Likewise.
> 
> gcc/testsuite/
> 	* gcc.target/powerpc/target_efficient_unaligned_fixedpoint-1.c: New.
> 	* gcc.target/powerpc/target_efficient_unaligned_fixedpoint-2.c: New.
> 
> patch.diff
> diff --git a/gcc/config/rs6000/rs6000-string.cc b/gcc/config/rs6000/rs6000-string.cc
> index 44a946cd453..d4030854b2a 100644
> --- a/gcc/config/rs6000/rs6000-string.cc
> +++ b/gcc/config/rs6000/rs6000-string.cc
> @@ -305,7 +305,7 @@ select_block_compare_mode (unsigned HOST_WIDE_INT offset,
>    else if (bytes == GET_MODE_SIZE (QImode))
>      return QImode;
>    else if (bytes < GET_MODE_SIZE (SImode)
> -	   && TARGET_EFFICIENT_OVERLAPPING_UNALIGNED
> +	   && TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT
>  	   && offset >= GET_MODE_SIZE (SImode) - bytes)
>      /* This matches the case were we have SImode and 3 bytes
>         and offset >= 1 and permits us to move back one and overlap
> @@ -313,7 +313,7 @@ select_block_compare_mode (unsigned HOST_WIDE_INT offset,
>         unwanted bytes off of the input.  */
>      return SImode;
>    else if (word_mode_ok && bytes < UNITS_PER_WORD
> -	   && TARGET_EFFICIENT_OVERLAPPING_UNALIGNED
> +	   && TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT
>  	   && offset >= UNITS_PER_WORD-bytes)
>      /* Similarly, if we can use DImode it will get matched here and
>         can do an overlapping read that ends at the end of the block.  */
> @@ -1749,7 +1749,7 @@ expand_block_compare_gpr(unsigned HOST_WIDE_INT bytes, unsigned int base_align,
>        load_mode_size = GET_MODE_SIZE (load_mode);
>        if (bytes >= load_mode_size)
>  	cmp_bytes = load_mode_size;
> -      else if (TARGET_EFFICIENT_OVERLAPPING_UNALIGNED)
> +      else if (TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT)
>  	{
>  	  /* Move this load back so it doesn't go past the end.
>  	     P8/P9 can do this efficiently.  */
> @@ -2026,7 +2026,7 @@ expand_block_compare (rtx operands[])
>    /* The code generated for p7 and older is not faster than glibc
>       memcmp if alignment is small and length is not short, so bail
>       out to avoid those conditions.  */
> -  if (!TARGET_EFFICIENT_OVERLAPPING_UNALIGNED
> +  if (!TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT
>        && ((base_align == 1 && bytes > 16)
>  	  || (base_align == 2 && bytes > 32)))
>      return false;
> @@ -2168,7 +2168,7 @@ expand_strncmp_gpr_sequence (unsigned HOST_WIDE_INT bytes_to_compare,
>        load_mode_size = GET_MODE_SIZE (load_mode);
>        if (bytes_to_compare >= load_mode_size)
>  	cmp_bytes = load_mode_size;
> -      else if (TARGET_EFFICIENT_OVERLAPPING_UNALIGNED)
> +      else if (TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT)
>  	{
>  	  /* Move this load back so it doesn't go past the end.
>  	     P8/P9 can do this efficiently.  */
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 326c45221e9..2f3a82942c1 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -483,9 +483,9 @@ extern int rs6000_vector_align[];
>  #define TARGET_NO_SF_SUBREG	TARGET_DIRECT_MOVE_64BIT
>  #define TARGET_ALLOW_SF_SUBREG	(!TARGET_DIRECT_MOVE_64BIT)
> 
> -/* This wants to be set for p8 and newer.  On p7, overlapping unaligned
> -   loads are slow. */
> -#define TARGET_EFFICIENT_OVERLAPPING_UNALIGNED TARGET_EFFICIENT_UNALIGNED_VSX
> +/* Like TARGET_EFFICIENT_UNALIGNED_VSX, indicates if unaligned fixed point
> +   loads/stores are efficient.  */
> +#define TARGET_EFFICIENT_UNALIGNED_FIXEDPOINT (!STRICT_ALIGNMENT)
> 
>  /* Byte/char syncs were added as phased in for ISA 2.06B, but are not present
>     in power7, so conditionalize them on p8 features.  TImode syncs need quad
> diff --git a/gcc/testsuite/gcc.target/powerpc/target_efficient_unaligned_fixedpoint-1.c b/gcc/testsuite/gcc.target/powerpc/target_efficient_unaligned_fixedpoint-1.c
> new file mode 100644
> index 00000000000..e090755e23f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/target_efficient_unaligned_fixedpoint-1.c

Nit: I found we have block-move-{1,2}, can we make these cases as block-cmp-{1,2}.c,
put some details as comments in these test cases themselves.  The potential future
test cases for this block comparison enhancement can have the same naming style then.

> @@ -0,0 +1,8 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -mdejagnu-cpu=power8 -mno-vsx" } */
> +/* { dg-final { scan-assembler-not {\mbl memcmp\M} } }  */
> +
> +int foo (const char* s1, const char* s2)
> +{
> +  return __builtin_memcmp (s1, s2, 20);
> +}
> diff --git a/gcc/testsuite/gcc.target/powerpc/target_efficient_unaligned_fixedpoint-2.c b/gcc/testsuite/gcc.target/powerpc/target_efficient_unaligned_fixedpoint-2.c
> new file mode 100644
> index 00000000000..c7ebcd90a0b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/target_efficient_unaligned_fixedpoint-2.c
> @@ -0,0 +1,8 @@
> +/* { dg-do compile { target le } } */

Since STRICT_ALIGNMENT is defined as 0 on non-sysv4, I'm not sure if it's possible to
have such combination (in future) and make it fail, maybe we can instead add with one
explicit -mstrict-align and check.

BR,
Kewen

> +/* { dg-options "-O2 -mdejagnu-cpu=power7 -mno-vsx" } */
> +/* { dg-final { scan-assembler-times {\mbl memcmp\M} 1 } }  */
> +
> +int foo (const char* s1, const char* s2)
> +{
> +  return __builtin_memcmp (s1, s2, 20);
> +}
>