On Tue, Oct 15, 2024 at 4:34 AM Wilco Dijkstra <wilco.dijks...@arm.com> wrote:
>
>
> Add support for SVE xor immediate when generating AdvSIMD code and SVE is
> available.
>
> Passes bootstrap & regress, OK for commit?
>
> gcc/ChangeLog:
>
> * config/aarch64/aarch64.cc (enum simd_immediate_check): Add
> AARCH64_CHECK_XOR.
> (aarch64_simd_valid_xor_imm): New function.
> (aarch64_output_simd_imm): Add AARCH64_CHECK_XOR support.
> (aarch64_output_simd_xor_imm): New function.
> * config/aarch64/aarch64-protos.h (aarch64_output_simd_xor_imm): New
> prototype.
> (aarch64_simd_valid_xor_imm): New prototype.
> * config/aarch64/aarch64-simd.md (xor<mode>3<vczle><vczbe>):
> Use aarch64_reg_or_xor_imm predicate and add an immediate alternative.
> * config/aarch64/predicates.md (aarch64_reg_or_xor_imm): Add new
> predicate.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.target/aarch64/sve/simd_imm.c: New test.
>
> ---
>
> diff --git a/gcc/config/aarch64/aarch64-protos.h
> b/gcc/config/aarch64/aarch64-protos.h
> index
> 3f2d40603426a590a0a14ba4792fe9b325d1e585..16ab79c02da62c1a8aa03309708dfe401d1ffb7e
> 100644
> --- a/gcc/config/aarch64/aarch64-protos.h
> +++ b/gcc/config/aarch64/aarch64-protos.h
> @@ -827,6 +827,7 @@ char *aarch64_output_scalar_simd_mov_immediate (rtx,
> scalar_int_mode);
> char *aarch64_output_simd_mov_imm (rtx, unsigned);
> char *aarch64_output_simd_orr_imm (rtx, unsigned);
> char *aarch64_output_simd_and_imm (rtx, unsigned);
> +char *aarch64_output_simd_xor_imm (rtx, unsigned);
>
> char *aarch64_output_sve_mov_immediate (rtx);
> char *aarch64_output_sve_ptrues (rtx);
> @@ -844,6 +845,7 @@ bool aarch64_sve_ptrue_svpattern_p (rtx, struct
> simd_immediate_info *);
> bool aarch64_simd_valid_and_imm (rtx);
> bool aarch64_simd_valid_mov_imm (rtx);
> bool aarch64_simd_valid_orr_imm (rtx);
> +bool aarch64_simd_valid_xor_imm (rtx);
> bool aarch64_valid_sysreg_name_p (const char *);
> const char *aarch64_retrieve_sysreg (const char *, bool, bool);
> rtx aarch64_check_zero_based_sve_index_immediate (rtx);
> diff --git a/gcc/config/aarch64/aarch64-simd.md
> b/gcc/config/aarch64/aarch64-simd.md
> index
> 5c1de57ce6c3f2064d8be25f903a6a8d949685ef..18795a08b61da874a9e811822ed82e7eb9350bb4
> 100644
> --- a/gcc/config/aarch64/aarch64-simd.md
> +++ b/gcc/config/aarch64/aarch64-simd.md
> @@ -1144,12 +1144,16 @@ (define_insn "ior<mode>3<vczle><vczbe>"
> [(set_attr "type" "neon_logic<q>")]
> )
>
> +;; For EOR (vector, register) and SVE EOR (vector, immediate)
> (define_insn "xor<mode>3<vczle><vczbe>"
> - [(set (match_operand:VDQ_I 0 "register_operand" "=w")
> - (xor:VDQ_I (match_operand:VDQ_I 1 "register_operand" "w")
> - (match_operand:VDQ_I 2 "register_operand" "w")))]
> + [(set (match_operand:VDQ_I 0 "register_operand")
> + (xor:VDQ_I (match_operand:VDQ_I 1 "register_operand")
> + (match_operand:VDQ_I 2 "aarch64_reg_or_xor_imm")))]
> "TARGET_SIMD"
> - "eor\t%0.<Vbtype>, %1.<Vbtype>, %2.<Vbtype>"
> + {@ [ cons: =0 , 1 , 2 ]
> + [ w , w , w ] eor\t%0.<Vbtype>, %1.<Vbtype>, %2.<Vbtype>
> + [ w , 0 , Do ] << aarch64_output_simd_xor_imm (operands[2],
> <bitsize>);
The constraint here allows slightly more than the predicate which
definitely can cause issues with the register allocator when it comes
to constants.
See PR 117292 which has a testcase which shows how the register
allocator will choose `Do` alternative while not checking the
predicate.
Thanks,
Andrew
> + }
> [(set_attr "type" "neon_logic<q>")]
> )
>
> diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
> index
> 1a228147e6f945772edbd5540c44167e3a876a74..c019f21e39d9773746792d5885fa0f6805f9bb44
> 100644
> --- a/gcc/config/aarch64/aarch64.cc
> +++ b/gcc/config/aarch64/aarch64.cc
> @@ -134,7 +134,8 @@ constexpr auto AARCH64_STATE_OUT = 1U << 2;
> enum simd_immediate_check {
> AARCH64_CHECK_MOV,
> AARCH64_CHECK_ORR,
> - AARCH64_CHECK_AND
> + AARCH64_CHECK_AND,
> + AARCH64_CHECK_XOR
> };
>
> /* Information about a legitimate vector immediate operand. */
> @@ -23320,6 +23321,13 @@ aarch64_simd_valid_and_imm (rtx op)
> return aarch64_simd_valid_imm (op, NULL, AARCH64_CHECK_AND);
> }
>
> +/* Return true if OP is a valid SIMD xor immediate for SVE. */
> +bool
> +aarch64_simd_valid_xor_imm (rtx op)
> +{
> + return aarch64_simd_valid_imm (op, NULL, AARCH64_CHECK_XOR);
> +}
> +
> /* Check whether X is a VEC_SERIES-like constant that starts at 0 and
> has a step in the range of INDEX. Return the index expression if so,
> otherwise return null. */
> @@ -25503,10 +25511,12 @@ aarch64_output_simd_imm (rtx const_vector, unsigned
> width,
> }
> else
> {
> - /* AARCH64_CHECK_ORR or AARCH64_CHECK_AND. */
> + /* AARCH64_CHECK_ORR, AARCH64_CHECK_AND or AARCH64_CHECK_XOR. */
> mnemonic = "orr";
> if (which == AARCH64_CHECK_AND)
> mnemonic = info.insn == simd_immediate_info::MVN ? "bic" : "and";
> + else if (which == AARCH64_CHECK_XOR)
> + mnemonic = "eor";
>
> if (info.insn == simd_immediate_info::SVE_MOV)
> {
> @@ -25544,6 +25554,14 @@ aarch64_output_simd_and_imm (rtx const_vector,
> unsigned width)
> return aarch64_output_simd_imm (const_vector, width, AARCH64_CHECK_AND);
> }
>
> +/* Returns the string with the EOR instruction for the SIMD immediate
> + CONST_VECTOR of WIDTH bits. */
> +char*
> +aarch64_output_simd_xor_imm (rtx const_vector, unsigned width)
> +{
> + return aarch64_output_simd_imm (const_vector, width, AARCH64_CHECK_XOR);
> +}
> +
> /* Returns the string with the MOV instruction for the SIMD immediate
> CONST_VECTOR of WIDTH bits. */
> char*
> diff --git a/gcc/config/aarch64/predicates.md
> b/gcc/config/aarch64/predicates.md
> index
> 2c18af94b8eca7a7985a238a4de8c5d0b3766acb..6ad9a4bd8b92fcbe10260093abf1331c909a474a
> 100644
> --- a/gcc/config/aarch64/predicates.md
> +++ b/gcc/config/aarch64/predicates.md
> @@ -125,6 +125,11 @@ (define_predicate "aarch64_reg_or_and_imm"
> (and (match_code "const_vector")
> (match_test "aarch64_simd_valid_and_imm (op)"))))
>
> +(define_predicate "aarch64_reg_or_xor_imm"
> + (ior (match_operand 0 "register_operand")
> + (and (match_code "const_vector")
> + (match_test "aarch64_simd_valid_xor_imm (op)"))))
> +
> (define_predicate "aarch64_fp_compare_operand"
> (ior (match_operand 0 "register_operand")
> (and (match_code "const_double")
> diff --git a/gcc/testsuite/gcc.target/aarch64/sve/simd_imm.c
> b/gcc/testsuite/gcc.target/aarch64/sve/simd_imm.c
> new file mode 100644
> index
> 0000000000000000000000000000000000000000..69ece074eed03c1b1779eea979318343b8bd7b61
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/aarch64/sve/simd_imm.c
> @@ -0,0 +1,35 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { check-function-bodies "**" "" "" } } */
> +
> +#include <arm_neon.h>
> +
> +/*
> +** t1:
> +** and z[0-9]+.s, z[0-9]+.s, #?3
> +** ret
> +*/
> +uint32x2_t t1 (uint32x2_t a)
> +{
> + return vand_u32 (a, vdup_n_u32 (3));
> +}
> +
> +/*
> +** t2:
> +** orr z[0-9]+.s, z[0-9]+.s, #?-3
> +** ret
> +*/
> +uint32x2_t t2 (uint32x2_t a)
> +{
> + return vorr_u32 (a, vdup_n_u32 (~2));
> +}
> +
> +/*
> +** t3:
> +** eor z[0-9]+.s, z[0-9]+.s, #?3
> +** ret
> +*/
> +uint32x2_t t3 (uint32x2_t a)
> +{
> + return veor_u32 (a, vdup_n_u32 (3));
> +}
>
