From patchwork Thu Dec 1 17:25:11 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Andre Vieira (lists)" X-Patchwork-Id: 701614 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3tV41d3m3mz9ssP for ; Fri, 2 Dec 2016 04:25:40 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b="DSMHMkzT"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=c0DyBYFqS9g+8njTq if2FsqdIVZL825LWyc4C8sz+tdubxIZgPu+NCjF8b/yTs5ce1oYaeB8XPGn35wZw dZleIQg0ADwuptmA4nkKxWJpK3pFrUUlE601S4MRRBk9/0yjmLyHnEYSDEtCgSeB KebGU/NZpYJPMJ44Ez3P9qJmXo= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:cc:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=dkxgQbcA2QS3kLMZFnoeU9T 8ItQ=; b=DSMHMkzTgJL3olI+QFBPbJLEiMTPCBIyT1LOjFJMIlkPxGZhEDvNskZ TfUxoE8V3425tjQ9zXx06lAN1JiJt7L55aaA09XcVCOCYDt0LEV5VOLfypDDHqmg 6DougPLEkLXDFc9eF0fCkCmYxXa1hvOoEXo62tl8jxx9wPDhPSCM= Received: (qmail 72955 invoked by alias); 1 Dec 2016 17:25:27 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 72776 invoked by uid 89); 1 Dec 2016 17:25:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=nx, NX, Fall, signedness X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 01 Dec 2016 17:25:15 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id BB07516; Thu, 1 Dec 2016 09:25:13 -0800 (PST) Received: from [10.2.206.251] (e107157-lin.cambridge.arm.com [10.2.206.251]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 2A0A73F318; Thu, 1 Dec 2016 09:25:13 -0800 (PST) Subject: Re: [PATCH 1/6][ARM] Refactor NEON builtin framework to work for other builtins To: Kyrill Tkachov , gcc-patches@gcc.gnu.org References: <5822F3CB.3040202@arm.com> <5822F636.6090903@arm.com> <582D8990.3050006@foss.arm.com> Cc: Ramana Radhakrishnan From: "Andre Vieira (lists)" Message-ID: <58405CF7.3040009@arm.com> Date: Thu, 1 Dec 2016 17:25:11 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <582D8990.3050006@foss.arm.com> X-IsSubscribed: yes On 17/11/16 10:42, Kyrill Tkachov wrote: > Hi Andre, > > On 09/11/16 10:11, Andre Vieira (lists) wrote: >> Hi, >> >> Refactor NEON builtin framework such that it can be used to implement >> other builtins. >> >> Is this OK for trunk? >> >> Regards, >> Andre >> >> gcc/ChangeLog >> 2016-11-09 Andre Vieira >> >> * config/arm/arm-builtins.c (neon_builtin_datum): Rename to .. >> (arm_builtin_datum): ... this. >> (arm_init_neon_builtin): Rename to ... >> (arm_init_builtin): ... this. Add a new parameters PREFIX >> and USE_SIG_IN_NAME. >> (arm_init_neon_builtins): Replace 'arm_init_neon_builtin' with >> 'arm_init_builtin'. Replace type 'neon_builtin_datum' with >> 'arm_builtin_datum'. >> (arm_init_vfp_builtins): Likewise. >> (builtin_arg): Rename enum's replacing 'NEON_ARG' with >> 'ARG_BUILTIN' and add a 'ARG_BUILTIN_NEON_MEMORY. >> (arm_expand_neon_args): Rename to ... >> (arm_expand_builtin_args): ... this. Rename builtin_arg >> enum values and differentiate between ARG_BUILTIN_MEMORY >> and ARG_BUILTIN_NEON_MEMORY. >> (arm_expand_neon_builtin_1): Rename to ... >> (arm_expand_builtin_1): ... this. Rename builtin_arg enum >> values, arm_expand_builtin_args and add bool parameter NEON. >> (arm_expand_neon_builtin): Use arm_expand_builtin_1. >> (arm_expand_vfp_builtin): Likewise. >> (NEON_MAX_BUILTIN_ARGS): Remove, it was unused. > > /* Expand a neon builtin. This is also used for vfp builtins, which > behave in > the same way. These builtins are "special" because they don't have > symbolic > constants defined per-instruction or per instruction-variant. > Instead, the > - required info is looked up in the NEON_BUILTIN_DATA record that is > passed > + required info is looked up in the ARM_BUILTIN_DATA record that is > passed > into the function. */ > > > The comment should be updated now that it's not just NEON builtins that > are expanded through this function. > > static rtx > -arm_expand_neon_builtin_1 (int fcode, tree exp, rtx target, > - neon_builtin_datum *d) > +arm_expand_builtin_1 (int fcode, tree exp, rtx target, > + arm_builtin_datum *d, bool neon) > { > > I'm not a fan of this 'neon' boolean as it can cause confusion among the > users of the function > (see long thread at https://gcc.gnu.org/ml/gcc/2016-10/msg00004.html). > Whether the builtin is a NEON/VFP builtin > can be distinguished from FCODE, so lets just make that bool neon a > local variable and initialise it accordingly > from FCODE. > > Same for: > +/* Set up a builtin. It will use information stored in the argument > struct D to > + derive the builtin's type signature and name. It will append the > name in D > + to the PREFIX passed and use these to create a builtin declaration > that is > + then stored in 'arm_builtin_decls' under index FCODE. This FCODE is > also > + written back to D for future use. If USE_SIG_IN_NAME is true the > builtin's > + name is appended with type signature information to distinguish between > + signedness and poly. */ > > static void > -arm_init_neon_builtin (unsigned int fcode, > - neon_builtin_datum *d) > +arm_init_builtin (unsigned int fcode, arm_builtin_datum *d, > + const char * prefix, bool use_sig_in_name) > > use_sig_in_name is dependent on FCODE so just deduce it from that > locally in arm_init_builtin. > > This is ok otherwise. > Thanks, > Kyrill > > Hi, Reworked patch according to comments. No changes to ChangeLog. Is this OK? Cheers, Andre diff --git a/gcc/config/arm/arm-builtins.c b/gcc/config/arm/arm-builtins.c index 5ed38d1608cfbfbd1248d76705fcf675bc36c2b2..da6331fdc729461adeb81d84c0c425bc45b80b8c 100644 --- a/gcc/config/arm/arm-builtins.c +++ b/gcc/config/arm/arm-builtins.c @@ -202,7 +202,7 @@ typedef struct { const enum insn_code code; unsigned int fcode; enum arm_type_qualifiers *qualifiers; -} neon_builtin_datum; +} arm_builtin_datum; #define CF(N,X) CODE_FOR_neon_##N##X @@ -242,7 +242,7 @@ typedef struct { VAR11 (T, N, A, B, C, D, E, F, G, H, I, J, K) \ VAR1 (T, N, L) -/* The NEON builtin data can be found in arm_neon_builtins.def and +/* The builtin data can be found in arm_neon_builtins.def, arm_vfp_builtins.def. The entries in arm_neon_builtins.def require TARGET_NEON to be true. The feature tests are checked when the builtins are expanded. @@ -252,14 +252,14 @@ typedef struct { would be specified after the assembler mnemonic, which usually refers to the last vector operand. The modes listed per instruction should be the same as those defined for that - instruction's pattern in neon.md. */ + instruction's pattern, for instance in neon.md. */ -static neon_builtin_datum vfp_builtin_data[] = +static arm_builtin_datum vfp_builtin_data[] = { #include "arm_vfp_builtins.def" }; -static neon_builtin_datum neon_builtin_data[] = +static arm_builtin_datum neon_builtin_data[] = { #include "arm_neon_builtins.def" }; @@ -916,11 +916,15 @@ arm_init_simd_builtin_scalar_types (void) "__builtin_neon_uti"); } -/* Set up a NEON builtin. */ +/* Set up a builtin. It will use information stored in the argument struct D to + derive the builtin's type signature and name. It will append the name in D + to the PREFIX passed and use these to create a builtin declaration that is + then stored in 'arm_builtin_decls' under index FCODE. This FCODE is also + written back to D for future use. */ static void -arm_init_neon_builtin (unsigned int fcode, - neon_builtin_datum *d) +arm_init_builtin (unsigned int fcode, arm_builtin_datum *d, + const char * prefix) { bool print_type_signature_p = false; char type_signature[SIMD_MAX_BUILTIN_ARGS] = { 0 }; @@ -1008,12 +1012,13 @@ arm_init_neon_builtin (unsigned int fcode, gcc_assert (ftype != NULL); - if (print_type_signature_p) - snprintf (namebuf, sizeof (namebuf), "__builtin_neon_%s_%s", - d->name, type_signature); + if (print_type_signature_p + && IN_RANGE (fcode, ARM_BUILTIN_VFP_BASE, ARM_BUILTIN_MAX - 1)) + snprintf (namebuf, sizeof (namebuf), "%s_%s_%s", + prefix, d->name, type_signature); else - snprintf (namebuf, sizeof (namebuf), "__builtin_neon_%s", - d->name); + snprintf (namebuf, sizeof (namebuf), "%s_%s", + prefix, d->name); fndecl = add_builtin_function (namebuf, ftype, fcode, BUILT_IN_MD, NULL, NULL_TREE); @@ -1049,8 +1054,8 @@ arm_init_neon_builtins (void) for (i = 0; i < ARRAY_SIZE (neon_builtin_data); i++, fcode++) { - neon_builtin_datum *d = &neon_builtin_data[i]; - arm_init_neon_builtin (fcode, d); + arm_builtin_datum *d = &neon_builtin_data[i]; + arm_init_builtin (fcode, d, "__builtin_neon"); } } @@ -1063,8 +1068,8 @@ arm_init_vfp_builtins (void) for (i = 0; i < ARRAY_SIZE (vfp_builtin_data); i++, fcode++) { - neon_builtin_datum *d = &vfp_builtin_data[i]; - arm_init_neon_builtin (fcode, d); + arm_builtin_datum *d = &vfp_builtin_data[i]; + arm_init_builtin (fcode, d, "__builtin_neon"); } } @@ -2017,15 +2022,15 @@ arm_expand_unop_builtin (enum insn_code icode, } 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 + ARG_BUILTIN_COPY_TO_REG, + ARG_BUILTIN_CONSTANT, + ARG_BUILTIN_LANE_INDEX, + ARG_BUILTIN_STRUCT_LOAD_STORE_LANE_INDEX, + ARG_BUILTIN_NEON_MEMORY, + ARG_BUILTIN_MEMORY, + ARG_BUILTIN_STOP } builtin_arg; -#define NEON_MAX_BUILTIN_ARGS 5 /* EXP is a pointer argument to a Neon load or store intrinsic. Derive and return an expression for the accessed memory. @@ -2075,9 +2080,9 @@ neon_dereference_pointer (tree exp, tree type, machine_mode mem_mode, build_int_cst (build_pointer_type (array_type), 0)); } -/* Expand a Neon builtin. */ +/* Expand a builtin. */ static rtx -arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode, +arm_expand_builtin_args (rtx target, machine_mode map_mode, int fcode, int icode, int have_retval, tree exp, builtin_arg *args) { @@ -2101,14 +2106,14 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode, { builtin_arg thisarg = args[argc]; - if (thisarg == NEON_ARG_STOP) + if (thisarg == ARG_BUILTIN_STOP) break; else { int opno = argc + have_retval; arg[argc] = CALL_EXPR_ARG (exp, argc); mode[argc] = insn_data[icode].operand[opno].mode; - if (thisarg == NEON_ARG_MEMORY) + if (thisarg == ARG_BUILTIN_NEON_MEMORY) { machine_mode other_mode = insn_data[icode].operand[1 - opno].mode; @@ -2118,15 +2123,17 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode, map_mode); } - /* Use EXPAND_MEMORY for NEON_ARG_MEMORY to ensure a MEM_P - be returned. */ + /* Use EXPAND_MEMORY for ARG_BUILTIN_MEMORY and + ARG_BUILTIN_NEON_MEMORY to ensure a MEM_P be returned. */ op[argc] = expand_expr (arg[argc], NULL_RTX, VOIDmode, - (thisarg == NEON_ARG_MEMORY + ((thisarg == ARG_BUILTIN_MEMORY + || thisarg == ARG_BUILTIN_NEON_MEMORY) ? EXPAND_MEMORY : EXPAND_NORMAL)); switch (thisarg) { - case NEON_ARG_COPY_TO_REG: + case ARG_BUILTIN_MEMORY: + case ARG_BUILTIN_COPY_TO_REG: if (POINTER_TYPE_P (TREE_TYPE (arg[argc]))) op[argc] = convert_memory_address (Pmode, op[argc]); /*gcc_assert (GET_MODE (op[argc]) == mode[argc]); */ @@ -2135,7 +2142,7 @@ 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: + case ARG_BUILTIN_STRUCT_LOAD_STORE_LANE_INDEX: gcc_assert (argc > 1); if (CONST_INT_P (op[argc])) { @@ -2147,7 +2154,7 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode, } goto constant_arg; - case NEON_ARG_LANE_INDEX: + case ARG_BUILTIN_LANE_INDEX: /* Previous argument must be a vector, which this indexes. */ gcc_assert (argc > 0); if (CONST_INT_P (op[argc])) @@ -2158,7 +2165,7 @@ arm_expand_neon_args (rtx target, machine_mode map_mode, int fcode, /* If the lane index isn't a constant then the next case will error. */ /* Fall through. */ - case NEON_ARG_CONSTANT: + case ARG_BUILTIN_CONSTANT: constant_arg: if (!(*insn_data[icode].operand[opno].predicate) (op[argc], mode[argc])) @@ -2169,7 +2176,7 @@ constant_arg: } break; - case NEON_ARG_MEMORY: + case ARG_BUILTIN_NEON_MEMORY: /* Check if expand failed. */ if (op[argc] == const0_rtx) return 0; @@ -2186,7 +2193,7 @@ constant_arg: copy_to_mode_reg (Pmode, XEXP (op[argc], 0)))); break; - case NEON_ARG_STOP: + case ARG_BUILTIN_STOP: gcc_unreachable (); } @@ -2255,21 +2262,24 @@ constant_arg: return target; } -/* Expand a neon builtin. This is also used for vfp builtins, which behave in - the same way. These builtins are "special" because they don't have symbolic - constants defined per-instruction or per instruction-variant. Instead, the - required info is looked up in the NEON_BUILTIN_DATA record that is passed - into the function. */ +/* Expand a builtin. These builtins are "special" because they don't have + symbolic constants defined per-instruction or per instruction-variant. + Instead, the required info is looked up in the ARM_BUILTIN_DATA record that + is passed into the function. */ static rtx -arm_expand_neon_builtin_1 (int fcode, tree exp, rtx target, - neon_builtin_datum *d) +arm_expand_builtin_1 (int fcode, tree exp, rtx target, + arm_builtin_datum *d) { enum insn_code icode = d->code; builtin_arg args[SIMD_MAX_BUILTIN_ARGS + 1]; int num_args = insn_data[d->code].n_operands; int is_void = 0; int k; + bool neon = false; + + if (IN_RANGE (fcode, ARM_BUILTIN_VFP_BASE, ARM_BUILTIN_MAX - 1)) + neon = true; is_void = !!(d->qualifiers[0] & qualifier_void); @@ -2289,11 +2299,11 @@ arm_expand_neon_builtin_1 (int fcode, tree exp, rtx target, int expr_args_k = k - 1; if (d->qualifiers[qualifiers_k] & qualifier_lane_index) - args[k] = NEON_ARG_LANE_INDEX; + args[k] = ARG_BUILTIN_LANE_INDEX; else if (d->qualifiers[qualifiers_k] & qualifier_struct_load_store_lane_index) - args[k] = NEON_ARG_STRUCT_LOAD_STORE_LANE_INDEX; + args[k] = ARG_BUILTIN_STRUCT_LOAD_STORE_LANE_INDEX; else if (d->qualifiers[qualifiers_k] & qualifier_immediate) - args[k] = NEON_ARG_CONSTANT; + args[k] = ARG_BUILTIN_CONSTANT; else if (d->qualifiers[qualifiers_k] & qualifier_maybe_immediate) { rtx arg @@ -2304,18 +2314,23 @@ arm_expand_neon_builtin_1 (int fcode, tree exp, rtx target, (CONST_INT_P (arg) && (*insn_data[icode].operand[operands_k].predicate) (arg, insn_data[icode].operand[operands_k].mode)); - args[k] = op_const_int_p ? NEON_ARG_CONSTANT : NEON_ARG_COPY_TO_REG; + args[k] = op_const_int_p ? ARG_BUILTIN_CONSTANT : ARG_BUILTIN_COPY_TO_REG; } else if (d->qualifiers[qualifiers_k] & qualifier_pointer) - args[k] = NEON_ARG_MEMORY; + { + if (neon) + args[k] = ARG_BUILTIN_NEON_MEMORY; + else + args[k] = ARG_BUILTIN_MEMORY; + } else - args[k] = NEON_ARG_COPY_TO_REG; + args[k] = ARG_BUILTIN_COPY_TO_REG; } - args[k] = NEON_ARG_STOP; + args[k] = ARG_BUILTIN_STOP; - /* The interface to arm_expand_neon_args expects a 0 if + /* The interface to arm_expand_builtin_args expects a 0 if the function is void, and a 1 if it is not. */ - return arm_expand_neon_args + return arm_expand_builtin_args (target, d->mode, fcode, icode, !is_void, exp, &args[1]); } @@ -2353,10 +2368,10 @@ arm_expand_neon_builtin (int fcode, tree exp, rtx target) return const0_rtx; } - neon_builtin_datum *d + arm_builtin_datum *d = &neon_builtin_data[fcode - ARM_BUILTIN_NEON_PATTERN_START]; - return arm_expand_neon_builtin_1 (fcode, exp, target, d); + return arm_expand_builtin_1 (fcode, exp, target, d); } /* Expand a VFP builtin. These builtins are treated like @@ -2374,10 +2389,10 @@ arm_expand_vfp_builtin (int fcode, tree exp, rtx target) return const0_rtx; } - neon_builtin_datum *d + arm_builtin_datum *d = &vfp_builtin_data[fcode - ARM_BUILTIN_VFP_PATTERN_START]; - return arm_expand_neon_builtin_1 (fcode, exp, target, d); + return arm_expand_builtin_1 (fcode, exp, target, d); } /* Expand an expression EXP that calls a built-in function,