Re: [PATCH 13/13 ver4] rs6000, remove vector set and vector init built-ins
Kewen: On 6/18/24 20:04, Kewen.Lin wrote: Hi Carl, on 2024/6/14 03:40, Carl Love wrote: GCC maintainers: The patch has been updated per the feedback from version 3. Please let me know it 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. gcc/ChangeLog: * config/rs6000/rs6000-builtins.def (__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. --- gcc/config/rs6000/rs6000-builtins.def | 44 +++ 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def index 02aa04e5698..053dc0115d2 100644 --- a/gcc/config/rs6000/rs6000-builtins.def +++ b/gcc/config/rs6000/rs6000-builtins.def @@ -1118,37 +1118,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} I just realized this {init} is customized for vec_init only, these removed vec_init bifs are the only users of it, so we should remove this attribute as well. Sorry that I should have found and pointed out this in the previous review. I think it means some removals are needed on: 1) comments in rs6000-builtins.def ; init Process as a vec_init function 2) related gen code for this attribute bit, like: fprintf (header_file, "#define bif_init_bit\t\t(0x0001)\n"); fprintf (header_file, "#define bif_is_init(x)\t\t((x).bifattrs & bif_init_bit)\n"); if (bifp->attrs.isinit) fprintf (init_file, " | bif_init_bit"); OK, Yes, we can remove the attribute string for the vec_init built-in. In addition to the code you mentioned, we will need to remove the uses of bif_init_bit, bif_is_init and the function altivec_expand_vec_init_builtin. Carl
Re: [PATCH 13/13 ver4] rs6000, remove vector set and vector init built-ins
Hi Carl, on 2024/6/14 03:40, Carl Love wrote: > GCC maintainers: > > The patch has been updated per the feedback from version 3. Please let me > know it 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. > > gcc/ChangeLog: > * config/rs6000/rs6000-builtins.def (__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. > --- > gcc/config/rs6000/rs6000-builtins.def | 44 +++ > 1 file changed, 4 insertions(+), 40 deletions(-) > > diff --git a/gcc/config/rs6000/rs6000-builtins.def > b/gcc/config/rs6000/rs6000-builtins.def > index 02aa04e5698..053dc0115d2 100644 > --- a/gcc/config/rs6000/rs6000-builtins.def > +++ b/gcc/config/rs6000/rs6000-builtins.def > @@ -1118,37 +1118,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} I just realized this {init} is customized for vec_init only, these removed vec_init bifs are the only users of it, so we should remove this attribute as well. Sorry that I should have found and pointed out this in the previous review. I think it means some removals are needed on: 1) comments in rs6000-builtins.def ; init Process as a vec_init function 2) related gen code for this attribute bit, like: fprintf (header_file, "#define bif_init_bit\t\t(0x0001)\n"); fprintf (header_file, "#define bif_is_init(x)\t\t((x).bifattrs & bif_init_bit)\n"); if (bifp->attrs.isinit) fprintf (init_file, " | bif_init_bit"); The others look good to me! BR, Kewen
Re: [PATCH 13/13 ver4] rs6000, remove vector set and vector init built-ins
GCC maintainers: The patch has been updated per the feedback from version 3. Please let me know it 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. gcc/ChangeLog: * config/rs6000/rs6000-builtins.def (__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. --- gcc/config/rs6000/rs6000-builtins.def | 44 +++ 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def index 02aa04e5698..053dc0115d2 100644 --- a/gcc/config/rs6000/rs6000-builtins.def +++ b/gcc/config/rs6000/rs6000-builtins.def @@ -1118,37 +1118,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 +1264,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} -- 2.45.0