* [PATCH 0/2] rs6000, fix vec_replace_unaligned built-in arguments
@ 2023-07-17 18:51 Carl Love
2023-07-17 19:19 ` [PATCH 1/2] rs6000, add argument to function find_instance Carl Love
2023-07-17 19:20 ` [PATCH 2/2 ver 4] rs6000, fix vec_replace_unaligned built-in arguments Carl Love
0 siblings, 2 replies; 7+ messages in thread
From: Carl Love @ 2023-07-17 18:51 UTC (permalink / raw)
To: Kewen.Lin, Segher Boessenkool, David Edelsohn, gcc-patches
Cc: cel, Peter Bergner
GCC maintianers:
In the process of fixing the powerpc/vec-replace-word-runnable.c test I
found there is an existing issue with function find_instance in rs6000-
c.cc. Per the review comments from Kewen in
https://gcc.gnu.org/pipermail/gcc-patches/2023-July/624401.html
The fix for function find_instance was put into a separate patch
followed by a patch for the vec-replace-word-runnable.c test fixes.
The two patches have been tested on Power 10 LE with no regression
failures.
Carl
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] rs6000, add argument to function find_instance
2023-07-17 18:51 [PATCH 0/2] rs6000, fix vec_replace_unaligned built-in arguments Carl Love
@ 2023-07-17 19:19 ` Carl Love
2023-07-21 2:19 ` Kewen.Lin
2023-07-17 19:20 ` [PATCH 2/2 ver 4] rs6000, fix vec_replace_unaligned built-in arguments Carl Love
1 sibling, 1 reply; 7+ messages in thread
From: Carl Love @ 2023-07-17 19:19 UTC (permalink / raw)
To: Kewen.Lin, Segher Boessenkool, David Edelsohn, gcc-patches
Cc: Peter Bergner, cel
GCC maintainers:
The rs6000 function find_instance assumes that it is called for built-
ins with only two arguments. There is no checking for the actual
number of aruguments used in the built-in. This patch adds an
additional parameter to the function call containing the number of
aruguments in the built-in. The function will now do the needed checks
for all of the arguments.
This fix is needed for the next patch in the series that fixes the
vec_replace_unaligned built-in.c test.
Please let me know if this patch is acceptable for mainline. Thanks.
Carl
--------------------------------------------
rs6000, add argument to function find_instance
The function find_instance assumes it is called to check a built-in with
only two arguments. Ths patch extends the function by adding a parameter
specifying the number of buit-in arguments to check.
gcc/ChangeLog:
* config/rs6000/rs6000-c.cc (find_instance): Add new parameter that
specifies the number of built-in arguments to check.
(altivec_resolve_overloaded_builtin): Update calls to find_instance
to pass the number of built-in argument to be checked.
---
gcc/config/rs6000/rs6000-c.cc | 27 +++++++++++++++++++--------
1 file changed, 19 insertions(+), 8 deletions(-)
diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
index a353bca19ef..350987b851b 100644
--- a/gcc/config/rs6000/rs6000-c.cc
+++ b/gcc/config/rs6000/rs6000-c.cc
@@ -1679,7 +1679,7 @@ tree
find_instance (bool *unsupported_builtin, ovlddata **instance,
rs6000_gen_builtins instance_code,
rs6000_gen_builtins fcode,
- tree *types, tree *args)
+ tree *types, tree *args, int nargs)
{
while (*instance && (*instance)->bifid != instance_code)
*instance = (*instance)->next;
@@ -1691,17 +1691,28 @@ find_instance (bool *unsupported_builtin, ovlddata **instance,
if (!inst->fntype)
return error_mark_node;
tree fntype = rs6000_builtin_info[inst->bifid].fntype;
- tree parmtype0 = TREE_VALUE (TYPE_ARG_TYPES (fntype));
- tree parmtype1 = TREE_VALUE (TREE_CHAIN (TYPE_ARG_TYPES (fntype)));
+ tree argtype = TYPE_ARG_TYPES (fntype);
+ tree parmtype;
+ int args_compatible = true;
- if (rs6000_builtin_type_compatible (types[0], parmtype0)
- && rs6000_builtin_type_compatible (types[1], parmtype1))
+ for (int i = 0; i <nargs; i++)
{
+ parmtype = TREE_VALUE (argtype);
+ if (! rs6000_builtin_type_compatible (types[i], parmtype))
+ {
+ args_compatible = false;
+ break;
+ }
+ argtype = TREE_CHAIN (argtype);
+ }
+
+ if (args_compatible)
+ {
if (rs6000_builtin_decl (inst->bifid, false) != error_mark_node
&& rs6000_builtin_is_supported (inst->bifid))
{
tree ret_type = TREE_TYPE (inst->fntype);
- return altivec_build_resolved_builtin (args, 2, fntype, ret_type,
+ return altivec_build_resolved_builtin (args, nargs, fntype, ret_type,
inst->bifid, fcode);
}
else
@@ -1921,7 +1932,7 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
instance_code = RS6000_BIF_CMPB_32;
tree call = find_instance (&unsupported_builtin, &instance,
- instance_code, fcode, types, args);
+ instance_code, fcode, types, args, nargs);
if (call != error_mark_node)
return call;
break;
@@ -1958,7 +1969,7 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
}
tree call = find_instance (&unsupported_builtin, &instance,
- instance_code, fcode, types, args);
+ instance_code, fcode, types, args, nargs);
if (call != error_mark_node)
return call;
break;
--
2.37.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2 ver 4] rs6000, fix vec_replace_unaligned built-in arguments
2023-07-17 18:51 [PATCH 0/2] rs6000, fix vec_replace_unaligned built-in arguments Carl Love
2023-07-17 19:19 ` [PATCH 1/2] rs6000, add argument to function find_instance Carl Love
@ 2023-07-17 19:20 ` Carl Love
2023-07-21 5:04 ` Kewen.Lin
1 sibling, 1 reply; 7+ messages in thread
From: Carl Love @ 2023-07-17 19:20 UTC (permalink / raw)
To: Kewen.Lin, Segher Boessenkool, David Edelsohn, gcc-patches
Cc: Peter Bergner, cel
GCC maintainers:
Version 4, changed the new RS6000_OVLD_VEC_REPLACE_UN case statement
rs6000/rs6000-c.cc. The existing REPLACE_ELT iterator name was changed
to REPLACE_ELT_V along with the associated define_mode_attr. Renamed
VEC_RU to REPLACE_ELT for the iterator name and VEC_RU_char to
REPLACE_ELT_char. Fixed the double test in vec-replace-word-
runnable_1.c to be consistent with the other tests. Removed the "dg-do
link" from both tests. Put in an explicit cast in test vec-replace-word-runnable_2.c to eliminate the need for the -flax-vector-conversions dg-option.
Version 3, added code to altivec_resolve_overloaded_builtin so the
correct instruction is selected for the size of the second argument.
This restores the instruction counts to the original values where the
correct instructions were originally being generated. The naming of
the overloaded builtin instances and builtin definitions were changed
to reflect the type of the second argument since the type of the first
argument is now the same for all overloaded instances. A new builtin
test file was added for the case where the first argument is cast to
the unsigned long long type. This test requires the -flax-vector-
conversions gcc command line option. Since the other tests do not
require this option, I felt that the new test needed to be in a
separate file. Finally some formatting fixes were made in the original
test file. Patch has been retested on Power 10 with no regressions.
Version 2, fixed various typos. Updated the change log body to say the
instruction counts were updated. The instruction counts changed as a
result of changing the first argument of the vec_replace_unaligned
builtin call from vector unsigned long long (vull) to vector unsigned
char (vuc). When the first argument was vull the builtin call
generated the vinsd instruction for the two test cases. The updated
call with vuc as the first argument generates two vinsw instructions
instead. Patch was retested on Power 10 with no regressions.
The following patch fixes the first argument in the builtin definition
and the corresponding test cases. Initially, the builtin specification
was wrong due to a cut and past error. The documentation was fixed in:
commit ed3fea09b18f67e757b5768b42cb6e816626f1db
Author: Bill Schmidt <wschmidt@linux.ibm.com>
Date: Fri Feb 4 13:07:17 2022 -0600
rs6000: Correct function prototypes for vec_replace_unaligned
Due to a pasto error in the documentation, vec_replace_unaligned was
implemented with the same function prototypes as vec_replace_elt.
It was intended that vec_replace_unaligned always specify output
vectors as having type vector unsigned char, to emphasize that
elements are potentially misaligned by this built-in function.
This patch corrects the misimplementation.
....
This patch fixes the arguments in the definitions and updates the
testcases accordingly. Additionally, a few minor spacing issues are
fixed.
The patch has been tested on Power 10 with no regressions. Please let
me know if the patch is acceptable for mainline. Thanks.
Carl
----------------------------
rs6000, fix vec_replace_unaligned built-in arguments
The first argument of the vec_replace_unaligned built-in should always be
of type unsigned char, as specified in gcc/doc/extend.texi.
This patch fixes the builtin definitions and updates the test cases to use
the correct arguments. The original test file is renamed and a second test
file is added for a new test case.
gcc/ChangeLog:
* config/rs6000/rs6000-builtins.def: Rename
__builtin_altivec_vreplace_un_uv2di as __builtin_altivec_vreplace_un_udi
__builtin_altivec_vreplace_un_uv4si as __builtin_altivec_vreplace_un_usi
__builtin_altivec_vreplace_un_v2df as __builtin_altivec_vreplace_un_df
__builtin_altivec_vreplace_un_v2di as __builtin_altivec_vreplace_un_di
__builtin_altivec_vreplace_un_v4sf as __builtin_altivec_vreplace_un_sf
__builtin_altivec_vreplace_un_v4si as __builtin_altivec_vreplace_un_si.
Rename VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_UV4SI as
VREPLACE_UN_USI, VREPLACE_UN_V2DF as VREPLACE_UN_DF,
VREPLACE_UN_V2DI as VREPLACE_UN_DI, VREPLACE_UN_V4SF as
VREPLACE_UN_SF, VREPLACE_UN_V4SI as VREPLACE_UN_SI.
Rename vreplace_un_v2di as vreplace_un_di, vreplace_un_v4si as
vreplace_un_si, vreplace_un_v2df as vreplace_un_df,
vreplace_un_v2di as vreplace_un_di, vreplace_un_v4sf as
vreplace_un_sf, vreplace_un_v4si as vreplace_un_si.
* config/rs6000/rs6000-c.cc (find_instance): Add case
RS6000_OVLD_VEC_REPLACE_UN.
* config/rs6000/rs6000-overload.def (__builtin_vec_replace_un):
Fix first argument type. Rename VREPLACE_UN_UV4SI as
VREPLACE_UN_USI, VREPLACE_UN_V4SI as VREPLACE_UN_SI,
VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_V2DI as
VREPLACE_UN_DI, VREPLACE_UN_V4SF as VREPLACE_UN_SF,
VREPLACE_UN_V2DF as VREPLACE_UN_DF.
* config/rs6000/vsx.md (REPLACE_ELT): Renamed the mode_iterator
REPLACE_ELT_V.
(REPLACE_ELT_char): Renamed REPLACE_ELT_V_char.
(REPLACE_ELT_sh): Renamed REPLACE_ELT_V_sh.
(REPLACE_ELT_max): Renamed REPLACE_ELT_V_max.
(REPLACE_ELT): New mode iterator.
(REPLACE_ELT_char): New mode attribute.
(vreplace_un_<mode>): Change iterator and mode attribute.
gcc/testsuite/ChangeLog:
* gcc.target/powerpc/vec-replace-word-runnable.c: Renamed
vec-replace-word-runnable_1.c.
* gcc.target/powerpc/vec-replace-word-runnable_1.c
(dg-options): add -flax-vector-conversions.
(vec_replace_unaligned) Fix first argument type.
(vresult_uchar): Fix expected results.
(vec_replace_unaligned): Update for loop to check uchar results.
Remove extra spaces in if statements. Insert missing spaces in
for statements.
* gcc.target/powerpc/vec-replace-word-runnable_2.c: New test file.
---
gcc/config/rs6000/rs6000-builtins.def | 28 +--
gcc/config/rs6000/rs6000-c.cc | 23 +++
gcc/config/rs6000/rs6000-overload.def | 24 +--
gcc/config/rs6000/vsx.md | 52 ++---
...nnable.c => vec-replace-word-runnable_1.c} | 177 ++++++++++--------
.../powerpc/vec-replace-word-runnable_2.c | 50 +++++
6 files changed, 224 insertions(+), 130 deletions(-)
rename gcc/testsuite/gcc.target/powerpc/{vec-replace-word-runnable.c => vec-replace-word-runnable_1.c} (51%)
create mode 100644 gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_2.c
diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
index c0ef717161f..58c53cd462e 100644
--- a/gcc/config/rs6000/rs6000-builtins.def
+++ b/gcc/config/rs6000/rs6000-builtins.def
@@ -3397,26 +3397,26 @@
const vull __builtin_altivec_vpextd (vull, vull);
VPEXTD vpextd {}
- const vuc __builtin_altivec_vreplace_un_uv2di (vull, unsigned long long, \
- const int<4>);
- VREPLACE_UN_UV2DI vreplace_un_v2di {}
+ const vuc __builtin_altivec_vreplace_un_udi (vuc, unsigned long long, \
+ const int<4>);
+ VREPLACE_UN_UDI vreplace_un_di {}
- const vuc __builtin_altivec_vreplace_un_uv4si (vui, unsigned int, \
- const int<4>);
- VREPLACE_UN_UV4SI vreplace_un_v4si {}
+ const vuc __builtin_altivec_vreplace_un_usi (vuc, unsigned int, \
+ const int<4>);
+ VREPLACE_UN_USI vreplace_un_si {}
- const vuc __builtin_altivec_vreplace_un_v2df (vd, double, const int<4>);
- VREPLACE_UN_V2DF vreplace_un_v2df {}
+ const vuc __builtin_altivec_vreplace_un_df (vuc, double, const int<4>);
+ VREPLACE_UN_DF vreplace_un_df {}
- const vuc __builtin_altivec_vreplace_un_v2di (vsll, signed long long, \
+ const vuc __builtin_altivec_vreplace_un_di (vuc, signed long long, \
const int<4>);
- VREPLACE_UN_V2DI vreplace_un_v2di {}
+ VREPLACE_UN_DI vreplace_un_di {}
- const vuc __builtin_altivec_vreplace_un_v4sf (vf, float, const int<4>);
- VREPLACE_UN_V4SF vreplace_un_v4sf {}
+ const vuc __builtin_altivec_vreplace_un_sf (vuc, float, const int<4>);
+ VREPLACE_UN_SF vreplace_un_sf {}
- const vuc __builtin_altivec_vreplace_un_v4si (vsi, signed int, const int<4>);
- VREPLACE_UN_V4SI vreplace_un_v4si {}
+ const vuc __builtin_altivec_vreplace_un_si (vuc, signed int, const int<4>);
+ VREPLACE_UN_SI vreplace_un_si {}
const vull __builtin_altivec_vreplace_uv2di (vull, unsigned long long, \
const int<1>);
diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
index 350987b851b..63ab360d352 100644
--- a/gcc/config/rs6000/rs6000-c.cc
+++ b/gcc/config/rs6000/rs6000-c.cc
@@ -1968,6 +1968,29 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
instance_code = RS6000_BIF_VSIEDP;
}
+ tree call = find_instance (&unsupported_builtin, &instance,
+ instance_code, fcode, types, args, nargs);
+ if (call != error_mark_node)
+ return call;
+ break;
+ }
+ case RS6000_OVLD_VEC_REPLACE_UN:
+ {
+ machine_mode arg2_mode = TYPE_MODE (types[1]);
+
+ if (arg2_mode == SImode)
+ /* Signed and unsigned are handled the same. */
+ instance_code = RS6000_BIF_VREPLACE_UN_USI;
+ else if (arg2_mode == SFmode)
+ instance_code = RS6000_BIF_VREPLACE_UN_SF;
+ else if (arg2_mode == DImode)
+ /* Signed and unsigned are handled the same. */
+ instance_code = RS6000_BIF_VREPLACE_UN_UDI;
+ else if (arg2_mode == DFmode)
+ instance_code = RS6000_BIF_VREPLACE_UN_DF;
+ else
+ break;
+
tree call = find_instance (&unsupported_builtin, &instance,
instance_code, fcode, types, args, nargs);
if (call != error_mark_node)
diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
index 470d718efde..b83946f5ad8 100644
--- a/gcc/config/rs6000/rs6000-overload.def
+++ b/gcc/config/rs6000/rs6000-overload.def
@@ -3059,18 +3059,18 @@
VREPLACE_ELT_V2DF
[VEC_REPLACE_UN, vec_replace_unaligned, __builtin_vec_replace_un]
- vuc __builtin_vec_replace_un (vui, unsigned int, const int);
- VREPLACE_UN_UV4SI
- vuc __builtin_vec_replace_un (vsi, signed int, const int);
- VREPLACE_UN_V4SI
- vuc __builtin_vec_replace_un (vull, unsigned long long, const int);
- VREPLACE_UN_UV2DI
- vuc __builtin_vec_replace_un (vsll, signed long long, const int);
- VREPLACE_UN_V2DI
- vuc __builtin_vec_replace_un (vf, float, const int);
- VREPLACE_UN_V4SF
- vuc __builtin_vec_replace_un (vd, double, const int);
- VREPLACE_UN_V2DF
+ vuc __builtin_vec_replace_un (vuc, unsigned int, const int);
+ VREPLACE_UN_USI
+ vuc __builtin_vec_replace_un (vuc, signed int, const int);
+ VREPLACE_UN_SI
+ vuc __builtin_vec_replace_un (vuc, unsigned long long, const int);
+ VREPLACE_UN_UDI
+ vuc __builtin_vec_replace_un (vuc, signed long long, const int);
+ VREPLACE_UN_DI
+ vuc __builtin_vec_replace_un (vuc, float, const int);
+ VREPLACE_UN_SF
+ vuc __builtin_vec_replace_un (vuc, double, const int);
+ VREPLACE_UN_DF
[VEC_REVB, vec_revb, __builtin_vec_revb]
vss __builtin_vec_revb (vss);
diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
index 0c269e4e8d9..d76f9f4864e 100644
--- a/gcc/config/rs6000/vsx.md
+++ b/gcc/config/rs6000/vsx.md
@@ -381,13 +381,20 @@ (define_int_attr xvcvbf16 [(UNSPEC_VSX_XVCVSPBF16 "xvcvspbf16")
(define_mode_iterator VI2 [V4SI V8HI V16QI V2DI])
;; Vector extract_elt iterator/attr for 32-bit and 64-bit elements
-(define_mode_iterator REPLACE_ELT [V4SI V4SF V2DI V2DF])
-(define_mode_attr REPLACE_ELT_char [(V4SI "w") (V4SF "w")
- (V2DI "d") (V2DF "d")])
-(define_mode_attr REPLACE_ELT_sh [(V4SI "2") (V4SF "2")
- (V2DI "3") (V2DF "3")])
-(define_mode_attr REPLACE_ELT_max [(V4SI "12") (V4SF "12")
- (V2DI "8") (V2DF "8")])
+(define_mode_iterator REPLACE_ELT_V [V4SI V4SF V2DI V2DF])
+(define_mode_attr REPLACE_ELT_V_char [(V4SI "w") (V4SF "w")
+ (V2DI "d") (V2DF "d")])
+(define_mode_attr REPLACE_ELT_V_sh [(V4SI "2") (V4SF "2")
+ (V2DI "3") (V2DF "3")])
+(define_mode_attr REPLACE_ELT_V_max [(V4SI "12") (V4SF "12")
+ (V2DI "8") (V2DF "8")])
+
+;; vector replace unaligned modes
+(define_mode_iterator REPLACE_ELT [SI SF DI DF])
+(define_mode_attr REPLACE_ELT_char [(SI "w")
+ (SF "w")
+ (DI "d")
+ (DF "d")])
;; Like VM2 in altivec.md, just do char, short, int, long, float and double
(define_mode_iterator VM3 [V4SI
@@ -4183,21 +4190,22 @@ (define_insn "vinsertgr_internal_<mode>"
[(set_attr "type" "vecsimple")])
(define_expand "vreplace_elt_<mode>"
- [(set (match_operand:REPLACE_ELT 0 "register_operand")
- (unspec:REPLACE_ELT [(match_operand:REPLACE_ELT 1 "register_operand")
- (match_operand:<VEC_base> 2 "register_operand")
- (match_operand:QI 3 "const_0_to_3_operand")]
- UNSPEC_REPLACE_ELT))]
+ [(set (match_operand:REPLACE_ELT_V 0 "register_operand")
+ (unspec:REPLACE_ELT_V [(match_operand:REPLACE_ELT_V 1 "register_operand")
+ (match_operand:<VEC_base> 2 "register_operand")
+ (match_operand:QI 3 "const_0_to_3_operand")]
+ UNSPEC_REPLACE_ELT))]
"TARGET_POWER10"
{
int index;
/* Immediate value is the word index, convert to byte index and adjust for
Endianness if needed. */
if (BYTES_BIG_ENDIAN)
- index = INTVAL (operands[3]) << <REPLACE_ELT_sh>;
+ index = INTVAL (operands[3]) << <REPLACE_ELT_V_sh>;
else
- index = <REPLACE_ELT_max> - (INTVAL (operands[3]) << <REPLACE_ELT_sh>);
+ index = <REPLACE_ELT_V_max>
+ - (INTVAL (operands[3]) << <REPLACE_ELT_V_sh>);
emit_insn (gen_vreplace_elt_<mode>_inst (operands[0], operands[1],
operands[2],
@@ -4207,19 +4215,19 @@ (define_expand "vreplace_elt_<mode>"
[(set_attr "type" "vecsimple")])
(define_insn "vreplace_elt_<mode>_inst"
- [(set (match_operand:REPLACE_ELT 0 "register_operand" "=v")
- (unspec:REPLACE_ELT [(match_operand:REPLACE_ELT 1 "register_operand" "0")
- (match_operand:<VEC_base> 2 "register_operand" "r")
- (match_operand:QI 3 "const_0_to_12_operand" "n")]
- UNSPEC_REPLACE_ELT))]
+ [(set (match_operand:REPLACE_ELT_V 0 "register_operand" "=v")
+ (unspec:REPLACE_ELT_V [(match_operand:REPLACE_ELT_V 1 "register_operand" "0")
+ (match_operand:<VEC_base> 2 "register_operand" "r")
+ (match_operand:QI 3 "const_0_to_12_operand" "n")]
+ UNSPEC_REPLACE_ELT))]
"TARGET_POWER10"
- "vins<REPLACE_ELT_char> %0,%2,%3"
+ "vins<REPLACE_ELT_V_char> %0,%2,%3"
[(set_attr "type" "vecsimple")])
(define_insn "vreplace_un_<mode>"
[(set (match_operand:V16QI 0 "register_operand" "=v")
- (unspec:V16QI [(match_operand:REPLACE_ELT 1 "register_operand" "0")
- (match_operand:<VEC_base> 2 "register_operand" "r")
+ (unspec:V16QI [(match_operand:V16QI 1 "register_operand" "0")
+ (match_operand:REPLACE_ELT 2 "register_operand" "r")
(match_operand:QI 3 "const_0_to_12_operand" "n")]
UNSPEC_REPLACE_UN))]
"TARGET_POWER10"
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable.c b/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_1.c
similarity index 51%
rename from gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable.c
rename to gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_1.c
index 27318822871..7e7485ca371 100644
--- a/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable.c
+++ b/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_1.c
@@ -1,5 +1,4 @@
/* { dg-do run { target { power10_hw } } } */
-/* { dg-do link { target { ! power10_hw } } } */
/* { dg-require-effective-target power10_ok } */
/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
@@ -20,6 +19,9 @@ main (int argc, char *argv [])
unsigned char ch;
unsigned int index;
+ vector unsigned char src_va_uchar;
+ vector unsigned char expected_vresult_uchar;
+
vector unsigned int vresult_uint;
vector unsigned int expected_vresult_uint;
vector unsigned int src_va_uint;
@@ -64,10 +66,10 @@ main (int argc, char *argv [])
vresult_uint = vec_replace_elt (src_va_uint, src_a_uint, 2);
- if (!vec_all_eq (vresult_uint, expected_vresult_uint)) {
+ if (!vec_all_eq (vresult_uint, expected_vresult_uint)) {
#if DEBUG
printf("ERROR, vec_replace_elt (src_vb_uint, src_va_uint, index)\n");
- for(i = 0; i < 4; i++)
+ for (i = 0; i < 4; i++)
printf(" vresult_uint[%d] = %d, expected_vresult_uint[%d] = %d\n",
i, vresult_uint[i], i, expected_vresult_uint[i]);
#else
@@ -82,10 +84,10 @@ main (int argc, char *argv [])
vresult_int = vec_replace_elt (src_va_int, src_a_int, 1);
- if (!vec_all_eq (vresult_int, expected_vresult_int)) {
+ if (!vec_all_eq (vresult_int, expected_vresult_int)) {
#if DEBUG
- printf("ERROR, vec_replace_elt (src_vb_int, src_va_int, index)\n");
- for(i = 0; i < 4; i++)
+ printf("ERROR, vec_replace_elt (src_vb_int, src_a_int, index)\n");
+ for (i = 0; i < 4; i++)
printf(" vresult_int[%d] = %d, expected_vresult_int[%d] = %d\n",
i, vresult_int[i], i, expected_vresult_int[i]);
#else
@@ -100,10 +102,10 @@ main (int argc, char *argv [])
vresult_float = vec_replace_elt (src_va_float, src_a_float, 1);
- if (!vec_all_eq (vresult_float, expected_vresult_float)) {
+ if (!vec_all_eq (vresult_float, expected_vresult_float)) {
#if DEBUG
- printf("ERROR, vec_replace_elt (src_vb_float, src_va_float, index)\n");
- for(i = 0; i < 4; i++)
+ printf("ERROR, vec_replace_elt (src_vb_float, src_a_float, index)\n");
+ for (i = 0; i < 4; i++)
printf(" vresult_float[%d] = %f, expected_vresult_float[%d] = %f\n",
i, vresult_float[i], i, expected_vresult_float[i]);
#else
@@ -121,8 +123,8 @@ main (int argc, char *argv [])
if (!vec_all_eq (vresult_ullint, expected_vresult_ullint)) {
#if DEBUG
- printf("ERROR, vec_replace_elt (src_vb_ullint, src_va_ullint, index)\n");
- for(i = 0; i < 2; i++)
+ printf("ERROR, vec_replace_elt (src_vb_ullint, src_a_ullint, index)\n");
+ for (i = 0; i < 2; i++)
printf(" vresult_ullint[%d] = %d, expected_vresult_ullint[%d] = %d\n",
i, vresult_ullint[i], i, expected_vresult_ullint[i]);
#else
@@ -137,10 +139,10 @@ main (int argc, char *argv [])
vresult_llint = vec_replace_elt (src_va_llint, src_a_llint, 1);
- if (!vec_all_eq (vresult_llint, expected_vresult_llint)) {
+ if (!vec_all_eq (vresult_llint, expected_vresult_llint)) {
#if DEBUG
- printf("ERROR, vec_replace_elt (src_vb_llint, src_va_llint, index)\n");
- for(i = 0; i < 2; i++)
+ printf("ERROR, vec_replace_elt (src_vb_llint, src_a_llint, index)\n");
+ for (i = 0; i < 2; i++)
printf(" vresult_llint[%d] = %d, expected_vresult_llint[%d] = %d\n",
i, vresult_llint[i], i, expected_vresult_llint[i]);
#else
@@ -155,10 +157,10 @@ main (int argc, char *argv [])
vresult_double = vec_replace_elt (src_va_double, src_a_double, 1);
- if (!vec_all_eq (vresult_double, expected_vresult_double)) {
+ if (!vec_all_eq (vresult_double, expected_vresult_double)) {
#if DEBUG
- printf("ERROR, vec_replace_elt (src_vb_double, src_va_double, index)\n");
- for(i = 0; i < 2; i++)
+ printf("ERROR, vec_replace_elt (src_vb_double, src_a_double, index)\n");
+ for (i = 0; i < 2; i++)
printf(" vresult_double[%d] = %f, expected_vresult_double[%d] = %f\n",
i, vresult_double[i], i, expected_vresult_double[i]);
#else
@@ -169,60 +171,68 @@ main (int argc, char *argv [])
/* Vector replace 32-bit element, unaligned */
src_a_uint = 345;
- src_va_uint = (vector unsigned int) { 1, 2, 0, 0 };
- vresult_uint = (vector unsigned int) { 0, 0, 0, 0 };
+ src_va_uchar = (vector unsigned char) { 1, 0, 0, 0, 2, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0 };
+ vresult_uchar = (vector unsigned char) { 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0 };
/* Byte index 7 will overwrite part of elements 2 and 3 */
- expected_vresult_uint = (vector unsigned int) { 1, 2, 345*256, 0 };
+ expected_vresult_uchar
+ = (vector unsigned char) { 1, 0, 0, 0, 2, 0, 0, 0,
+ 0, 0x59, 0x1, 0, 0, 0, 0, 0 };
- vresult_uchar = vec_replace_unaligned (src_va_uint, src_a_uint, 3);
- vresult_uint = (vector unsigned int) vresult_uchar;
+ vresult_uchar = vec_replace_unaligned (src_va_uchar, src_a_uint, 3);
- if (!vec_all_eq (vresult_uint, expected_vresult_uint)) {
+ if (!vec_all_eq (vresult_uchar, expected_vresult_uchar)) {
#if DEBUG
- printf("ERROR, vec_replace_unaligned (src_vb_uint, src_va_uint, index)\n");
- for(i = 0; i < 4; i++)
- printf(" vresult_uint[%d] = %d, expected_vresult_uint[%d] = %d\n",
- i, vresult_uint[i], i, expected_vresult_uint[i]);
+ printf("ERROR, vec_replace_unaligned (src_va_uchar, src_a_uint, index)\n");
+ for (i = 0; i < 16; i++)
+ printf(" vresult_uchar[%d] = 0x%x, expected_vresult_uint[%d] = 0x%x\n",
+ i, vresult_uchar[i], i, expected_vresult_uchar[i]);
#else
abort();
#endif
}
src_a_int = 234;
- src_va_int = (vector int) { 1, 0, 3, 4 };
- vresult_int = (vector int) { 0, 0, 0, 0 };
+ src_va_uchar = (vector unsigned char) { 1, 0, 0, 0, 0, 0, 0, 0,
+ 3, 0, 0, 0, 4, 0, 0, 0 };
+ vresult_uchar = (vector unsigned char) { 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0 };
/* Byte index 7 will over write part of elements 1 and 2 */
- expected_vresult_int = (vector int) { 1, 234*256, 0, 4 };
-
- vresult_uchar = vec_replace_unaligned (src_va_int, src_a_int, 7);
- vresult_int = (vector signed int) vresult_uchar;
+ expected_vresult_uchar = (vector unsigned char) { 1, 0, 0, 0, 0, 0xea, 0, 0,
+ 0, 0, 0, 0, 4, 0, 0, 0 };
- if (!vec_all_eq (vresult_int, expected_vresult_int)) {
+ vresult_uchar = vec_replace_unaligned (src_va_uchar, src_a_int, 7);
+
+ if (!vec_all_eq (vresult_uchar, expected_vresult_uchar)) {
#if DEBUG
- printf("ERROR, vec_replace_unaligned (src_vb_int, src_va_int, index)\n");
- for(i = 0; i < 4; i++)
- printf(" vresult_int[%d] = %d, expected_vresult_int[%d] = %d\n",
- i, vresult_int[i], i, expected_vresult_int[i]);
+ printf("ERROR, vec_replace_unaligned (src_va_uchar, src_a_int, index)\n");
+ for (i = 0; i < 16; i++)
+ printf(" vresult_int[%d] = 0x%x, expected_vresult_int[%d] = 0x%x\n",
+ i, vresult_uchar[i], i, expected_vresult_uchar[i]);
#else
abort();
#endif
}
src_a_float = 34.0;
- src_va_float = (vector float) { 0.0, 10.0, 20.0, 30.0 };
- vresult_float = (vector float) { 0.0, 0.0, 0.0, 0.0 };
- expected_vresult_float = (vector float) { 0.0, 34.0, 20.0, 30.0 };
+ src_va_uchar = (vector unsigned char) { 0, 0, 0, 0, 0, 0, 0x41, 0xa0,
+ 5, 6, 7, 8, 0x41, 0xf0, 0, 0};
+ vresult_uchar = (vector unsigned char) { 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0 };
+ expected_vresult_uchar
+ = (vector unsigned char) { 0, 0, 0, 0, 0, 0, 8, 0x42,
+ 5, 6, 7, 8, 0x41, 0xf0, 0, 0 };
- vresult_uchar = vec_replace_unaligned (src_va_float, src_a_float, 8);
- vresult_float = (vector float) vresult_uchar;
+ vresult_uchar = vec_replace_unaligned (src_va_uchar, src_a_float, 8);
- if (!vec_all_eq (vresult_float, expected_vresult_float)) {
+ if (!vec_all_eq (vresult_uchar, expected_vresult_uchar)) {
#if DEBUG
- printf("ERROR, vec_replace_unaligned (src_vb_float, src_va_float, "
+ printf("ERROR, vec_replace_unaligned (src_va_uchar, src_a_float, "
"index)\n");
- for(i = 0; i < 4; i++)
- printf(" vresult_float[%d] = %f, expected_vresult_float[%d] = %f\n",
- i, vresult_float[i], i, expected_vresult_float[i]);
+ for (i = 0; i < 16; i++)
+ printf(" vresult_uchar[%d] = 0x%x, expected_vresult_uchar[%d] = 0x%x\n",
+ i, vresult_uchar[i], i, expected_vresult_uchar[i]);
#else
abort();
#endif
@@ -230,72 +240,75 @@ main (int argc, char *argv [])
/* Vector replace 64-bit element, unaligned */
src_a_ullint = 456;
- src_va_ullint = (vector unsigned long long int) { 0, 0x222 };
- vresult_ullint = (vector unsigned long long int) { 0, 0 };
- expected_vresult_ullint = (vector unsigned long long int) { 456*256,
- 0x200 };
+ src_va_uchar = (vector unsigned char) { 0, 0xc, 0x1, 0, 0, 0, 0, 0,
+ 0x22, 0x2, 0, 0, 0, 0, 0, 0 };
+ vresult_uchar = (vector unsigned char) { 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0 };
+ expected_vresult_uchar
+ = (vector unsigned char) { 0, 0xc8, 0x1, 0, 0, 0, 0, 0,
+ 0, 2, 0, 0, 0, 0, 0, 0 };
/* Byte index 7 will over write least significant byte of element 0 */
- vresult_uchar = vec_replace_unaligned (src_va_ullint, src_a_ullint, 7);
- vresult_ullint = (vector unsigned long long) vresult_uchar;
+ vresult_uchar = vec_replace_unaligned (src_va_uchar, src_a_ullint, 7);
- if (!vec_all_eq (vresult_ullint, expected_vresult_ullint)) {
+ if (!vec_all_eq (vresult_uchar, expected_vresult_uchar)) {
#if DEBUG
- printf("ERROR, vec_replace_unaligned (src_vb_ullint, src_va_ullint, "
+ printf("ERROR, vec_replace_unaligned (src_va_uchar, src_a_ullint, "
"index)\n");
- for(i = 0; i < 2; i++)
- printf(" vresult_ullint[%d] = %d, expected_vresult_ullint[%d] = %d\n",
- i, vresult_ullint[i], i, expected_vresult_ullint[i]);
+ for (i = 0; i < 16; i++)
+ printf(" vresult_uchar[%d] = 0x%x, expected_vresult_uchar[%d] = 0x%x\n",
+ i, vresult_uchar[i], i, expected_vresult_uchar[i]);
#else
abort();
#endif
}
src_a_llint = 678;
- src_va_llint = (vector long long int) { 0, 0x101 };
+ src_va_uchar = (vector unsigned char) { 0, 0xa6, 0x2, 0, 0, 0, 0, 0,
+ 0x0, 0x1, 0, 0, 0, 0, 0, 0 };
vresult_llint = (vector long long int) { 0, 0 };
/* Byte index 7 will over write least significant byte of element 0 */
- expected_vresult_llint = (vector long long int) { 678*256, 0x100 };
+ expected_vresult_uchar
+ = (vector unsigned char) { 0x0, 0xa6, 0x2, 0x0, 0x0, 0x0, 0x0, 0x0,
+ 0x0, 0x1, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 };
- vresult_uchar = vec_replace_unaligned (src_va_llint, src_a_llint, 7);
- vresult_llint = (vector signed long long) vresult_uchar;
+ vresult_uchar = vec_replace_unaligned (src_va_uchar, src_a_llint, 7);
- if (!vec_all_eq (vresult_llint, expected_vresult_llint)) {
+ if (!vec_all_eq (vresult_uchar, expected_vresult_uchar)) {
#if DEBUG
- printf("ERROR, vec_replace_unaligned (src_vb_llint, src_va_llint, "
+ printf("ERROR, vec_replace_unaligned (src_va_uchar, src_a_llint, "
"index)\n");
- for(i = 0; i < 2; i++)
- printf(" vresult_llint[%d] = %d, expected_vresult_llint[%d] = %d\n",
- i, vresult_llint[i], i, expected_vresult_llint[i]);
+ for (i = 0; i < 16; i++)
+ printf(" vresult_uchar[%d] = 0x%x, expected_vresult_uchar[%d] = 0x%x\n",
+ i, vresult_uchar[i], i, expected_vresult_uchar[i]);
#else
abort();
#endif
}
src_a_double = 678.0;
- src_va_double = (vector double) { 0.0, 50.0 };
- vresult_double = (vector double) { 0.0, 0.0 };
- expected_vresult_double = (vector double) { 0.0, 678.0 };
+ src_va_uchar = (vector unsigned char) { 0, 0, 0, 0, 0, 0, 0, 0,
+ 0, 0, 0, 0, 0, 0, 0, 0 };
+ expected_vresult_uchar
+ = (vector unsigned char) { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+ 0x0, 0x0, 0x0, 0x0, 0x0, 0x30, 0x85, 0x40 };
- vresult_uchar = vec_replace_unaligned (src_va_double, src_a_double, 0);
- vresult_double = (vector double) vresult_uchar;
+ vresult_uchar = vec_replace_unaligned (src_va_uchar, src_a_double, 0);
- if (!vec_all_eq (vresult_double, expected_vresult_double)) {
+ if (!vec_all_eq (vresult_uchar, expected_vresult_uchar)) {
#if DEBUG
- printf("ERROR, vec_replace_unaligned (src_vb_double, src_va_double, "
- "index)\n");
- for(i = 0; i < 2; i++)
- printf(" vresult_double[%d] = %f, expected_vresult_double[%d] = %f\n",
- i, vresult_double[i], i, expected_vresult_double[i]);
+ printf("ERROR, vec_replace_unaligned (src_va_uchar, src_a_double, "
+ "0)\n");
+ for (i = 0; i < 16; i++)
+ printf(" vresult_uchar[%d] = 0x%x, expected_vresult_uchar[%d] = 0x%x\n",
+ i, vresult_uchar[i], i, expected_vresult_uchar[i]);
#else
abort();
#endif
}
-
+
return 0;
}
/* { dg-final { scan-assembler-times {\mvinsw\M} 6 } } */
/* { dg-final { scan-assembler-times {\mvinsd\M} 6 } } */
-
-
diff --git a/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_2.c b/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_2.c
new file mode 100644
index 00000000000..bf0952c029f
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_2.c
@@ -0,0 +1,50 @@
+/* { dg-do run { target { power10_hw } } } */
+/* { dg-require-effective-target power10_ok } */
+/* { dg-options "-mdejagnu-cpu=power10 -save-temps" } */
+
+#include <altivec.h>
+
+#define DEBUG 0
+
+#if DEBUG
+#include <stdio.h>
+#endif
+
+extern void abort (void);
+
+int
+main (int argc, char *argv [])
+{
+ int i;
+ vector unsigned long long int vresult_ullint;
+ vector unsigned long long int expected_vresult_ullint;
+ vector unsigned long long int src_va_ullint;
+ unsigned int long long src_a_ullint;
+
+ /* Replace doubleword size chunk specified as a constant that can be
+ represented by an int. Should generate a vinsd instruction. Note, this
+ test requires command line option -flax-vector-conversions. */
+ src_a_ullint = 456;
+ src_va_ullint = (vector unsigned long long int) { 0, 11 };
+ vresult_ullint = (vector unsigned long long int) { 0, 2 };
+ expected_vresult_ullint = (vector unsigned long long int) { 0, 456 };
+
+ vresult_ullint = (vector unsigned long long int)
+ vec_replace_unaligned ((vector unsigned char)src_va_ullint,
+ src_a_ullint, 0);
+
+ if (!vec_all_eq (vresult_ullint, expected_vresult_ullint)) {
+#if DEBUG
+ printf("ERROR, vec_replace_unaligned ((vector unsigned char)src_vb_ullint, src_a_ullint, index)\n");
+ for (i = 0; i < 2; i++)
+ printf(" vresult_ullint[%d] = %d, expected_vresult_ullint[%d] = %d\n",
+ i, vresult_ullint[i], i, expected_vresult_ullint[i]);
+#else
+ abort();
+#endif
+ }
+
+ return 0;
+}
+
+/* { dg-final { scan-assembler-times {\mvinsd\M} 1 } } */
--
2.37.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] rs6000, add argument to function find_instance
2023-07-17 19:19 ` [PATCH 1/2] rs6000, add argument to function find_instance Carl Love
@ 2023-07-21 2:19 ` Kewen.Lin
2023-07-21 23:23 ` Carl Love
0 siblings, 1 reply; 7+ messages in thread
From: Kewen.Lin @ 2023-07-21 2:19 UTC (permalink / raw)
To: Carl Love; +Cc: Peter Bergner, Segher Boessenkool, David Edelsohn, gcc-patches
Hi Carl,
on 2023/7/18 03:19, Carl Love wrote:
>
> GCC maintainers:
>
> The rs6000 function find_instance assumes that it is called for built-
> ins with only two arguments. There is no checking for the actual
> number of aruguments used in the built-in. This patch adds an
> additional parameter to the function call containing the number of
> aruguments in the built-in. The function will now do the needed checks
> for all of the arguments.
>
> This fix is needed for the next patch in the series that fixes the
> vec_replace_unaligned built-in.c test.
>
> Please let me know if this patch is acceptable for mainline. Thanks.
>
> Carl
>
>
> --------------------------------------------
> rs6000, add argument to function find_instance
>
> The function find_instance assumes it is called to check a built-in with ~~ two spaces.
> only two arguments. Ths patch extends the function by adding a parameter
s/Ths/This/
> specifying the number of buit-in arguments to check.
s/bult-in/built-in/
>
> gcc/ChangeLog:
> * config/rs6000/rs6000-c.cc (find_instance): Add new parameter that
> specifies the number of built-in arguments to check.
> (altivec_resolve_overloaded_builtin): Update calls to find_instance
> to pass the number of built-in argument to be checked.
s/argument/arguments/
> ---
> gcc/config/rs6000/rs6000-c.cc | 27 +++++++++++++++++++--------
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
> index a353bca19ef..350987b851b 100644
> --- a/gcc/config/rs6000/rs6000-c.cc
> +++ b/gcc/config/rs6000/rs6000-c.cc
> @@ -1679,7 +1679,7 @@ tree
There is one function comment here describing the meaning of each parameter,
I think we should add a corresponding for NARGS, may be something like:
"; and NARGS specifies the number of built-in arguments."
Also we need to update the below "two"s with "NARGS".
"TYPES contains an array of two types..." and "ARGS contains an array of two arguments..."
since we already extend this to handle NARGS instead of two.
> find_instance (bool *unsupported_builtin, ovlddata **instance,
> rs6000_gen_builtins instance_code,
> rs6000_gen_builtins fcode,
> - tree *types, tree *args)
> + tree *types, tree *args, int nargs)
> {
> while (*instance && (*instance)->bifid != instance_code)
> *instance = (*instance)->next;
> @@ -1691,17 +1691,28 @@ find_instance (bool *unsupported_builtin, ovlddata **instance,
> if (!inst->fntype)
> return error_mark_node;
> tree fntype = rs6000_builtin_info[inst->bifid].fntype;
> - tree parmtype0 = TREE_VALUE (TYPE_ARG_TYPES (fntype));
> - tree parmtype1 = TREE_VALUE (TREE_CHAIN (TYPE_ARG_TYPES (fntype)));
> + tree argtype = TYPE_ARG_TYPES (fntype);
> + tree parmtype;
Nit: We can move "tree parmtype" into the loop (close to its only use).
> + int args_compatible = true;
s/int/bool/
>
> - if (rs6000_builtin_type_compatible (types[0], parmtype0)
> - && rs6000_builtin_type_compatible (types[1], parmtype1))
> + for (int i = 0; i <nargs; i++)
Nit: formatting issue, space before nargs.
> {
> + parmtype = TREE_VALUE (argtype);
tree parmtype = TREE_VALUE (argtype);
> + if (! rs6000_builtin_type_compatible (types[i], parmtype))
Nit: One unexpected(?) space after "!".
> + {
> + args_compatible = false;
> + break;
> + }
> + argtype = TREE_CHAIN (argtype);
> + }
> +
> + if (args_compatible)
> + {
Nit: indent issue for "{".
Ok for trunk with these nits fixed. Btw, the description doesn't say
how this was tested, I'm not sure if it's only tested together with
"patch 2/2", but please ensure it's bootstrapped and regress-tested
on BE and LE when committing. Thanks!
BR,
Kewen
> if (rs6000_builtin_decl (inst->bifid, false) != error_mark_node
> && rs6000_builtin_is_supported (inst->bifid))
> {
> tree ret_type = TREE_TYPE (inst->fntype);
> - return altivec_build_resolved_builtin (args, 2, fntype, ret_type,
> + return altivec_build_resolved_builtin (args, nargs, fntype, ret_type,
> inst->bifid, fcode);
> }
> else
> @@ -1921,7 +1932,7 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
> instance_code = RS6000_BIF_CMPB_32;
>
> tree call = find_instance (&unsupported_builtin, &instance,
> - instance_code, fcode, types, args);
> + instance_code, fcode, types, args, nargs);
> if (call != error_mark_node)
> return call;
> break;
> @@ -1958,7 +1969,7 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
> }
>
> tree call = find_instance (&unsupported_builtin, &instance,
> - instance_code, fcode, types, args);
> + instance_code, fcode, types, args, nargs);
> if (call != error_mark_node)
> return call;
> break;
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 ver 4] rs6000, fix vec_replace_unaligned built-in arguments
2023-07-17 19:20 ` [PATCH 2/2 ver 4] rs6000, fix vec_replace_unaligned built-in arguments Carl Love
@ 2023-07-21 5:04 ` Kewen.Lin
2023-07-21 23:23 ` Carl Love
0 siblings, 1 reply; 7+ messages in thread
From: Kewen.Lin @ 2023-07-21 5:04 UTC (permalink / raw)
To: Carl Love; +Cc: Peter Bergner, Segher Boessenkool, David Edelsohn, gcc-patches
Hi Carl,
on 2023/7/18 03:20, Carl Love wrote:
> GCC maintainers:
>
> Version 4, changed the new RS6000_OVLD_VEC_REPLACE_UN case statement
> rs6000/rs6000-c.cc. The existing REPLACE_ELT iterator name was changed
> to REPLACE_ELT_V along with the associated define_mode_attr. Renamed
> VEC_RU to REPLACE_ELT for the iterator name and VEC_RU_char to
> REPLACE_ELT_char. Fixed the double test in vec-replace-word-
> runnable_1.c to be consistent with the other tests. Removed the "dg-do
> link" from both tests. Put in an explicit cast in test vec-replace-word-runnable_2.c to eliminate the need for the -flax-vector-conversions dg-option.
>
> Version 3, added code to altivec_resolve_overloaded_builtin so the
> correct instruction is selected for the size of the second argument.
> This restores the instruction counts to the original values where the
> correct instructions were originally being generated. The naming of
> the overloaded builtin instances and builtin definitions were changed
> to reflect the type of the second argument since the type of the first
> argument is now the same for all overloaded instances. A new builtin
> test file was added for the case where the first argument is cast to
> the unsigned long long type. This test requires the -flax-vector-
> conversions gcc command line option. Since the other tests do not
> require this option, I felt that the new test needed to be in a
> separate file. Finally some formatting fixes were made in the original
> test file. Patch has been retested on Power 10 with no regressions.
>
> Version 2, fixed various typos. Updated the change log body to say the
> instruction counts were updated. The instruction counts changed as a
> result of changing the first argument of the vec_replace_unaligned
> builtin call from vector unsigned long long (vull) to vector unsigned
> char (vuc). When the first argument was vull the builtin call
> generated the vinsd instruction for the two test cases. The updated
> call with vuc as the first argument generates two vinsw instructions
> instead. Patch was retested on Power 10 with no regressions.
>
> The following patch fixes the first argument in the builtin definition
> and the corresponding test cases. Initially, the builtin specification
> was wrong due to a cut and past error. The documentation was fixed in:
>
> commit ed3fea09b18f67e757b5768b42cb6e816626f1db
> Author: Bill Schmidt <wschmidt@linux.ibm.com>
> Date: Fri Feb 4 13:07:17 2022 -0600
>
> rs6000: Correct function prototypes for vec_replace_unaligned
>
> Due to a pasto error in the documentation, vec_replace_unaligned was
> implemented with the same function prototypes as vec_replace_elt.
> It was intended that vec_replace_unaligned always specify output
> vectors as having type vector unsigned char, to emphasize that
> elements are potentially misaligned by this built-in function.
> This patch corrects the misimplementation.
> ....
>
> This patch fixes the arguments in the definitions and updates the
> testcases accordingly. Additionally, a few minor spacing issues are
> fixed.
>
> The patch has been tested on Power 10 with no regressions. Please let
> me know if the patch is acceptable for mainline. Thanks.
>
> Carl
>
>
> ----------------------------
> rs6000, fix vec_replace_unaligned built-in arguments
>
> The first argument of the vec_replace_unaligned built-in should always be
> of type unsigned char, as specified in gcc/doc/extend.texi.
Shouldn't be "vector unsigned char" instead of "unsigned char"?
Or do I miss something?
>
> This patch fixes the builtin definitions and updates the test cases to use
> the correct arguments. The original test file is renamed and a second test
> file is added for a new test case.
>
> gcc/ChangeLog:
> * config/rs6000/rs6000-builtins.def: Rename
> __builtin_altivec_vreplace_un_uv2di as __builtin_altivec_vreplace_un_udi
> __builtin_altivec_vreplace_un_uv4si as __builtin_altivec_vreplace_un_usi
> __builtin_altivec_vreplace_un_v2df as __builtin_altivec_vreplace_un_df
> __builtin_altivec_vreplace_un_v2di as __builtin_altivec_vreplace_un_di
> __builtin_altivec_vreplace_un_v4sf as __builtin_altivec_vreplace_un_sf
> __builtin_altivec_vreplace_un_v4si as __builtin_altivec_vreplace_un_si.
> Rename VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_UV4SI as
> VREPLACE_UN_USI, VREPLACE_UN_V2DF as VREPLACE_UN_DF,
> VREPLACE_UN_V2DI as VREPLACE_UN_DI, VREPLACE_UN_V4SF as
> VREPLACE_UN_SF, VREPLACE_UN_V4SI as VREPLACE_UN_SI.
> Rename vreplace_un_v2di as vreplace_un_di, vreplace_un_v4si as
> vreplace_un_si, vreplace_un_v2df as vreplace_un_df,
> vreplace_un_v2di as vreplace_un_di, vreplace_un_v4sf as
> vreplace_un_sf, vreplace_un_v4si as vreplace_un_si.
> * config/rs6000/rs6000-c.cc (find_instance): Add case
> RS6000_OVLD_VEC_REPLACE_UN.
> * config/rs6000/rs6000-overload.def (__builtin_vec_replace_un):
> Fix first argument type. Rename VREPLACE_UN_UV4SI as
> VREPLACE_UN_USI, VREPLACE_UN_V4SI as VREPLACE_UN_SI,
> VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_V2DI as
> VREPLACE_UN_DI, VREPLACE_UN_V4SF as VREPLACE_UN_SF,
> VREPLACE_UN_V2DF as VREPLACE_UN_DF.
> * config/rs6000/vsx.md (REPLACE_ELT): Renamed the mode_iterator
> REPLACE_ELT_V.
> (REPLACE_ELT_char): Renamed REPLACE_ELT_V_char.
> (REPLACE_ELT_sh): Renamed REPLACE_ELT_V_sh.
> (REPLACE_ELT_max): Renamed REPLACE_ELT_V_max.
> (REPLACE_ELT): New mode iterator.
> (REPLACE_ELT_char): New mode attribute.
> (vreplace_un_<mode>): Change iterator and mode attribute.
>
> gcc/testsuite/ChangeLog:
> * gcc.target/powerpc/vec-replace-word-runnable.c: Renamed
> vec-replace-word-runnable_1.c.
> * gcc.target/powerpc/vec-replace-word-runnable_1.c
> (dg-options): add -flax-vector-conversions.
> (vec_replace_unaligned) Fix first argument type.
> (vresult_uchar): Fix expected results.
> (vec_replace_unaligned): Update for loop to check uchar results.
> Remove extra spaces in if statements. Insert missing spaces in
> for statements.
> * gcc.target/powerpc/vec-replace-word-runnable_2.c: New test file.
> ---
> gcc/config/rs6000/rs6000-builtins.def | 28 +--
> gcc/config/rs6000/rs6000-c.cc | 23 +++
> gcc/config/rs6000/rs6000-overload.def | 24 +--
> gcc/config/rs6000/vsx.md | 52 ++---
> ...nnable.c => vec-replace-word-runnable_1.c} | 177 ++++++++++--------
> .../powerpc/vec-replace-word-runnable_2.c | 50 +++++
> 6 files changed, 224 insertions(+), 130 deletions(-)
> rename gcc/testsuite/gcc.target/powerpc/{vec-replace-word-runnable.c => vec-replace-word-runnable_1.c} (51%)
> create mode 100644 gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_2.c
>
> diff --git a/gcc/config/rs6000/rs6000-builtins.def b/gcc/config/rs6000/rs6000-builtins.def
> index c0ef717161f..58c53cd462e 100644
> --- a/gcc/config/rs6000/rs6000-builtins.def
> +++ b/gcc/config/rs6000/rs6000-builtins.def
> @@ -3397,26 +3397,26 @@
> const vull __builtin_altivec_vpextd (vull, vull);
> VPEXTD vpextd {}
>
> - const vuc __builtin_altivec_vreplace_un_uv2di (vull, unsigned long long, \
> - const int<4>);
> - VREPLACE_UN_UV2DI vreplace_un_v2di {}
> + const vuc __builtin_altivec_vreplace_un_udi (vuc, unsigned long long, \
> + const int<4>);
> + VREPLACE_UN_UDI vreplace_un_di {}
>
> - const vuc __builtin_altivec_vreplace_un_uv4si (vui, unsigned int, \
> - const int<4>);
> - VREPLACE_UN_UV4SI vreplace_un_v4si {}
> + const vuc __builtin_altivec_vreplace_un_usi (vuc, unsigned int, \
> + const int<4>);
> + VREPLACE_UN_USI vreplace_un_si {}
>
> - const vuc __builtin_altivec_vreplace_un_v2df (vd, double, const int<4>);
> - VREPLACE_UN_V2DF vreplace_un_v2df {}
> + const vuc __builtin_altivec_vreplace_un_df (vuc, double, const int<4>);
> + VREPLACE_UN_DF vreplace_un_df {}
>
> - const vuc __builtin_altivec_vreplace_un_v2di (vsll, signed long long, \
> + const vuc __builtin_altivec_vreplace_un_di (vuc, signed long long, \
> const int<4>);
> - VREPLACE_UN_V2DI vreplace_un_v2di {}
> + VREPLACE_UN_DI vreplace_un_di {}
>
> - const vuc __builtin_altivec_vreplace_un_v4sf (vf, float, const int<4>);
> - VREPLACE_UN_V4SF vreplace_un_v4sf {}
> + const vuc __builtin_altivec_vreplace_un_sf (vuc, float, const int<4>);
> + VREPLACE_UN_SF vreplace_un_sf {}
>
> - const vuc __builtin_altivec_vreplace_un_v4si (vsi, signed int, const int<4>);
> - VREPLACE_UN_V4SI vreplace_un_v4si {}
> + const vuc __builtin_altivec_vreplace_un_si (vuc, signed int, const int<4>);
> + VREPLACE_UN_SI vreplace_un_si {}
>
> const vull __builtin_altivec_vreplace_uv2di (vull, unsigned long long, \
> const int<1>);
> diff --git a/gcc/config/rs6000/rs6000-c.cc b/gcc/config/rs6000/rs6000-c.cc
> index 350987b851b..63ab360d352 100644
> --- a/gcc/config/rs6000/rs6000-c.cc
> +++ b/gcc/config/rs6000/rs6000-c.cc
> @@ -1968,6 +1968,29 @@ altivec_resolve_overloaded_builtin (location_t loc, tree fndecl,
> instance_code = RS6000_BIF_VSIEDP;
> }
>
> + tree call = find_instance (&unsupported_builtin, &instance,
> + instance_code, fcode, types, args, nargs);
> + if (call != error_mark_node)
> + return call;
> + break;
> + }
> + case RS6000_OVLD_VEC_REPLACE_UN:
> + {
> + machine_mode arg2_mode = TYPE_MODE (types[1]);
> +
> + if (arg2_mode == SImode)
> + /* Signed and unsigned are handled the same. */
> + instance_code = RS6000_BIF_VREPLACE_UN_USI;
> + else if (arg2_mode == SFmode)
> + instance_code = RS6000_BIF_VREPLACE_UN_SF;
> + else if (arg2_mode == DImode)
> + /* Signed and unsigned are handled the same. */
> + instance_code = RS6000_BIF_VREPLACE_UN_UDI;
> + else if (arg2_mode == DFmode)
> + instance_code = RS6000_BIF_VREPLACE_UN_DF;
> + else
> + break;
> +
> tree call = find_instance (&unsupported_builtin, &instance,
> instance_code, fcode, types, args, nargs);
> if (call != error_mark_node)
> diff --git a/gcc/config/rs6000/rs6000-overload.def b/gcc/config/rs6000/rs6000-overload.def
> index 470d718efde..b83946f5ad8 100644
> --- a/gcc/config/rs6000/rs6000-overload.def
> +++ b/gcc/config/rs6000/rs6000-overload.def
> @@ -3059,18 +3059,18 @@
> VREPLACE_ELT_V2DF
>
> [VEC_REPLACE_UN, vec_replace_unaligned, __builtin_vec_replace_un]
> - vuc __builtin_vec_replace_un (vui, unsigned int, const int);
> - VREPLACE_UN_UV4SI
> - vuc __builtin_vec_replace_un (vsi, signed int, const int);
> - VREPLACE_UN_V4SI
> - vuc __builtin_vec_replace_un (vull, unsigned long long, const int);
> - VREPLACE_UN_UV2DI
> - vuc __builtin_vec_replace_un (vsll, signed long long, const int);
> - VREPLACE_UN_V2DI
> - vuc __builtin_vec_replace_un (vf, float, const int);
> - VREPLACE_UN_V4SF
> - vuc __builtin_vec_replace_un (vd, double, const int);
> - VREPLACE_UN_V2DF
> + vuc __builtin_vec_replace_un (vuc, unsigned int, const int);
> + VREPLACE_UN_USI
> + vuc __builtin_vec_replace_un (vuc, signed int, const int);
> + VREPLACE_UN_SI
> + vuc __builtin_vec_replace_un (vuc, unsigned long long, const int);
> + VREPLACE_UN_UDI
> + vuc __builtin_vec_replace_un (vuc, signed long long, const int);
> + VREPLACE_UN_DI
> + vuc __builtin_vec_replace_un (vuc, float, const int);
> + VREPLACE_UN_SF
> + vuc __builtin_vec_replace_un (vuc, double, const int);
> + VREPLACE_UN_DF
>
> [VEC_REVB, vec_revb, __builtin_vec_revb]
> vss __builtin_vec_revb (vss);
> diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> index 0c269e4e8d9..d76f9f4864e 100644
> --- a/gcc/config/rs6000/vsx.md
> +++ b/gcc/config/rs6000/vsx.md
> @@ -381,13 +381,20 @@ (define_int_attr xvcvbf16 [(UNSPEC_VSX_XVCVSPBF16 "xvcvspbf16")
> (define_mode_iterator VI2 [V4SI V8HI V16QI V2DI])
>
> ;; Vector extract_elt iterator/attr for 32-bit and 64-bit elements
> -(define_mode_iterator REPLACE_ELT [V4SI V4SF V2DI V2DF])
> -(define_mode_attr REPLACE_ELT_char [(V4SI "w") (V4SF "w")
> - (V2DI "d") (V2DF "d")])
> -(define_mode_attr REPLACE_ELT_sh [(V4SI "2") (V4SF "2")
> - (V2DI "3") (V2DF "3")])
> -(define_mode_attr REPLACE_ELT_max [(V4SI "12") (V4SF "12")
> - (V2DI "8") (V2DF "8")])
> +(define_mode_iterator REPLACE_ELT_V [V4SI V4SF V2DI V2DF])
> +(define_mode_attr REPLACE_ELT_V_char [(V4SI "w") (V4SF "w")
> + (V2DI "d") (V2DF "d")])
> +(define_mode_attr REPLACE_ELT_V_sh [(V4SI "2") (V4SF "2")
> + (V2DI "3") (V2DF "3")])
> +(define_mode_attr REPLACE_ELT_V_max [(V4SI "12") (V4SF "12")
> + (V2DI "8") (V2DF "8")])
> +
> +;; vector replace unaligned modes
> +(define_mode_iterator REPLACE_ELT [SI SF DI DF])
> +(define_mode_attr REPLACE_ELT_char [(SI "w")
> + (SF "w")
> + (DI "d")
> + (DF "d")])
Sorry if I didn't describe it clearly, I actually adviced that:
(define_mode_iterator REPLACE_ELT_V [V4SI V4SF V2DI V2DF])
(define_mode_iterator REPLACE_ELT [SI SF DI DF])
(define_mode_attr REPLACE_ELT_char [(V4SI "w") (V4SF "w")
(V2DI "d") (V2DF "d")
(SI "w") (SF "w")
(DI "d") (DF "d")])
It leaves REPLACE_ELT_sh and REPLACE_ELT_max unchanged, otherwise
we have to update the preparation statements like what you did in
this patch, IMHO both aligned and unaligned versions are replacing
things with the element size, it's not quite meaningful to further
distinguish the mode attributes.
>
> ;; Like VM2 in altivec.md, just do char, short, int, long, float and double
> (define_mode_iterator VM3 [V4SI
> @@ -4183,21 +4190,22 @@ (define_insn "vinsertgr_internal_<mode>"
> [(set_attr "type" "vecsimple")])
>
> (define_expand "vreplace_elt_<mode>"
> - [(set (match_operand:REPLACE_ELT 0 "register_operand")
> - (unspec:REPLACE_ELT [(match_operand:REPLACE_ELT 1 "register_operand")
> - (match_operand:<VEC_base> 2 "register_operand")
> - (match_operand:QI 3 "const_0_to_3_operand")]
> - UNSPEC_REPLACE_ELT))]
> + [(set (match_operand:REPLACE_ELT_V 0 "register_operand")
> + (unspec:REPLACE_ELT_V [(match_operand:REPLACE_ELT_V 1 "register_operand")
> + (match_operand:<VEC_base> 2 "register_operand")
> + (match_operand:QI 3 "const_0_to_3_operand")]
> + UNSPEC_REPLACE_ELT))]
> "TARGET_POWER10"
> {
> int index;
> /* Immediate value is the word index, convert to byte index and adjust for
> Endianness if needed. */
> if (BYTES_BIG_ENDIAN)
> - index = INTVAL (operands[3]) << <REPLACE_ELT_sh>;
> + index = INTVAL (operands[3]) << <REPLACE_ELT_V_sh>;
>
> else
> - index = <REPLACE_ELT_max> - (INTVAL (operands[3]) << <REPLACE_ELT_sh>);
> + index = <REPLACE_ELT_V_max>
> + - (INTVAL (operands[3]) << <REPLACE_ELT_V_sh>);
>
> emit_insn (gen_vreplace_elt_<mode>_inst (operands[0], operands[1],
> operands[2],
> @@ -4207,19 +4215,19 @@ (define_expand "vreplace_elt_<mode>"
> [(set_attr "type" "vecsimple")])
>
> (define_insn "vreplace_elt_<mode>_inst"
> - [(set (match_operand:REPLACE_ELT 0 "register_operand" "=v")
> - (unspec:REPLACE_ELT [(match_operand:REPLACE_ELT 1 "register_operand" "0")
> - (match_operand:<VEC_base> 2 "register_operand" "r")
> - (match_operand:QI 3 "const_0_to_12_operand" "n")]
> - UNSPEC_REPLACE_ELT))]
> + [(set (match_operand:REPLACE_ELT_V 0 "register_operand" "=v")
> + (unspec:REPLACE_ELT_V [(match_operand:REPLACE_ELT_V 1 "register_operand" "0")
> + (match_operand:<VEC_base> 2 "register_operand" "r")
> + (match_operand:QI 3 "const_0_to_12_operand" "n")]
> + UNSPEC_REPLACE_ELT))]
> "TARGET_POWER10"
> - "vins<REPLACE_ELT_char> %0,%2,%3"
> + "vins<REPLACE_ELT_V_char> %0,%2,%3"
> [(set_attr "type" "vecsimple")])
>
> (define_insn "vreplace_un_<mode>"
> [(set (match_operand:V16QI 0 "register_operand" "=v")
> - (unspec:V16QI [(match_operand:REPLACE_ELT 1 "register_operand" "0")
> - (match_operand:<VEC_base> 2 "register_operand" "r")
> + (unspec:V16QI [(match_operand:V16QI 1 "register_operand" "0")
> + (match_operand:REPLACE_ELT 2 "register_operand" "r")
> (match_operand:QI 3 "const_0_to_12_operand" "n")]
> UNSPEC_REPLACE_UN))]
> "TARGET_POWER10"
> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable.c b/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_1.c
> similarity index 51%
> rename from gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable.c
> rename to gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_1.c
> index 27318822871..7e7485ca371 100644
> --- a/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable.c
> +++ b/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable_1.c
> @@ -1,5 +1,4 @@
> /* { dg-do run { target { power10_hw } } } */
> -/* { dg-do link { target { ! power10_hw } } } */
As your explanation that this "dg-do link" has existed for a long time,
I'd a check and found it's from r11-4397-g8d8fef197114a9 "[RS6000] Link
power10 testcases", I think it wanted to test assembler and linker when
there are few actual P10 hw (imagining at the early stage of P10 support).
Since there are a few test cases like this, I'm inclined to just leave it
alone.
BR,
Kewen
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] rs6000, add argument to function find_instance
2023-07-21 2:19 ` Kewen.Lin
@ 2023-07-21 23:23 ` Carl Love
0 siblings, 0 replies; 7+ messages in thread
From: Carl Love @ 2023-07-21 23:23 UTC (permalink / raw)
To: Kewen.Lin, cel
Cc: Peter Bergner, Segher Boessenkool, David Edelsohn, gcc-patches
On Fri, 2023-07-21 at 10:19 +0800, Kewen.Lin wrote:
> Hi Carl,
>
> on 2023/7/18 03:19, Carl Love wrote:
> > GCC maintainers:
> >
> > The rs6000 function find_instance assumes that it is called for
> > built-
> > ins with only two arguments. There is no checking for the actual
> > number of aruguments used in the built-in. This patch adds an
> > additional parameter to the function call containing the number of
> > aruguments in the built-in. The function will now do the needed
> > checks
> > for all of the arguments.
> >
> > This fix is needed for the next patch in the series that fixes the
> > vec_replace_unaligned built-in.c test.
> >
> > Please let me know if this patch is acceptable for
> > mainline. Thanks.
> >
> > Carl
> >
> >
> > --------------------------------------------
> > rs6000, add argument to function find_instance
> >
> > The function find_instance assumes it is called to check a built-
> > in with
Fixed
> > ~~ two spaces.
> > only two arguments. Ths patch extends the function by adding a
> > parameter
> s/Ths/This/
> > specifying the number of buit-in arguments to check.
> s/bult-in/built-in/
>
Fixed both typos.
> > gcc/ChangeLog:
> > * config/rs6000/rs6000-c.cc (find_instance): Add new parameter
> > that
> > specifies the number of built-in arguments to check.
> > (altivec_resolve_overloaded_builtin): Update calls to
> > find_instance
> > to pass the number of built-in argument to be checked.
>
> s/argument/arguments/
fixed
>
> > ---
> > gcc/config/rs6000/rs6000-c.cc | 27 +++++++++++++++++++--------
> > 1 file changed, 19 insertions(+), 8 deletions(-)
> >
> > diff --git a/gcc/config/rs6000/rs6000-c.cc
> > b/gcc/config/rs6000/rs6000-c.cc
> > index a353bca19ef..350987b851b 100644
> > --- a/gcc/config/rs6000/rs6000-c.cc
> > +++ b/gcc/config/rs6000/rs6000-c.cc
> > @@ -1679,7 +1679,7 @@ tree
>
> There is one function comment here describing the meaning of each
> parameter,
> I think we should add a corresponding for NARGS, may be something
> like:
>
> "; and NARGS specifies the number of built-in arguments."
>
Added NARGS description.
> Also we need to update the below "two"s with "NARGS".
>
> "TYPES contains an array of two types..." and "ARGS contains an array
> of two arguments..."
>
Replaced multiple "two" occurrences with NARGS.
> since we already extend this to handle NARGS instead of two.
>
> > find_instance (bool *unsupported_builtin, ovlddata **instance,
> > rs6000_gen_builtins instance_code,
> > rs6000_gen_builtins fcode,
> > - tree *types, tree *args)
> > + tree *types, tree *args, int nargs)
> > {
> > while (*instance && (*instance)->bifid != instance_code)
> > *instance = (*instance)->next;
> > @@ -1691,17 +1691,28 @@ find_instance (bool *unsupported_builtin,
> > ovlddata **instance,
> > if (!inst->fntype)
> > return error_mark_node;
> > tree fntype = rs6000_builtin_info[inst->bifid].fntype;
> > - tree parmtype0 = TREE_VALUE (TYPE_ARG_TYPES (fntype));
> > - tree parmtype1 = TREE_VALUE (TREE_CHAIN (TYPE_ARG_TYPES
> > (fntype)));
> > + tree argtype = TYPE_ARG_TYPES (fntype);
> > + tree parmtype;
>
> Nit: We can move "tree parmtype" into the loop (close to its only
> use).
Moved and combined declaration with assignment as you noted below.
>
> > + int args_compatible = true;
>
> s/int/bool/
Changed.
>
> >
> > - if (rs6000_builtin_type_compatible (types[0], parmtype0)
> > - && rs6000_builtin_type_compatible (types[1], parmtype1))
> > + for (int i = 0; i <nargs; i++)
>
> Nit: formatting issue, space before nargs.
>
> > {
> > + parmtype = TREE_VALUE (argtype);
>
> tree parmtype = TREE_VALUE (argtype);
Changed
>
> > + if (! rs6000_builtin_type_compatible (types[i], parmtype))
>
> Nit: One unexpected(?) space after "!".
Removed extra space after "!".
>
> > + {
> > + args_compatible = false;
> > + break;
> > + }
> > + argtype = TREE_CHAIN (argtype);
> > + }
> > +
> > + if (args_compatible)
> > + {
>
> Nit: indent issue for "{".
Fixed indent.
>
> Ok for trunk with these nits fixed. Btw, the description doesn't say
> how this was tested, I'm not sure if it's only tested together with
> "patch 2/2", but please ensure it's bootstrapped and regress-tested
> on BE and LE when committing. Thanks!
>
Yes, it was tested with patch 2/2 on Power 10 LE. I did do a test on
Power 9 as well but don't recall if I tested for both BE and LE. Will
retest on Power 8 LE/BE, Power 9 LE/BE and Power 10.
Carl
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2 ver 4] rs6000, fix vec_replace_unaligned built-in arguments
2023-07-21 5:04 ` Kewen.Lin
@ 2023-07-21 23:23 ` Carl Love
0 siblings, 0 replies; 7+ messages in thread
From: Carl Love @ 2023-07-21 23:23 UTC (permalink / raw)
To: Kewen.Lin, cel
Cc: Peter Bergner, Segher Boessenkool, David Edelsohn, gcc-patches
On Fri, 2023-07-21 at 13:04 +0800, Kewen.Lin wrote:
> Hi Carl,
>
> on 2023/7/18 03:20, Carl Love wrote:
> > GCC maintainers:
> >
> > Version 4, changed the new RS6000_OVLD_VEC_REPLACE_UN case
> > statement
> > rs6000/rs6000-c.cc. The existing REPLACE_ELT iterator name was
> > changed
> > to REPLACE_ELT_V along with the associated
> > define_mode_attr. Renamed
> > VEC_RU to REPLACE_ELT for the iterator name and VEC_RU_char to
> > REPLACE_ELT_char. Fixed the double test in vec-replace-word-
> > runnable_1.c to be consistent with the other tests. Removed the
> > "dg-do
> > link" from both tests. Put in an explicit cast in test vec-
> > replace-word-runnable_2.c to eliminate the need for the -flax-
> > vector-conversions dg-option.
> >
> > Version 3, added code to altivec_resolve_overloaded_builtin so the
> > correct instruction is selected for the size of the second
> > argument.
> > This restores the instruction counts to the original values where
> > the
> > correct instructions were originally being generated. The naming
> > of
> > the overloaded builtin instances and builtin definitions were
> > changed
> > to reflect the type of the second argument since the type of the
> > first
> > argument is now the same for all overloaded instances. A new
> > builtin
> > test file was added for the case where the first argument is cast
> > to
> > the unsigned long long type. This test requires the -flax-vector-
> > conversions gcc command line option. Since the other tests do not
> > require this option, I felt that the new test needed to be in a
> > separate file. Finally some formatting fixes were made in the
> > original
> > test file. Patch has been retested on Power 10 with no
> > regressions.
> >
> > Version 2, fixed various typos. Updated the change log body to say
> > the
> > instruction counts were updated. The instruction counts changed as
> > a
> > result of changing the first argument of the vec_replace_unaligned
> > builtin call from vector unsigned long long (vull) to vector
> > unsigned
> > char (vuc). When the first argument was vull the builtin call
> > generated the vinsd instruction for the two test cases. The
> > updated
> > call with vuc as the first argument generates two vinsw
> > instructions
> > instead. Patch was retested on Power 10 with no regressions.
> >
> > The following patch fixes the first argument in the builtin
> > definition
> > and the corresponding test cases. Initially, the builtin
> > specification
> > was wrong due to a cut and past error. The documentation was fixed
> > in:
> >
> > commit ed3fea09b18f67e757b5768b42cb6e816626f1db
> > Author: Bill Schmidt <wschmidt@linux.ibm.com>
> > Date: Fri Feb 4 13:07:17 2022 -0600
> >
> > rs6000: Correct function prototypes for
> > vec_replace_unaligned
> >
> > Due to a pasto error in the documentation,
> > vec_replace_unaligned was
> > implemented with the same function prototypes as
> > vec_replace_elt.
> > It was intended that vec_replace_unaligned always specify
> > output
> > vectors as having type vector unsigned char, to emphasize
> > that
> > elements are potentially misaligned by this built-in
> > function.
> > This patch corrects the misimplementation.
> > ....
> >
> > This patch fixes the arguments in the definitions and updates the
> > testcases accordingly. Additionally, a few minor spacing issues
> > are
> > fixed.
> >
> > The patch has been tested on Power 10 with no regressions. Please
> > let
> > me know if the patch is acceptable for mainline. Thanks.
> >
> > Carl
> >
> >
> > ----------------------------
> > rs6000, fix vec_replace_unaligned built-in arguments
> >
> > The first argument of the vec_replace_unaligned built-in should
> > always be
> > of type unsigned char, as specified in gcc/doc/extend.texi.
>
> Shouldn't be "vector unsigned char" instead of "unsigned char"?
>
> Or do I miss something?
Nope, I missed saying "vector". Fixed.
>
> > This patch fixes the builtin definitions and updates the test cases
> > to use
> > the correct arguments. The original test file is renamed and a
> > second test
> > file is added for a new test case.
> >
> > gcc/ChangeLog:
> > * config/rs6000/rs6000-builtins.def: Rename
> > __builtin_altivec_vreplace_un_uv2di as
> > __builtin_altivec_vreplace_un_udi
> > __builtin_altivec_vreplace_un_uv4si as
> > __builtin_altivec_vreplace_un_usi
> > __builtin_altivec_vreplace_un_v2df as
> > __builtin_altivec_vreplace_un_df
> > __builtin_altivec_vreplace_un_v2di as
> > __builtin_altivec_vreplace_un_di
> > __builtin_altivec_vreplace_un_v4sf as
> > __builtin_altivec_vreplace_un_sf
> > __builtin_altivec_vreplace_un_v4si as
> > __builtin_altivec_vreplace_un_si.
> > Rename VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_UV4SI
> > as
> > VREPLACE_UN_USI, VREPLACE_UN_V2DF as VREPLACE_UN_DF,
> > VREPLACE_UN_V2DI as VREPLACE_UN_DI, VREPLACE_UN_V4SF as
> > VREPLACE_UN_SF, VREPLACE_UN_V4SI as VREPLACE_UN_SI.
> > Rename vreplace_un_v2di as vreplace_un_di, vreplace_un_v4si as
> > vreplace_un_si, vreplace_un_v2df as vreplace_un_df,
> > vreplace_un_v2di as vreplace_un_di, vreplace_un_v4sf as
> > vreplace_un_sf, vreplace_un_v4si as vreplace_un_si.
> > * config/rs6000/rs6000-c.cc (find_instance): Add case
> > RS6000_OVLD_VEC_REPLACE_UN.
> > * config/rs6000/rs6000-overload.def (__builtin_vec_replace_un):
> > Fix first argument type. Rename VREPLACE_UN_UV4SI as
> > VREPLACE_UN_USI, VREPLACE_UN_V4SI as VREPLACE_UN_SI,
> > VREPLACE_UN_UV2DI as VREPLACE_UN_UDI, VREPLACE_UN_V2DI as
> > VREPLACE_UN_DI, VREPLACE_UN_V4SF as VREPLACE_UN_SF,
> > VREPLACE_UN_V2DF as VREPLACE_UN_DF.
> > * config/rs6000/vsx.md (REPLACE_ELT): Renamed the mode_iterator
> > REPLACE_ELT_V.
> > (REPLACE_ELT_char): Renamed REPLACE_ELT_V_char.
> > (REPLACE_ELT_sh): Renamed REPLACE_ELT_V_sh.
> > (REPLACE_ELT_max): Renamed REPLACE_ELT_V_max.
> > (REPLACE_ELT): New mode iterator.
> > (REPLACE_ELT_char): New mode attribute.
> > (vreplace_un_<mode>): Change iterator and mode attribute.
> >
> > gcc/testsuite/ChangeLog:
> > * gcc.target/powerpc/vec-replace-word-runnable.c: Renamed
> > vec-replace-word-runnable_1.c.
> > * gcc.target/powerpc/vec-replace-word-runnable_1.c
> > (dg-options): add -flax-vector-conversions.
> > (vec_replace_unaligned) Fix first argument type.
> > (vresult_uchar): Fix expected results.
> > (vec_replace_unaligned): Update for loop to check uchar
> > results.
> > Remove extra spaces in if statements. Insert missing spaces in
> > for statements.
> > * gcc.target/powerpc/vec-replace-word-runnable_2.c: New test
> > file.
> > ---
> > gcc/config/rs6000/rs6000-builtins.def | 28 +--
> > gcc/config/rs6000/rs6000-c.cc | 23 +++
> > gcc/config/rs6000/rs6000-overload.def | 24 +--
> > gcc/config/rs6000/vsx.md | 52 ++---
> > ...nnable.c => vec-replace-word-runnable_1.c} | 177 ++++++++++--
> > ------
> > .../powerpc/vec-replace-word-runnable_2.c | 50 +++++
> > 6 files changed, 224 insertions(+), 130 deletions(-)
> > rename gcc/testsuite/gcc.target/powerpc/{vec-replace-word-
> > runnable.c => vec-replace-word-runnable_1.c} (51%)
> > create mode 100644 gcc/testsuite/gcc.target/powerpc/vec-replace-
> > word-runnable_2.c
> >
> > diff --git a/gcc/config/rs6000/rs6000-builtins.def
> > b/gcc/config/rs6000/rs6000-builtins.def
> > index c0ef717161f..58c53cd462e 100644
> > --- a/gcc/config/rs6000/rs6000-builtins.def
> > +++ b/gcc/config/rs6000/rs6000-builtins.def
> > @@ -3397,26 +3397,26 @@
> > const vull __builtin_altivec_vpextd (vull, vull);
> > VPEXTD vpextd {}
> >
> > - const vuc __builtin_altivec_vreplace_un_uv2di (vull, unsigned
> > long long, \
> > - const int<4>);
> > - VREPLACE_UN_UV2DI vreplace_un_v2di {}
> > + const vuc __builtin_altivec_vreplace_un_udi (vuc, unsigned long
> > long, \
> > + const int<4>);
> > + VREPLACE_UN_UDI vreplace_un_di {}
> >
> > - const vuc __builtin_altivec_vreplace_un_uv4si (vui, unsigned
> > int, \
> > - const int<4>);
> > - VREPLACE_UN_UV4SI vreplace_un_v4si {}
> > + const vuc __builtin_altivec_vreplace_un_usi (vuc, unsigned int,
> > \
> > + const int<4>);
> > + VREPLACE_UN_USI vreplace_un_si {}
> >
> > - const vuc __builtin_altivec_vreplace_un_v2df (vd, double, const
> > int<4>);
> > - VREPLACE_UN_V2DF vreplace_un_v2df {}
> > + const vuc __builtin_altivec_vreplace_un_df (vuc, double, const
> > int<4>);
> > + VREPLACE_UN_DF vreplace_un_df {}
> >
> > - const vuc __builtin_altivec_vreplace_un_v2di (vsll, signed long
> > long, \
> > + const vuc __builtin_altivec_vreplace_un_di (vuc, signed long
> > long, \
> > const int<4>);
> > - VREPLACE_UN_V2DI vreplace_un_v2di {}
> > + VREPLACE_UN_DI vreplace_un_di {}
> >
> > - const vuc __builtin_altivec_vreplace_un_v4sf (vf, float, const
> > int<4>);
> > - VREPLACE_UN_V4SF vreplace_un_v4sf {}
> > + const vuc __builtin_altivec_vreplace_un_sf (vuc, float, const
> > int<4>);
> > + VREPLACE_UN_SF vreplace_un_sf {}
> >
> > - const vuc __builtin_altivec_vreplace_un_v4si (vsi, signed int,
> > const int<4>);
> > - VREPLACE_UN_V4SI vreplace_un_v4si {}
> > + const vuc __builtin_altivec_vreplace_un_si (vuc, signed int,
> > const int<4>);
> > + VREPLACE_UN_SI vreplace_un_si {}
> >
> > const vull __builtin_altivec_vreplace_uv2di (vull, unsigned long
> > long, \
> > const int<1>);
> > diff --git a/gcc/config/rs6000/rs6000-c.cc
> > b/gcc/config/rs6000/rs6000-c.cc
> > index 350987b851b..63ab360d352 100644
> > --- a/gcc/config/rs6000/rs6000-c.cc
> > +++ b/gcc/config/rs6000/rs6000-c.cc
> > @@ -1968,6 +1968,29 @@ altivec_resolve_overloaded_builtin
> > (location_t loc, tree fndecl,
> > instance_code = RS6000_BIF_VSIEDP;
> > }
> >
> > + tree call = find_instance (&unsupported_builtin, &instance,
> > + instance_code, fcode, types, args,
> > nargs);
> > + if (call != error_mark_node)
> > + return call;
> > + break;
> > + }
> > + case RS6000_OVLD_VEC_REPLACE_UN:
> > + {
> > + machine_mode arg2_mode = TYPE_MODE (types[1]);
> > +
> > + if (arg2_mode == SImode)
> > + /* Signed and unsigned are handled the same. */
> > + instance_code = RS6000_BIF_VREPLACE_UN_USI;
> > + else if (arg2_mode == SFmode)
> > + instance_code = RS6000_BIF_VREPLACE_UN_SF;
> > + else if (arg2_mode == DImode)
> > + /* Signed and unsigned are handled the same. */
> > + instance_code = RS6000_BIF_VREPLACE_UN_UDI;
> > + else if (arg2_mode == DFmode)
> > + instance_code = RS6000_BIF_VREPLACE_UN_DF;
> > + else
> > + break;
> > +
> > tree call = find_instance (&unsupported_builtin, &instance,
> > instance_code, fcode, types, args,
> > nargs);
> > if (call != error_mark_node)
> > diff --git a/gcc/config/rs6000/rs6000-overload.def
> > b/gcc/config/rs6000/rs6000-overload.def
> > index 470d718efde..b83946f5ad8 100644
> > --- a/gcc/config/rs6000/rs6000-overload.def
> > +++ b/gcc/config/rs6000/rs6000-overload.def
> > @@ -3059,18 +3059,18 @@
> > VREPLACE_ELT_V2DF
> >
> > [VEC_REPLACE_UN, vec_replace_unaligned, __builtin_vec_replace_un]
> > - vuc __builtin_vec_replace_un (vui, unsigned int, const int);
> > - VREPLACE_UN_UV4SI
> > - vuc __builtin_vec_replace_un (vsi, signed int, const int);
> > - VREPLACE_UN_V4SI
> > - vuc __builtin_vec_replace_un (vull, unsigned long long, const
> > int);
> > - VREPLACE_UN_UV2DI
> > - vuc __builtin_vec_replace_un (vsll, signed long long, const
> > int);
> > - VREPLACE_UN_V2DI
> > - vuc __builtin_vec_replace_un (vf, float, const int);
> > - VREPLACE_UN_V4SF
> > - vuc __builtin_vec_replace_un (vd, double, const int);
> > - VREPLACE_UN_V2DF
> > + vuc __builtin_vec_replace_un (vuc, unsigned int, const int);
> > + VREPLACE_UN_USI
> > + vuc __builtin_vec_replace_un (vuc, signed int, const int);
> > + VREPLACE_UN_SI
> > + vuc __builtin_vec_replace_un (vuc, unsigned long long, const
> > int);
> > + VREPLACE_UN_UDI
> > + vuc __builtin_vec_replace_un (vuc, signed long long, const int);
> > + VREPLACE_UN_DI
> > + vuc __builtin_vec_replace_un (vuc, float, const int);
> > + VREPLACE_UN_SF
> > + vuc __builtin_vec_replace_un (vuc, double, const int);
> > + VREPLACE_UN_DF
> >
> > [VEC_REVB, vec_revb, __builtin_vec_revb]
> > vss __builtin_vec_revb (vss);
> > diff --git a/gcc/config/rs6000/vsx.md b/gcc/config/rs6000/vsx.md
> > index 0c269e4e8d9..d76f9f4864e 100644
> > --- a/gcc/config/rs6000/vsx.md
> > +++ b/gcc/config/rs6000/vsx.md
> > @@ -381,13 +381,20 @@ (define_int_attr
> > xvcvbf16 [(UNSPEC_VSX_XVCVSPBF16 "xvcvspbf16")
> > (define_mode_iterator VI2 [V4SI V8HI V16QI V2DI])
> >
> > ;; Vector extract_elt iterator/attr for 32-bit and 64-bit elements
Missed this earlier, I think the comment needs fixing as well since we
now have both vector and scalar now. Changed to:
;; Vector and scalar extract_elt iterator/attr for 32-bit and 64-bit
elements
> > -(define_mode_iterator REPLACE_ELT [V4SI V4SF V2DI V2DF])
> > -(define_mode_attr REPLACE_ELT_char [(V4SI "w") (V4SF "w")
> > - (V2DI "d") (V2DF "d")])
> > -(define_mode_attr REPLACE_ELT_sh [(V4SI "2") (V4SF "2")
> > - (V2DI "3") (V2DF "3")])
> > -(define_mode_attr REPLACE_ELT_max [(V4SI "12") (V4SF "12")
> > - (V2DI "8") (V2DF "8")])
> > +(define_mode_iterator REPLACE_ELT_V [V4SI V4SF V2DI V2DF])
> > +(define_mode_attr REPLACE_ELT_V_char [(V4SI "w") (V4SF "w")
> > + (V2DI "d") (V2DF "d")])
> > +(define_mode_attr REPLACE_ELT_V_sh [(V4SI "2") (V4SF "2")
> > + (V2DI "3") (V2DF "3")])
> > +(define_mode_attr REPLACE_ELT_V_max [(V4SI "12") (V4SF "12")
> > + (V2DI "8") (V2DF "8")])
> > +
> > +;; vector replace unaligned modes
> > +(define_mode_iterator REPLACE_ELT [SI SF DI DF])
> > +(define_mode_attr REPLACE_ELT_char [(SI "w")
> > + (SF "w")
> > + (DI "d")
> > + (DF "d")])
>
> Sorry if I didn't describe it clearly, I actually adviced that:
I think maybe it got lost in the first attempt to change this which
didn't work. Fixed it.
>
> (define_mode_iterator REPLACE_ELT_V [V4SI V4SF V2DI V2DF])
> (define_mode_iterator REPLACE_ELT [SI SF DI DF])
> (define_mode_attr REPLACE_ELT_char [(V4SI "w") (V4SF "w")
> (V2DI "d") (V2DF "d")
> (SI "w") (SF "w")
> (DI "d") (DF "d")])
>
Having a single mode_attr for scalar and vector is a bit cleaner.
> It leaves REPLACE_ELT_sh and REPLACE_ELT_max unchanged, otherwise
> we have to update the preparation statements like what you did in
> this patch, IMHO both aligned and unaligned versions are replacing
> things with the element size, it's not quite meaningful to further
> distinguish the mode attributes.
OK, so I reverted REPLACE_ELT_V_sh back to REPLACE_ELT_sh and
REPLACE_ELT_V_max back to REPLACE_ELT_max. Updated ChangeLog.
>
> >
> > ;; Like VM2 in altivec.md, just do char, short, int, long, float
> > and double
> > (define_mode_iterator VM3 [V4SI
> > @@ -4183,21 +4190,22 @@ (define_insn "vinsertgr_internal_<mode>"
> > [(set_attr "type" "vecsimple")])
> >
> > (define_expand "vreplace_elt_<mode>"
> > - [(set (match_operand:REPLACE_ELT 0 "register_operand")
> > - (unspec:REPLACE_ELT [(match_operand:REPLACE_ELT 1
> > "register_operand")
> > - (match_operand:<VEC_base> 2 "register_operand")
> > - (match_operand:QI 3 "const_0_to_3_operand")]
> > - UNSPEC_REPLACE_ELT))]
> > + [(set (match_operand:REPLACE_ELT_V 0 "register_operand")
> > + (unspec:REPLACE_ELT_V [(match_operand:REPLACE_ELT_V 1
> > "register_operand")
> > + (match_operand:<VEC_base> 2
> > "register_operand")
> > + (match_operand:QI 3 "const_0_to_3_operand")]
> > + UNSPEC_REPLACE_ELT))]
> > "TARGET_POWER10"
> > {
> > int index;
> > /* Immediate value is the word index, convert to byte index and
> > adjust for
> > Endianness if needed. */
> > if (BYTES_BIG_ENDIAN)
> > - index = INTVAL (operands[3]) << <REPLACE_ELT_sh>;
> > + index = INTVAL (operands[3]) << <REPLACE_ELT_V_sh>;
> >
> > else
> > - index = <REPLACE_ELT_max> - (INTVAL (operands[3]) <<
> > <REPLACE_ELT_sh>);
> > + index = <REPLACE_ELT_V_max>
> > + - (INTVAL (operands[3]) << <REPLACE_ELT_V_sh>);
> >
> > emit_insn (gen_vreplace_elt_<mode>_inst (operands[0],
> > operands[1],
> > operands[2],
> > @@ -4207,19 +4215,19 @@ (define_expand "vreplace_elt_<mode>"
> > [(set_attr "type" "vecsimple")])
> >
> > (define_insn "vreplace_elt_<mode>_inst"
> > - [(set (match_operand:REPLACE_ELT 0 "register_operand" "=v")
> > - (unspec:REPLACE_ELT [(match_operand:REPLACE_ELT 1
> > "register_operand" "0")
> > - (match_operand:<VEC_base> 2 "register_operand"
> > "r")
> > - (match_operand:QI 3 "const_0_to_12_operand"
> > "n")]
> > - UNSPEC_REPLACE_ELT))]
> > + [(set (match_operand:REPLACE_ELT_V 0 "register_operand" "=v")
> > + (unspec:REPLACE_ELT_V [(match_operand:REPLACE_ELT_V 1
> > "register_operand" "0")
> > + (match_operand:<VEC_base> 2 "register_operand"
> > "r")
> > + (match_operand:QI 3 "const_0_to_12_operand"
> > "n")]
> > + UNSPEC_REPLACE_ELT))]
> > "TARGET_POWER10"
> > - "vins<REPLACE_ELT_char> %0,%2,%3"
> > + "vins<REPLACE_ELT_V_char> %0,%2,%3"
> > [(set_attr "type" "vecsimple")])
> >
> > (define_insn "vreplace_un_<mode>"
> > [(set (match_operand:V16QI 0 "register_operand" "=v")
> > - (unspec:V16QI [(match_operand:REPLACE_ELT 1 "register_operand"
> > "0")
> > - (match_operand:<VEC_base> 2 "register_operand"
> > "r")
> > + (unspec:V16QI [(match_operand:V16QI 1 "register_operand" "0")
> > + (match_operand:REPLACE_ELT 2 "register_operand" "r")
> > (match_operand:QI 3 "const_0_to_12_operand" "n")]
> > UNSPEC_REPLACE_UN))]
> > "TARGET_POWER10"
> > diff --git a/gcc/testsuite/gcc.target/powerpc/vec-replace-word-
> > runnable.c b/gcc/testsuite/gcc.target/powerpc/vec-replace-word-
> > runnable_1.c
> > similarity index 51%
> > rename from gcc/testsuite/gcc.target/powerpc/vec-replace-word-
> > runnable.c
> > rename to gcc/testsuite/gcc.target/powerpc/vec-replace-word-
> > runnable_1.c
> > index 27318822871..7e7485ca371 100644
> > --- a/gcc/testsuite/gcc.target/powerpc/vec-replace-word-runnable.c
> > +++ b/gcc/testsuite/gcc.target/powerpc/vec-replace-word-
> > runnable_1.c
> > @@ -1,5 +1,4 @@
> > /* { dg-do run { target { power10_hw } } } */
> > -/* { dg-do link { target { ! power10_hw } } } */
>
> As your explanation that this "dg-do link" has existed for a long
> time,
> I'd a check and found it's from r11-4397-g8d8fef197114a9 "[RS6000]
> Link
> power10 testcases", I think it wanted to test assembler and linker
> when
> there are few actual P10 hw (imagining at the early stage of P10
> support).
>
> Since there are a few test cases like this, I'm inclined to just
> leave it
OK, put it back in.
Carl
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-07-21 23:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-17 18:51 [PATCH 0/2] rs6000, fix vec_replace_unaligned built-in arguments Carl Love
2023-07-17 19:19 ` [PATCH 1/2] rs6000, add argument to function find_instance Carl Love
2023-07-21 2:19 ` Kewen.Lin
2023-07-21 23:23 ` Carl Love
2023-07-17 19:20 ` [PATCH 2/2 ver 4] rs6000, fix vec_replace_unaligned built-in arguments Carl Love
2023-07-21 5:04 ` Kewen.Lin
2023-07-21 23:23 ` Carl Love
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).