Kewen:
On 7/22/24 2:09 AM, Kewen.Lin wrote:
Hi Carl,
on 2024/7/18 00:01, Carl Love wrote:
GCC maintainers:
This patch removes the __builtin_vec_set_v1ti, __builtin_vec_set_v2df and
__builtin_vec_set_v2di built-ins. The users should just use normal C-code to
update the various vector elements. This change was originally intended to be
part of the earlier series of cleanup patches. It was initially thought that
some additional work would be needed to do some gimple generation instead of
these built-ins. However, the existing default code generation does produce
the needed code. The code generated with normal C-code is as good or better
than the code generated with these built-ins.
I think we need to expand this a bit:
- For vec_set bif, the equivalent C code is as good as or better than it.
- For vec_insert bif whose resolving makes use of vec_set bif previously
(now get removed),
it's as good as before with optimization.
The patch has been tested on Power 10 LE with no regressions.
Please let me know if the patch is acceptable for mainline. Thanks.
Carl
---------------------------------------------------------------
rs6000, Remove __builtin_vec_set_v1ti, __builtin_vec_set_v2df,
__builtin_vec_set_v2di
Remove the built-ins, use the default gimple generation instead.
gcc/ChangeLog:
* config/rs6000/rs6000-builtins.def (__builtin_vec_set_v1ti,
__builtin_vec_set_v2df, __builtin_vec_set_v2di): Remove built-in
definitions.
* config/rs6000/rs6000-c.cc (resolve_vec_insert): Remove if
statemnts for mode == V2DFmode, mode == V2DImode and
Nit: s/statemnts/statements/
OK, fixed
Maybe a bit more meaningful like: Remove the handling for constant vec_insert
position
with VECTOR_UNIT_VSX_P V1TImode, V2DFmode and V2DImode modes.
OK, changed
mode == V1TImode that reference RS6000_BIF_VEC_SET_V2DF,
RS6000_BIF_VEC_SET_V2DI and RS6000_BIF_VEC_SET_V1TI.
---
gcc/config/rs6000/rs6000-builtins.def | 13 ---------
gcc/config/rs6000/rs6000-c.cc | 40 ---------------------------
2 files changed, 53 deletions(-)
diff --git a/gcc/config/rs6000/rs6000-builtins.def
b/gcc/config/rs6000/rs6000-builtins.def
index 896d9686ac6..0ebc940f395 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -1263,19 +1263,6 @@
const signed long long __builtin_vec_ext_v2di (vsll, signed int);
VEC_EXT_V2DI nothing {extract}
-;; 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}
-
- const vd __builtin_vec_set_v2df (vd, double, const int<1>);
- VEC_SET_V2DF nothing {set}
-
- const vsll __builtin_vec_set_v2di (vsll, signed long long, const int<1>);
- VEC_SET_V2DI nothing {set}
-
Unexpected empty line removed.
?? I don't remove the blank line before the removed comment, so there
is still a single blank line before the next entry. Specifically, the
code with the above removed now looks like:
...
const signed long long __builtin_vec_ext_v2di (vsll, signed int);
VEC_EXT_V2DI nothing {extract}
const vsc __builtin_vsx_cmpge_16qi (vsc, vsc);
CMPGE_16QI vector_nltv16qi {}
const vsll __builtin_vsx_cmpge_2di (vsll, vsll);
CMPGE_2DI vector_nltv2di {}
....
Which looks OK to me?
Similar to vec_init removal, we should also get rid of set bif attribute,
bif_is_set and altivec_expand_vec_set_builtin etc.
That will also require removing:
const vsq __builtin_vsx_set_1ti (vsq, signed __int128, const int<0,0>);
SET_1TI vsx_set_v1ti {set}
const vd __builtin_vsx_set_2df (vd, double, const int<0,1>);
SET_2DF vsx_set_v2df {set}
const vsll __builtin_vsx_set_2di (vsll, signed long long, const int<0,1>);
SET_2DI vsx_set_v2di {set}
I would assume the C-code generation for the above will be as good or
better than the code generation for the built-ins but will need to
verify that. I haven't looked at them specifically.
Carl
