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 <andre.simoesdiasvie...@arm.com> >> >> * 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,