"Kewen.Lin" <li...@linux.ibm.com> writes:
> 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?
Yeah, I agree there doesn't seem to be a good reason to exclude vectors.
Sorry to dive straight into details, but maybe we should have something
called bitwise_mode_for_size that tries to use integer modes where possible,
but falls back to vector modes otherwise. That mode could then be used
for copying, storing, bitwise ops, and equality comparisons (if there
is appropriate optabs support).
Thanks,
Richard
> 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} } } */
