Hi Carl, on 2024/7/4 07:51, Carl Love wrote: > GCC maintainers: > > The patch has been updated to remove the customized vec_init built-in code. > Specfivically the init identifier, the related generated code for the init > built-in attribute bit, function altivec_expand_vec_init_builtin and calls to > the function. > > Please let me know if the patch is acceptable for mainline. Thanks. > > Carl > > ------------------------------------------------------------------- > > rs6000, remove vector set and vector init built-ins. > > The vector init built-ins: > > __builtin_vec_init_v16qi, __builtin_vec_init_v8hi, > __builtin_vec_init_v4si, __builtin_vec_init_v4sf, > __builtin_vec_init_v2di, __builtin_vec_init_v2df, > __builtin_vec_init_v1ti > > perform the same operation as initializing the vector in C code. For > example: > > result_v4si = __builtin_vec_init_v4si (1, 2, 3, 4); > result_v4si = {1, 2, 3, 4}; > > These two constructs were tested and verified they generate identical > assembly instructions with no optimization and -O3 optimization. > > The vector set built-ins: > > __builtin_vec_set_v16qi, __builtin_vec_set_v8hi. > __builtin_vec_set_v4si, __builtin_vec_set_v4sf, > __builtin_vec_set_v1ti, __builtin_vec_set_v2di, > __builtin_vec_set_v2df > > perform the same operation as setting a specific element in the vector in > C code. For example: > > src_v4si = __builtin_vec_set_v4si (src_v4si, int_val, index); > src_v4si[index] = int_val; > > The built-in actually generates more instructions than the inline C code > with no optimization but is identical with -O3 optimizations. > > All of the above built-ins that are removed do not have test cases and > are not documented. > > Built-ins __builtin_vec_set_v1ti __builtin_vec_set_v2di, > __builtin_vec_set_v2df are not removed as they are used in function > resolve_vec_insert() in file rs6000-c.cc. > > The built-ins are removed as they don't provide any benefit over just > using C code. > > The code to define the bif_init_bit, bif_is_init, as well as their uses > is removed. The function altivec_expand_vec_init_builtin is also removed.
Nit: s/is removed/are removed/ ? > > gcc/ChangeLog: > * config/rs6000/rs6000-builtin.cc (altivec_expand_vec_init_builtin): > Removed the function. Nit: s/Removed/Remove/, applied for the other changelog entries. > (rs6000_expand_builtin): Removed the if bif_is_int check to call > the altivec_expand_vec_init_builtin function. > * config/rs6000/rs6000-builtins.def: Removed the attribute string > comment for init. > (__builtin_vec_init_v16qi, > __builtin_vec_init_v4sf, __builtin_vec_init_v4si, > __builtin_vec_init_v8hi, __builtin_vec_init_v1ti, > __builtin_vec_init_v2df, __builtin_vec_init_v2di, > __builtin_vec_set_v16qi, __builtin_vec_set_v4sf, > __builtin_vec_set_v4si, __builtin_vec_set_v8hi): Remove > built-in definitions. > * config/rs6000-gen-builtins.cc: Removed comment for init attribute > string. > (struct attrinfo): Removed isint entry. Typo: s/isint/isinit/ > (parse_bif_attrs): Removed the if statement to check for attribute > init. > (ifdef DEBUG): Removed print for init attribute string. > (write_decls): Removed print for define bif_init_bit and > define for bif_is_init. > (write_bif_static_init): Removed if bifp->attrs.isinit statement. > --- > gcc/config/rs6000/rs6000-builtin.cc | 40 --------------------- > gcc/config/rs6000/rs6000-builtins.def | 45 +++--------------------- > gcc/config/rs6000/rs6000-gen-builtins.cc | 16 +++------ > 3 files changed, 8 insertions(+), 93 deletions(-) > > diff --git a/gcc/config/rs6000/rs6000-builtin.cc > b/gcc/config/rs6000/rs6000-builtin.cc > index 646e740774e..0a24d20a58c 100644 > --- a/gcc/config/rs6000/rs6000-builtin.cc > +++ b/gcc/config/rs6000/rs6000-builtin.cc > @@ -2313,43 +2313,6 @@ altivec_expand_predicate_builtin (enum insn_code > icode, tree exp, rtx target) > return target; > } > > -/* Expand vec_init builtin. */ > -static rtx > -altivec_expand_vec_init_builtin (tree type, tree exp, rtx target) > -{ > - machine_mode tmode = TYPE_MODE (type); > - machine_mode inner_mode = GET_MODE_INNER (tmode); > - int i, n_elt = GET_MODE_NUNITS (tmode); > - > - gcc_assert (VECTOR_MODE_P (tmode)); > - gcc_assert (n_elt == call_expr_nargs (exp)); > - > - if (!target || !register_operand (target, tmode)) > - target = gen_reg_rtx (tmode); > - > - /* If we have a vector compromised of a single element, such as V1TImode, > do > - the initialization directly. */ > - if (n_elt == 1 && GET_MODE_SIZE (tmode) == GET_MODE_SIZE (inner_mode)) > - { > - rtx x = expand_normal (CALL_EXPR_ARG (exp, 0)); > - emit_move_insn (target, gen_lowpart (tmode, x)); > - } > - else > - { > - rtvec v = rtvec_alloc (n_elt); > - > - for (i = 0; i < n_elt; ++i) > - { > - rtx x = expand_normal (CALL_EXPR_ARG (exp, i)); > - RTVEC_ELT (v, i) = gen_lowpart (inner_mode, x); > - } > - > - rs6000_expand_vector_init (target, gen_rtx_PARALLEL (tmode, v)); > - } > - > - return target; > -} > - > /* Return the integer constant in ARG. Constrain it to be in the range > of the subparts of VEC_TYPE; issue an error if not. */ > > @@ -3401,9 +3364,6 @@ rs6000_expand_builtin (tree exp, rtx target, rtx /* > subtarget */, > if (bif_is_cpu (*bifaddr)) > return cpu_expand_builtin (fcode, exp, target); > > - if (bif_is_init (*bifaddr)) > - return altivec_expand_vec_init_builtin (TREE_TYPE (exp), exp, target); > - > if (bif_is_set (*bifaddr)) > return altivec_expand_vec_set_builtin (exp); > > diff --git a/gcc/config/rs6000/rs6000-builtins.def > b/gcc/config/rs6000/rs6000-builtins.def > index 02aa04e5698..7b4ed3eecbe 100644 > --- a/gcc/config/rs6000/rs6000-builtins.def > +++ b/gcc/config/rs6000/rs6000-builtins.def > @@ -115,7 +115,6 @@ > ; > ; Attributes are strings, and the allowed ones are listed below. > ; > -; init Process as a vec_init function > ; set Process as a vec_set function > ; extract Process as a vec_extract function > ; nosoft Not valid with -msoft-float > @@ -1118,37 +1117,6 @@ > const signed short __builtin_vec_ext_v8hi (vss, signed int); > VEC_EXT_V8HI nothing {extract} > > - const vsc __builtin_vec_init_v16qi (signed char, signed char, signed char, > \ > - signed char, signed char, signed char, signed char, signed char, > \ > - signed char, signed char, signed char, signed char, signed char, > \ > - signed char, signed char, signed char); > - VEC_INIT_V16QI nothing {init} > - > - const vf __builtin_vec_init_v4sf (float, float, float, float); > - VEC_INIT_V4SF nothing {init} > - > - const vsi __builtin_vec_init_v4si (signed int, signed int, signed int, \ > - signed int); > - VEC_INIT_V4SI nothing {init} > - > - const vss __builtin_vec_init_v8hi (signed short, signed short, signed > short,\ > - signed short, signed short, signed short, signed short, \ > - signed short); > - VEC_INIT_V8HI nothing {init} > - > - const vsc __builtin_vec_set_v16qi (vsc, signed char, const int<4>); > - VEC_SET_V16QI nothing {set} > - > - const vf __builtin_vec_set_v4sf (vf, float, const int<2>); > - VEC_SET_V4SF nothing {set} > - > - const vsi __builtin_vec_set_v4si (vsi, signed int, const int<2>); > - VEC_SET_V4SI nothing {set} > - > - const vss __builtin_vec_set_v8hi (vss, signed short, const int<3>); > - VEC_SET_V8HI nothing {set} > - > - > ; Cell builtins. > [cell] > pure vsc __builtin_altivec_lvlx (signed long, const void *); > @@ -1295,15 +1263,10 @@ > const signed long long __builtin_vec_ext_v2di (vsll, signed int); > VEC_EXT_V2DI nothing {extract} > > - const vsq __builtin_vec_init_v1ti (signed __int128); > - VEC_INIT_V1TI nothing {init} > - > - const vd __builtin_vec_init_v2df (double, double); > - VEC_INIT_V2DF nothing {init} > - > - const vsll __builtin_vec_init_v2di (signed long long, signed long long); > - VEC_INIT_V2DI nothing {init} > - > +;; VEC_SET_V1TI, VEC_SET_V2DF and VEC_SET_V2DI are used in > +;; resolve_vec_insert(), rs6000-c.cc > +;; TODO: Remove VEC_SET_V1TI, VEC_SET_V2DF and VEC_SET_V2DI once the uses > +;; in resolve_vec_insert are replaced by the equivalent gimple statements. > const vsq __builtin_vec_set_v1ti (vsq, signed __int128, const int<0,0>); > VEC_SET_V1TI nothing {set} > > diff --git a/gcc/config/rs6000/rs6000-gen-builtins.cc > b/gcc/config/rs6000/rs6000-gen-builtins.cc > index 9af92cc5185..b706cca3e6d 100644 > --- a/gcc/config/rs6000/rs6000-gen-builtins.cc > +++ b/gcc/config/rs6000/rs6000-gen-builtins.cc > @@ -70,7 +70,6 @@ along with GCC; see the file COPYING3. If not see > > Attributes are strings, such as these: > > - init Process as a vec_init function > set Process as a vec_set function > extract Process as a vec_extract function > nosoft Not valid with -msoft-float > @@ -371,7 +370,6 @@ struct typelist > /* Attributes of a builtin function. */ > struct attrinfo > { > - bool isinit; > bool isset; > bool isextract; > bool isnosoft; > @@ -1396,9 +1394,7 @@ parse_bif_attrs (attrinfo *attrptr) > attrname = match_identifier (); > if (attrname) > { > - if (!strcmp (attrname, "init")) > - attrptr->isinit = 1; > - else if (!strcmp (attrname, "set")) > + if (!strcmp (attrname, "set")) > attrptr->isset = 1; > else if (!strcmp (attrname, "extract")) > attrptr->isextract = 1; > @@ -1473,12 +1469,12 @@ parse_bif_attrs (attrinfo *attrptr) > > #ifdef DEBUG > diag (0, > - "attribute set: init = %d, set = %d, extract = %d, nosoft = %d, " > + "attribute set: set = %d, extract = %d, nosoft = %d, " > "ldvec = %d, stvec = %d, reve = %d, pred = %d, htm = %d, " > "htmspr = %d, htmcr = %d, mma = %d, quad = %d, pair = %d, " > "mmaint = %d, no32bit = %d, 32bit = %d, cpu = %d, ldstmask = %d, " > "lxvrse = %d, lxvrze = %d, endian = %d, ibmdld = %d, ibm128 = %d.\n", > - attrptr->isinit, attrptr->isset, attrptr->isextract, > + attrptr->isset, attrptr->isextract, > attrptr->isnosoft, attrptr->isldvec, attrptr->isstvec, > attrptr->isreve, attrptr->ispred, attrptr->ishtm, attrptr->ishtmspr, > attrptr->ishtmcr, attrptr->ismma, attrptr->isquad, attrptr->ispair, Nit: This hunk can be re-formatted, like: "lxvrse = %d, lxvrze = %d, endian = %d, ibmdld = %d, ibm128 = %d.\n", - attrptr->isinit, attrptr->isset, attrptr->isextract, - attrptr->isnosoft, attrptr->isldvec, attrptr->isstvec, - attrptr->isreve, attrptr->ispred, attrptr->ishtm, attrptr->ishtmspr, - attrptr->ishtmcr, attrptr->ismma, attrptr->isquad, attrptr->ispair, - attrptr->ismmaint, attrptr->isno32bit, attrptr->is32bit, - attrptr->iscpu, attrptr->isldstmask, attrptr->islxvrse, - attrptr->islxvrze, attrptr->isendian, attrptr->isibmld, - attrptr->isibm128); + attrptr->isset, attrptr->isextract, attrptr->isnosoft, attrptr->isldvec, + attrptr->isstvec, attrptr->isreve, attrptr->ispred, attrptr->ishtm, + attrptr->ishtmspr, attrptr->ishtmcr, attrptr->ismma, attrptr->isquad, + attrptr->ispair, attrptr->ismmaint, attrptr->isno32bit, + attrptr->is32bit, attrptr->iscpu, attrptr->isldstmask, + attrptr->islxvrse, attrptr->islxvrze, attrptr->isendian, + attrptr->isibmld, attrptr->isibm128); #endif OK for trunk with all above nits tweaked, thanks! BR, Kewen > @@ -2276,7 +2272,7 @@ write_decls (void) > fprintf (header_file, " rs6000_gen_builtins assoc_bif;\n"); > fprintf (header_file, "};\n\n"); > > - fprintf (header_file, "#define bif_init_bit\t\t(0x00000001)\n"); > + /* Bit pattern 0x00000001 is available. */ > fprintf (header_file, "#define bif_set_bit\t\t(0x00000002)\n"); > fprintf (header_file, "#define bif_extract_bit\t\t(0x00000004)\n"); > fprintf (header_file, "#define bif_nosoft_bit\t\t(0x00000008)\n"); > @@ -2301,8 +2297,6 @@ write_decls (void) > fprintf (header_file, "#define bif_ibmld_bit\t\t(0x00400000)\n"); > fprintf (header_file, "#define bif_ibm128_bit\t\t(0x00800000)\n"); > fprintf (header_file, "\n"); > - fprintf (header_file, > - "#define bif_is_init(x)\t\t((x).bifattrs & bif_init_bit)\n"); > fprintf (header_file, > "#define bif_is_set(x)\t\t((x).bifattrs & bif_set_bit)\n"); > fprintf (header_file, > @@ -2504,8 +2498,6 @@ write_bif_static_init (void) > fprintf (init_file, " /* nargs */\t%d,\n", > bifp->proto.nargs); > fprintf (init_file, " /* bifattrs */\t0"); > - if (bifp->attrs.isinit) > - fprintf (init_file, " | bif_init_bit"); > if (bifp->attrs.isset) > fprintf (init_file, " | bif_set_bit"); > if (bifp->attrs.isextract)