This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/4] [ARM] PR63870 Add qualifiers for NEON builtins
- From: Ramana Radhakrishnan <ramana dot radhakrishnan at foss dot arm dot com>
- To: charles dot baylis at linaro dot org, kyrylo dot tkachov at arm dot com, alan dot lawrence at arm dot com
- Cc: rearnsha at arm dot com, gcc-patches at gcc dot gnu dot org
- Date: Mon, 9 Nov 2015 09:03:03 +0000
- Subject: Re: [PATCH 1/4] [ARM] PR63870 Add qualifiers for NEON builtins
- Authentication-results: sourceware.org; auth=none
- References: <1446942404-11561-1-git-send-email-charles dot baylis at linaro dot org> <1446942404-11561-2-git-send-email-charles dot baylis at linaro dot org>
On 08/11/15 00:26, charles.baylis@linaro.org wrote:
> From: Charles Baylis <charles.baylis@linaro.org>
>
> gcc/ChangeLog:
>
> <DATE> Charles Baylis <charles.baylis@linaro.org>
>
> PR target/63870
> * config/arm/arm-builtins.c (enum arm_type_qualifiers): New enumerator
> qualifier_struct_load_store_lane_index.
> (builtin_arg): New enumerator NEON_ARG_STRUCT_LOAD_STORE_LANE_INDEX.
> (arm_expand_neon_args): New parameter. Remove ellipsis. Handle NEON
> argument qualifiers.
> (arm_expand_neon_builtin): Handle new NEON argument qualifier.
> * config/arm/arm.h (ENDIAN_LANE_N): New macro.
>
> Change-Id: Iaa14d8736879fa53776319977eda2089f0a26647
> ---
> gcc/config/arm/arm-builtins.c | 48 +++++++++++++++++++++++++++----------------
> gcc/config/arm/arm.c | 1 +
> gcc/config/arm/arm.h | 3 +++
> 3 files changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c
> index bad3dc3..6e3aad4 100644
> --- a/gcc/config/arm/arm-builtins.c
> +++ b/gcc/config/arm/arm-builtins.c
> @@ -67,7 +67,9 @@ enum arm_type_qualifiers
> /* Polynomial types. */
> qualifier_poly = 0x100,
> /* Lane indices - must be within range of previous argument = a vector. */
> - qualifier_lane_index = 0x200
> + qualifier_lane_index = 0x200,
> + /* Lane indices for single lane structure loads and stores. */
> + qualifier_struct_load_store_lane_index = 0x400
> };
>
> /* The qualifier_internal allows generation of a unary builtin from
> @@ -1963,6 +1965,7 @@ typedef enum {
> NEON_ARG_COPY_TO_REG,
> NEON_ARG_CONSTANT,
> NEON_ARG_LANE_INDEX,
> + NEON_ARG_STRUCT_LOAD_STORE_LANE_INDEX,
> NEON_ARG_MEMORY,
> NEON_ARG_STOP
> } builtin_arg;
> @@ -2020,9 +2023,9 @@ neon_dereference_pointer (tree exp, tree type, machine_mode mem_mode,
> /* Expand a Neon builtin. */
> static rtx
> arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode,
> - int icode, int have_retval, tree exp, ...)
> + int icode, int have_retval, tree exp,
> + builtin_arg *args)
> {
> - va_list ap;
> rtx pat;
> tree arg[SIMD_MAX_BUILTIN_ARGS];
> rtx op[SIMD_MAX_BUILTIN_ARGS];
> @@ -2037,13 +2040,11 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode,
> || !(*insn_data[icode].operand[0].predicate) (target, tmode)))
> target = gen_reg_rtx (tmode);
>
> - va_start (ap, exp);
> -
> formals = TYPE_ARG_TYPES (TREE_TYPE (arm_builtin_decls[fcode]));
>
> for (;;)
> {
> - builtin_arg thisarg = (builtin_arg) va_arg (ap, int);
> + builtin_arg thisarg = args[argc];
>
> if (thisarg == NEON_ARG_STOP)
> break;
> @@ -2079,6 +2080,18 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode,
> op[argc] = copy_to_mode_reg (mode[argc], op[argc]);
> break;
>
> + case NEON_ARG_STRUCT_LOAD_STORE_LANE_INDEX:
> + gcc_assert (argc > 1);
> + if (CONST_INT_P (op[argc]))
> + {
> + neon_lane_bounds (op[argc], 0,
> + GET_MODE_NUNITS (map_mode), exp);
> + /* Keep to GCC-vector-extension lane indices in the RTL. */
> + op[argc] =
> + GEN_INT (ENDIAN_LANE_N (map_mode, INTVAL (op[argc])));
> + }
> + goto constant_arg;
> +
> case NEON_ARG_LANE_INDEX:
> /* Previous argument must be a vector, which this indexes. */
> gcc_assert (argc > 0);
> @@ -2089,19 +2102,22 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode,
> }
> /* Fall through - if the lane index isn't a constant then
> the next case will error. */
> +
> case NEON_ARG_CONSTANT:
> +constant_arg:
> if (!(*insn_data[icode].operand[opno].predicate)
> (op[argc], mode[argc]))
> - error_at (EXPR_LOCATION (exp), "incompatible type for argument %d, "
> - "expected %<const int%>", argc + 1);
> + {
> + error ("%Kargument %d must be a constant immediate",
> + exp, argc + 1);
> + return const0_rtx;
> + }
> break;
> +
> case NEON_ARG_MEMORY:
> /* Check if expand failed. */
> if (op[argc] == const0_rtx)
> - {
> - va_end (ap);
> return 0;
> - }
> gcc_assert (MEM_P (op[argc]));
> PUT_MODE (op[argc], mode[argc]);
> /* ??? arm_neon.h uses the same built-in functions for signed
> @@ -2122,8 +2138,6 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode,
> }
> }
>
> - va_end (ap);
> -
> if (have_retval)
> switch (argc)
> {
> @@ -2235,6 +2249,8 @@ arm_expand_neon_builtin (int fcode, tree exp, rtx target)
>
> if (d->qualifiers[qualifiers_k] & qualifier_lane_index)
> args[k] = NEON_ARG_LANE_INDEX;
> + else if (d->qualifiers[qualifiers_k] & qualifier_struct_load_store_lane_index)
> + args[k] = NEON_ARG_STRUCT_LOAD_STORE_LANE_INDEX;
> else if (d->qualifiers[qualifiers_k] & qualifier_immediate)
> args[k] = NEON_ARG_CONSTANT;
> else if (d->qualifiers[qualifiers_k] & qualifier_maybe_immediate)
> @@ -2260,11 +2276,7 @@ arm_expand_neon_builtin (int fcode, tree exp, rtx target)
> the function is void, and a 1 if it is not. */
> return arm_expand_neon_args
> (target, d->mode, fcode, icode, !is_void, exp,
> - args[1],
> - args[2],
> - args[3],
> - args[4],
> - NEON_ARG_STOP);
> + &args[1]);
> }
>
> /* Expand an expression EXP that calls a built-in function,
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 61e2aa2..3d0c5d5 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -30111,4 +30111,5 @@ arm_sched_fusion_priority (rtx_insn *insn, int max_pri,
> *pri = tmp;
> return;
> }
> +
> #include "gt-arm.h"
> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> index a1a04a9..8136d2c 100644
> --- a/gcc/config/arm/arm.h
> +++ b/gcc/config/arm/arm.h
> @@ -284,6 +284,9 @@ extern void (*arm_lang_output_object_attributes_hook)(void);
> #define TARGET_BPABI false
> #endif
Missing comment and please prefix this with NEON_ or SIMD_ .
>
> +#define ENDIAN_LANE_N(mode, n) \
> + (BYTES_BIG_ENDIAN ? GET_MODE_NUNITS (mode) - 1 - n : n)
> +
Otherwise OK -
regards
Ramana