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)

Reply via email to