[v1] LoongArch: Modify the check type of the vector builtin function.
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
On LoongArch architecture, using the latest gcc14 in regression test,
it is found that the vector test cases in vector directory appear FAIL
entries with unmatched pointer types. In order to solve this kind of
problem, the type of the variable in the check result is modified with
the parameter type defined in the vector builtin function.
gcc/testsuite/ChangeLog:
* gcc.target/loongarch/vector/simd_correctness_check.h:The variable
types in the check results are modified in conjunction with the
parameter types defined in the vector builtin function.
---
.../gcc.target/loongarch/vector/simd_correctness_check.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
Comments
On Mon, 2023-12-04 at 20:14 +0800, chenxiaolong wrote:
> On LoongArch architecture, using the latest gcc14 in regression test,
> it is found that the vector test cases in vector directory appear FAIL
> entries with unmatched pointer types. In order to solve this kind of
> problem, the type of the variable in the check result is modified with
> the parameter type defined in the vector builtin function.
IMO we should write something more readable:
static inline
void ASSERTEQ_64 (int line, const void *_ref, const void *_res)
{
if (memcmp (_ref, _res, 16) == 0)
return;
const char *ref = (const char *)_ref;
const char *res = (const char *)_res;
printf ("error %s:%d: result mismatch\n", __FILE__, line);
printf ("ref:");
for (int i = 0; i < 16; i++)
printf (" %02x", ref[i]);
printf ("\nres:");
for (int i = 0; i < 16; i++)
printf (" %02x", res[i]);
putchar ('\n');
abort ();
}
> gcc/testsuite/ChangeLog:
>
> * gcc.target/loongarch/vector/simd_correctness_check.h:The variable
> types in the check results are modified in conjunction with the
> parameter types defined in the vector builtin function.
> ---
> .../gcc.target/loongarch/vector/simd_correctness_check.h | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/testsuite/gcc.target/loongarch/vector/simd_correctness_check.h b/gcc/testsuite/gcc.target/loongarch/vector/simd_correctness_check.h
> index eb7fbd59cc7..f780f6586b3 100644
> --- a/gcc/testsuite/gcc.target/loongarch/vector/simd_correctness_check.h
> +++ b/gcc/testsuite/gcc.target/loongarch/vector/simd_correctness_check.h
> @@ -8,7 +8,8 @@
> int fail = 0; \
> for (size_t i = 0; i < sizeof (res) / sizeof (res[0]); ++i) \
> { \
> - long *temp_ref = &ref[i], *temp_res = &res[i]; \
> + long long *temp_ref = (long long *)&ref[i], \
> + *temp_res = (long long *)&res[i]; \
> if (abs (*temp_ref - *temp_res) > 0) \
> { \
> printf (" error: %s at line %ld , expected " #ref \
> @@ -28,7 +29,7 @@
> int fail = 0; \
> for (size_t i = 0; i < sizeof (res) / sizeof (res[0]); ++i) \
> { \
> - int *temp_ref = &ref[i], *temp_res = &res[i]; \
> + int *temp_ref = (int *)&ref[i], *temp_res = (int *)&res[i]; \
> if (abs (*temp_ref - *temp_res) > 0) \
> { \
> printf (" error: %s at line %ld , expected " #ref \
On Mon, 2023-12-04 at 20:31 +0800, Xi Ruoyao wrote:
> On Mon, 2023-12-04 at 20:14 +0800, chenxiaolong wrote:
> > On LoongArch architecture, using the latest gcc14 in regression test,
> > it is found that the vector test cases in vector directory appear FAIL
> > entries with unmatched pointer types. In order to solve this kind of
> > problem, the type of the variable in the check result is modified with
> > the parameter type defined in the vector builtin function.
>
> IMO we should write something more readable:
>
> static inline
> void ASSERTEQ_64 (int line, const void *_ref, const void *_res)
> {
> if (memcmp (_ref, _res, 16) == 0)
> return;
>
> const char *ref = (const char *)_ref;
> const char *res = (const char *)_res;
>
> printf ("error %s:%d: result mismatch\n", __FILE__, line);
>
> printf ("ref:");
> for (int i = 0; i < 16; i++)
> printf (" %02x", ref[i]);
Sorry, should be " %02hhx" here.
>
> printf ("\nres:");
> for (int i = 0; i < 16; i++)
> printf (" %02x", res[i]);
Likewise.
> putchar ('\n');
> abort ();
> }
在 2023-12-04一的 20:38 +0800，Xi Ruoyao写道：
> On Mon, 2023-12-04 at 20:31 +0800, Xi Ruoyao wrote:
> > On Mon, 2023-12-04 at 20:14 +0800, chenxiaolong wrote:
> > > On LoongArch architecture, using the latest gcc14 in regression
> > > test,
> > > it is found that the vector test cases in vector directory appear
> > > FAIL
> > > entries with unmatched pointer types. In order to solve this kind
> > > of
> > > problem, the type of the variable in the check result is modified
> > > with
> > > the parameter type defined in the vector builtin function.
> >
> > IMO we should write something more readable:
> >
> > static inline
> > void ASSERTEQ_64 (int line, const void *_ref, const void *_res)
> > {
> > if (memcmp (_ref, _res, 16) == 0)
> > return;
> >
> > const char *ref = (const char *)_ref;
> > const char *res = (const char *)_res;
> >
> > printf ("error %s:%d: result mismatch\n", __FILE__, line);
> >
> > printf ("ref:");
> > for (int i = 0; i < 16; i++)
> > printf (" %02x", ref[i]);
>
> Sorry, should be " %02hhx" here.
>
> > printf ("\nres:");
> > for (int i = 0; i < 16; i++)
> > printf (" %02x", res[i]);
>
> Likewise.
>
> > putchar ('\n');
> > abort ();
> > }
According to your suggestion, the check of the built-in function was modifiedin the simd_correctness_check.h file, and the types of the actual parameters
of the built-in function were inconsistent with those of the formal parameters.
The problems with the GCC regression test are as follows:
...
note: expected 'const void *' but argument is of type '__m128i'
error: incompatible type for argument 3 of 'ASSERTEQ_64'
...
The reason is that the types used in __m{128i,128,128d} are defined in
the vector header file (lsxintrin.h or lasxintrin.h), and their basic
types do not match the parameter types corresponding to the functions.
On Tue, 2023-12-05 at 17:21 +0800, chenxiaolong wrote:
> According to your suggestion, the check of the built-in function was modifiedin the simd_correctness_check.h file, and the types of the actual parameters
> of the built-in function were inconsistent with those of the formal parameters.
> The problems with the GCC regression test are as follows:
>
> ...
> note: expected 'const void *' but argument is of type '__m128i'
> error: incompatible type for argument 3 of 'ASSERTEQ_64'
> ...
>
> The reason is that the types used in __m{128i,128,128d} are defined in
> the vector header file (lsxintrin.h or lasxintrin.h), and their basic
> types do not match the parameter types corresponding to the functions.
Ouch. I forgot that we are passing vectors themselves to ASSERTEQ_64,
not the pointers.
Now I come up with this:
#include <cstdio>
#include <cstring>
#include <cstdlib>
static inline void
dump (const void *_ptr, int size, const char *name)
{
const char *ptr = (const char *)_ptr;
printf("%s:", name);
for (int i = 0; i < size; i++)
printf(" %02hhx", ptr[i]);
putchar('\n');
}
template <class U, class V>
static inline void
assert_eq (const U &res, const V &ref, int line)
{
static_assert(sizeof (res) == sizeof (ref));
if (!memcmp (&res, &ref, sizeof(ref)))
return;
dump (res, sizeof (res), "res");
dump (ref, sizeof (ref), "ref");
}
int main()
{
float x[4] = {};
int y[4] = {};
assert_eq(x, y, __LINE__);
}
This is C++, not C. But IMO we can port the tests to C++ anyway.
在 2023-12-05二的 20:44 +0800，Xi Ruoyao写道：
> On Tue, 2023-12-05 at 17:21 +0800, chenxiaolong wrote:
> > According to your suggestion, the check of the built-in function
> > was modifiedin the simd_correctness_check.h file, and the types of
> > the actual parameters
> > of the built-in function were inconsistent with those of the formal
> > parameters.
> > The problems with the GCC regression test are as follows:
> >
> > ...
> > note: expected 'const void *' but argument is of type '__m128i'
> > error: incompatible type for argument 3 of 'ASSERTEQ_64'
> > ...
> >
> > The reason is that the types used in __m{128i,128,128d} are defined
> > in
> > the vector header file (lsxintrin.h or lasxintrin.h), and their
> > basic
> > types do not match the parameter types corresponding to the
> > functions.
>
> Ouch. I forgot that we are passing vectors themselves to
> ASSERTEQ_64,
> not the pointers.
>
> Now I come up with this:
>
> #include <cstdio>
> #include <cstring>
> #include <cstdlib>
>
> static inline void
> dump (const void *_ptr, int size, const char *name)
> {
> const char *ptr = (const char *)_ptr;
>
> printf("%s:", name);
>
> for (int i = 0; i < size; i++)
> printf(" %02hhx", ptr[i]);
>
> putchar('\n');
> }
>
> template <class U, class V>
> static inline void
> assert_eq (const U &res, const V &ref, int line)
> {
> static_assert(sizeof (res) == sizeof (ref));
> if (!memcmp (&res, &ref, sizeof(ref)))
> return;
>
> dump (res, sizeof (res), "res");
> dump (ref, sizeof (ref), "ref");
> }
>
> int main()
> {
> float x[4] = {};
> int y[4] = {};
> assert_eq(x, y, __LINE__);
> }
>
> This is C++, not C. But IMO we can port the tests to C++ anyway.
>
Following your idea, I tried to change C into C++ code. The problem is
that the tests cases of LoongArch architecture are written in the style
of C language, and the code changed to C++ involves more problems and
is not easy to completely modify. So it's best to keep the C language
style.
@@ -8,7 +8,8 @@
int fail = 0; \
for (size_t i = 0; i < sizeof (res) / sizeof (res[0]); ++i) \
{ \
- long *temp_ref = &ref[i], *temp_res = &res[i]; \
+ long long *temp_ref = (long long *)&ref[i], \
+ *temp_res = (long long *)&res[i]; \
if (abs (*temp_ref - *temp_res) > 0) \
{ \
printf (" error: %s at line %ld , expected " #ref \
@@ -28,7 +29,7 @@
int fail = 0; \
for (size_t i = 0; i < sizeof (res) / sizeof (res[0]); ++i) \
{ \
- int *temp_ref = &ref[i], *temp_res = &res[i]; \
+ int *temp_ref = (int *)&ref[i], *temp_res = (int *)&res[i]; \
if (abs (*temp_ref - *temp_res) > 0) \
{ \
printf (" error: %s at line %ld , expected " #ref \