Re: [PATCH, rs6000] Enable vector compare for 16-byte memory equality compare [PR111449]

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

Hi,

on 2023/9/20 16:49, HAO CHEN GUI wrote:
> Hi,
>   This patch enables vector compare for 16-byte memory equality compare.
> The 16-byte memory equality compare can be efficiently implemented by
> instruction "vcmpequb." It reduces one branch and one compare compared
> with two 8-byte compare sequence.

It looks nice to exploit vcmpequb. for this comparison.

> 
>   16-byte vector compare is not enabled on 32bit sub-targets as TImode
> hasn't been supported well on 32bit sub-targets.

But it sounds weird to say it is with TImode but the underlying instruction
is V16QImode.  This does NOT necessarily depend on TImode, so if it's coded
with V16QImode it would not suffer this unsupported issue.

The reason why you hacked with TImode seems that the generic part of code
only considers the scalar mode?  I wonder if we can extend the generic code
to consider the vector mode as well.  It also makes thing better if we will
have wider vector mode one day.

I guess there is no blocking/limitation for not considering vector modes?
CC some experts.

BR,
Kewen

> 
>   Bootstrapped and tested on powerpc64-linux BE and LE with no regressions.
> 
> Thanks
> Gui Haochen
> 
> ChangeLog
> rs6000: Enable vector compare for 16-byte memory equality compare
> 
> gcc/
> 	PR target/111449
> 	* config/rs6000/altivec.md (cbranchti4): New expand pattern.
> 	* config/rs6000/rs6000.cc (rs6000_generate_compare): Generate insn
> 	sequence for TImode vector equality compare.
> 	* config/rs6000/rs6000.h (MOVE_MAX_PIECES): Define.
> 	(COMPARE_MAX_PIECES): Define.
> 
> gcc/testsuite/
> 	PR target/111449
> 	* gcc.target/powerpc/pr111449.c: New.
> 
> patch.diff
> diff --git a/gcc/config/rs6000/altivec.md b/gcc/config/rs6000/altivec.md
> index e8a596fb7e9..99264235cbe 100644
> --- a/gcc/config/rs6000/altivec.md
> +++ b/gcc/config/rs6000/altivec.md
> @@ -2605,6 +2605,24 @@ (define_insn "altivec_vupklpx"
>  }
>    [(set_attr "type" "vecperm")])
> 
> +(define_expand "cbranchti4"
> +  [(use (match_operator 0 "equality_operator"
> +	[(match_operand:TI 1 "memory_operand")
> +	 (match_operand:TI 2 "memory_operand")]))
> +   (use (match_operand 3))]
> +  "VECTOR_UNIT_ALTIVEC_P (V16QImode)"
> +{
> +  rtx op1 = simplify_subreg (V16QImode, operands[1], TImode, 0);
> +  rtx op2 = simplify_subreg (V16QImode, operands[2], TImode, 0);
> +  operands[1] = force_reg (V16QImode, op1);
> +  operands[2] = force_reg (V16QImode, op2);
> +  rtx_code code = GET_CODE (operands[0]);
> +  operands[0] = gen_rtx_fmt_ee (code, V16QImode, operands[1],
> +				operands[2]);
> +  rs6000_emit_cbranch (TImode, operands);
> +  DONE;
> +})
> +
>  ;; Compare vectors producing a vector result and a predicate, setting CR6 to
>  ;; indicate a combined status
>  (define_insn "altivec_vcmpequ<VI_char>_p"
> diff --git a/gcc/config/rs6000/rs6000.cc b/gcc/config/rs6000/rs6000.cc
> index efe9adce1f8..c6b935a64e7 100644
> --- a/gcc/config/rs6000/rs6000.cc
> +++ b/gcc/config/rs6000/rs6000.cc
> @@ -15264,6 +15264,15 @@ rs6000_generate_compare (rtx cmp, machine_mode mode)
>  	  else
>  	    emit_insn (gen_stack_protect_testsi (compare_result, op0, op1b));
>  	}
> +      else if (mode == TImode)
> +	{
> +	  gcc_assert (code == EQ || code == NE);
> +
> +	  rtx result_vector = gen_reg_rtx (V16QImode);
> +	  compare_result = gen_rtx_REG (CCmode, CR6_REGNO);
> +	  emit_insn (gen_altivec_vcmpequb_p (result_vector, op0, op1));
> +	  code = (code == NE) ? GE : LT;
> +	}
>        else
>  	emit_insn (gen_rtx_SET (compare_result,
>  				gen_rtx_COMPARE (comp_mode, op0, op1)));
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 3503614efbd..dc33bca0802 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -1730,6 +1730,8 @@ typedef struct rs6000_args
>     in one reasonably fast instruction.  */
>  #define MOVE_MAX (! TARGET_POWERPC64 ? 4 : 8)
>  #define MAX_MOVE_MAX 8
> +#define MOVE_MAX_PIECES (!TARGET_POWERPC64 ? 4 : 16)
> +#define COMPARE_MAX_PIECES (!TARGET_POWERPC64 ? 4 : 16)
> 
>  /* Nonzero if access to memory by bytes is no faster than for words.
>     Also nonzero if doing byte operations (specifically shifts) in registers
> diff --git a/gcc/testsuite/gcc.target/powerpc/pr111449.c b/gcc/testsuite/gcc.target/powerpc/pr111449.c
> new file mode 100644
> index 00000000000..ab9583f47bb
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr111449.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-options "-maltivec -O2" } */
> +/* { dg-require-effective-target has_arch_ppc64 } */
> +
> +/* Ensure vector comparison is used for 16-byte memory equality compare.  */
> +
> +int compare (const char* s1, const char* s2)
> +{
> +  return __builtin_memcmp (s1, s2, 16) == 0;
> +}
> +
> +/* { dg-final { scan-assembler-times {\mvcmpequb\M} 1 } } */
> +/* { dg-final { scan-assembler-not {\mcmpd\M} } } */