* [PATCH v1] LoongArch: Modify the check type of the vector builtin function.
@ 2023-12-04 12:14 chenxiaolong
2023-12-04 12:31 ` Xi Ruoyao
0 siblings, 1 reply; 6+ messages in thread
From: chenxiaolong @ 2023-12-04 12:14 UTC (permalink / raw)
To: gcc-patches; +Cc: xry111, i, xuchenghua, chenglulu, chenxiaolong
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(-)
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 \
--
2.20.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] LoongArch: Modify the check type of the vector builtin function.
2023-12-04 12:14 [PATCH v1] LoongArch: Modify the check type of the vector builtin function chenxiaolong
@ 2023-12-04 12:31 ` Xi Ruoyao
2023-12-04 12:38 ` Xi Ruoyao
0 siblings, 1 reply; 6+ messages in thread
From: Xi Ruoyao @ 2023-12-04 12:31 UTC (permalink / raw)
To: chenxiaolong, gcc-patches; +Cc: i, xuchenghua, chenglulu
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 \
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] LoongArch: Modify the check type of the vector builtin function.
2023-12-04 12:31 ` Xi Ruoyao
@ 2023-12-04 12:38 ` Xi Ruoyao
2023-12-05 9:21 ` chenxiaolong
0 siblings, 1 reply; 6+ messages in thread
From: Xi Ruoyao @ 2023-12-04 12:38 UTC (permalink / raw)
To: chenxiaolong, gcc-patches; +Cc: i, xuchenghua, chenglulu
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 ();
> }
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] LoongArch: Modify the check type of the vector builtin function.
2023-12-04 12:38 ` Xi Ruoyao
@ 2023-12-05 9:21 ` chenxiaolong
2023-12-05 12:44 ` Xi Ruoyao
0 siblings, 1 reply; 6+ messages in thread
From: chenxiaolong @ 2023-12-05 9:21 UTC (permalink / raw)
To: Xi Ruoyao, gcc-patches; +Cc: i, xuchenghua, chenglulu
在 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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] LoongArch: Modify the check type of the vector builtin function.
2023-12-05 9:21 ` chenxiaolong
@ 2023-12-05 12:44 ` Xi Ruoyao
2023-12-07 3:21 ` chenxiaolong
0 siblings, 1 reply; 6+ messages in thread
From: Xi Ruoyao @ 2023-12-05 12:44 UTC (permalink / raw)
To: chenxiaolong, gcc-patches; +Cc: i, xuchenghua, chenglulu
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.
--
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1] LoongArch: Modify the check type of the vector builtin function.
2023-12-05 12:44 ` Xi Ruoyao
@ 2023-12-07 3:21 ` chenxiaolong
0 siblings, 0 replies; 6+ messages in thread
From: chenxiaolong @ 2023-12-07 3:21 UTC (permalink / raw)
To: Xi Ruoyao, gcc-patches; +Cc: i, xuchenghua, chenglulu
在 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-07 3:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-04 12:14 [PATCH v1] LoongArch: Modify the check type of the vector builtin function chenxiaolong
2023-12-04 12:31 ` Xi Ruoyao
2023-12-04 12:38 ` Xi Ruoyao
2023-12-05 9:21 ` chenxiaolong
2023-12-05 12:44 ` Xi Ruoyao
2023-12-07 3:21 ` chenxiaolong
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).