Re: Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]

2024-01-19 Thread juzhe.zh...@rivai.ai
Could you show me the ICE message ?
Is it in front-end ? If yes, it's ok.

I wonder whether it is "internal compiler error".




juzhe.zh...@rivai.ai
 
From: Li Xu
Date: 2024-01-19 16:04
To: juzhe.zhong; gcc-patches
CC: kito.cheng; palmer; zhengyu; pan2.li
Subject: Re: Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
you are right.

vint8mf8_t test_vle8_v_i8mf8_m(vbool64_t vm, const int32_t *rs1, size_t vl) {
  return __riscv_vle8(vm, rs1, vl);
}

This will cause ICE. I tried clang and it will also cause ICE.



xu...@eswincomputing.com
 
From: juzhe.zh...@rivai.ai
Date: 2024-01-19 15:53
To: Li Xu; gcc-patches
CC: kito.cheng; palmer; zhengyu; pan2.li; Li Xu
Subject: Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
Could you add a test for vle with mask?

For example:

__riscv_vle8 which overload __riscv_vle8_v_i8mf8_m and __riscv_vle8_v_u8mf8_m

You are using pointer type and mask type to resolve it.

So this pointer type is expecting const int8_t or const uint8_t.

Could you add test:
1.__riscv_vle8 (const int8_t *...)
2. __riscv_vle8 (const uint8_t *...)
3. __riscv_vle8 (const int32_t *...) ---> I worry this will cause ICE since 
pointer type doesn't match the expecting type,
I wonder whether it will cause ICE while resolving API.

Thanks.




juzhe.zh...@rivai.ai
 
From: Li Xu
Date: 2024-01-19 15:44
To: gcc-patches
CC: kito.cheng; palmer; juzhe.zhong; zhengyu; pan2.li; xuli
Subject: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
From: xuli 
 
Change the hash value of overloaded intrinsic from considering
all parameter types to:
1. Encoding vector data type
2. In order to distinguish vle8_v_i8mf8_m(vbool64_t vm, const int8_t *rs1, 
size_t vl)
   and vle8_v_u8mf8_m(vbool64_t vm, const uint8_t *rs1, size_t vl), encode the 
pointer type
3. In order to distinguish vfadd_vv_f32mf2_rm(vfloat32mf2_t vs2, vfloat32mf2_t 
vs1, size_t vl)
   and vfadd_vv_f32mf2(vfloat32mf2_t vs2, vfloat32mf2_t vs1, size_t vl), encode 
the number of
   parameters. The same goes for the vxrm intrinsics.
 
PR target/113420
 
gcc/ChangeLog:
 
* config/riscv/riscv-vector-builtins.cc (has_vxrm_or_frm_p): remove.
(registered_function::overloaded_hash): refactor.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/base/pr113420.c: New test.
---
gcc/config/riscv/riscv-vector-builtins.cc | 88 +++
.../gcc.target/riscv/rvv/base/pr113420.c  | 30 +++
2 files changed, 43 insertions(+), 75 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
 
diff --git a/gcc/config/riscv/riscv-vector-builtins.cc 
b/gcc/config/riscv/riscv-vector-builtins.cc
index 25e0b6e56de..5240f9e1f02 100644
--- a/gcc/config/riscv/riscv-vector-builtins.cc
+++ b/gcc/config/riscv/riscv-vector-builtins.cc
@@ -4271,24 +4271,22 @@ registered_function::overloaded_hash () const
: TYPE_UNSIGNED (type);
   mode_p = POINTER_TYPE_P (type) ? TYPE_MODE (TREE_TYPE (type))
 : TYPE_MODE (type);
-  h.add_int (unsigned_p);
-  h.add_int (mode_p);
+  if (POINTER_TYPE_P (type) || lookup_vector_type_attribute (type))
+ {
+   h.add_int (unsigned_p);
+   h.add_int (mode_p);
+ }
+  else if (instance.base->may_require_vxrm_p ()
+|| instance.base->may_require_frm_p ())
+ {
+   h.add_int (argument_types.length ());
+   break;
+ }
 }
   return h.end ();
}
-bool
-has_vxrm_or_frm_p (function_instance , const vec 
)
-{
-  if (instance.base->may_require_vxrm_p ()
-  || (instance.base->may_require_frm_p ()
-   && (TREE_CODE (TREE_TYPE (arglist[arglist.length () - 2]))
-   == INTEGER_TYPE)))
-return true;
-  return false;
-}
-
hashval_t
registered_function::overloaded_hash (const vec )
{
@@ -4296,68 +4294,8 @@ registered_function::overloaded_hash (const vec )
   unsigned int len = arglist.length ();
   for (unsigned int i = 0; i < len; i++)
-{
-  /* vint8m1_t __riscv_vget_i8m1(vint8m2_t src, size_t index);
-  When the user calls vget intrinsic, the __riscv_vget_i8m1(src, 1)
-   form is used. The compiler recognizes that the parameter index is signed
-   int, which is inconsistent with size_t, so the index is converted to
-   size_t type in order to get correct hash value. vint8m2_t
-   __riscv_vset(vint8m2_t dest, size_t index, vint8m1_t value); The reason
-   is the same as above. */
-  if ((instance.base == bases::vget && (i == (len - 1)))
-   || ((instance.base == bases::vset
-   || instance.shape == shapes::crypto_vi)
- && (i == (len - 2
- argument_types.safe_push (size_type_node);
-  /* Vector fixed-point arithmetic instructions requiring argument vxrm.
-  For example: vuint32m4_t __riscv_vaaddu(vuint32m4_t vs2,
-  vuint32m4_t vs1, unsigned int vxrm, size_t vl); The user calls vaaddu
-  intrinsic in the form of __riscv_vaaddu(vs2, vs1, 2, vl). The compiler
-  recognizes that the p

Re: Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]

2024-01-19 Thread Li Xu
you are right.

vint8mf8_t test_vle8_v_i8mf8_m(vbool64_t vm, const int32_t *rs1, size_t vl) {
  return __riscv_vle8(vm, rs1, vl);
}

This will cause ICE. I tried clang and it will also cause ICE.



xu...@eswincomputing.com
 
From: juzhe.zh...@rivai.ai
Date: 2024-01-19 15:53
To: Li Xu; gcc-patches
CC: kito.cheng; palmer; zhengyu; pan2.li; Li Xu
Subject: Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
Could you add a test for vle with mask?

For example:

__riscv_vle8 which overload __riscv_vle8_v_i8mf8_m and __riscv_vle8_v_u8mf8_m

You are using pointer type and mask type to resolve it.

So this pointer type is expecting const int8_t or const uint8_t.

Could you add test:
1.__riscv_vle8 (const int8_t *...)
2. __riscv_vle8 (const uint8_t *...)
3. __riscv_vle8 (const int32_t *...) ---> I worry this will cause ICE since 
pointer type doesn't match the expecting type,
I wonder whether it will cause ICE while resolving API.

Thanks.




juzhe.zh...@rivai.ai
 
From: Li Xu
Date: 2024-01-19 15:44
To: gcc-patches
CC: kito.cheng; palmer; juzhe.zhong; zhengyu; pan2.li; xuli
Subject: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
From: xuli 
 
Change the hash value of overloaded intrinsic from considering
all parameter types to:
1. Encoding vector data type
2. In order to distinguish vle8_v_i8mf8_m(vbool64_t vm, const int8_t *rs1, 
size_t vl)
   and vle8_v_u8mf8_m(vbool64_t vm, const uint8_t *rs1, size_t vl), encode the 
pointer type
3. In order to distinguish vfadd_vv_f32mf2_rm(vfloat32mf2_t vs2, vfloat32mf2_t 
vs1, size_t vl)
   and vfadd_vv_f32mf2(vfloat32mf2_t vs2, vfloat32mf2_t vs1, size_t vl), encode 
the number of
   parameters. The same goes for the vxrm intrinsics.
 
PR target/113420
 
gcc/ChangeLog:
 
* config/riscv/riscv-vector-builtins.cc (has_vxrm_or_frm_p): remove.
(registered_function::overloaded_hash): refactor.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/base/pr113420.c: New test.
---
gcc/config/riscv/riscv-vector-builtins.cc | 88 +++
.../gcc.target/riscv/rvv/base/pr113420.c  | 30 +++
2 files changed, 43 insertions(+), 75 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
 
diff --git a/gcc/config/riscv/riscv-vector-builtins.cc 
b/gcc/config/riscv/riscv-vector-builtins.cc
index 25e0b6e56de..5240f9e1f02 100644
--- a/gcc/config/riscv/riscv-vector-builtins.cc
+++ b/gcc/config/riscv/riscv-vector-builtins.cc
@@ -4271,24 +4271,22 @@ registered_function::overloaded_hash () const
: TYPE_UNSIGNED (type);
   mode_p = POINTER_TYPE_P (type) ? TYPE_MODE (TREE_TYPE (type))
 : TYPE_MODE (type);
-  h.add_int (unsigned_p);
-  h.add_int (mode_p);
+  if (POINTER_TYPE_P (type) || lookup_vector_type_attribute (type))
+ {
+   h.add_int (unsigned_p);
+   h.add_int (mode_p);
+ }
+  else if (instance.base->may_require_vxrm_p ()
+|| instance.base->may_require_frm_p ())
+ {
+   h.add_int (argument_types.length ());
+   break;
+ }
 }
   return h.end ();
}
-bool
-has_vxrm_or_frm_p (function_instance , const vec 
)
-{
-  if (instance.base->may_require_vxrm_p ()
-  || (instance.base->may_require_frm_p ()
-   && (TREE_CODE (TREE_TYPE (arglist[arglist.length () - 2]))
-   == INTEGER_TYPE)))
-return true;
-  return false;
-}
-
hashval_t
registered_function::overloaded_hash (const vec )
{
@@ -4296,68 +4294,8 @@ registered_function::overloaded_hash (const vec )
   unsigned int len = arglist.length ();
   for (unsigned int i = 0; i < len; i++)
-{
-  /* vint8m1_t __riscv_vget_i8m1(vint8m2_t src, size_t index);
-  When the user calls vget intrinsic, the __riscv_vget_i8m1(src, 1)
-   form is used. The compiler recognizes that the parameter index is signed
-   int, which is inconsistent with size_t, so the index is converted to
-   size_t type in order to get correct hash value. vint8m2_t
-   __riscv_vset(vint8m2_t dest, size_t index, vint8m1_t value); The reason
-   is the same as above. */
-  if ((instance.base == bases::vget && (i == (len - 1)))
-   || ((instance.base == bases::vset
-   || instance.shape == shapes::crypto_vi)
- && (i == (len - 2
- argument_types.safe_push (size_type_node);
-  /* Vector fixed-point arithmetic instructions requiring argument vxrm.
-  For example: vuint32m4_t __riscv_vaaddu(vuint32m4_t vs2,
-  vuint32m4_t vs1, unsigned int vxrm, size_t vl); The user calls vaaddu
-  intrinsic in the form of __riscv_vaaddu(vs2, vs1, 2, vl). The compiler
-  recognizes that the parameter vxrm is a signed int, which is inconsistent
-  with the parameter unsigned int vxrm declared by intrinsic, so the
-  parameter vxrm is converted to an unsigned int type in order to get
-  correct hash value.
-
-  Vector Floating-Point Instructions requiring argument frm.
-  DEF_RVV_FUNCTION (vfadd, alu, full_preds, f_v

Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]

2024-01-18 Thread juzhe.zh...@rivai.ai
Could you add a test for vle with mask?

For example:

__riscv_vle8 which overload __riscv_vle8_v_i8mf8_m and __riscv_vle8_v_u8mf8_m

You are using pointer type and mask type to resolve it.

So this pointer type is expecting const int8_t or const uint8_t.

Could you add test:
1.__riscv_vle8 (const int8_t *...)
2. __riscv_vle8 (const uint8_t *...)
3. __riscv_vle8 (const int32_t *...) ---> I worry this will cause ICE since 
pointer type doesn't match the expecting type,
I wonder whether it will cause ICE while resolving API.

Thanks.




juzhe.zh...@rivai.ai
 
From: Li Xu
Date: 2024-01-19 15:44
To: gcc-patches
CC: kito.cheng; palmer; juzhe.zhong; zhengyu; pan2.li; xuli
Subject: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
From: xuli 
 
Change the hash value of overloaded intrinsic from considering
all parameter types to:
1. Encoding vector data type
2. In order to distinguish vle8_v_i8mf8_m(vbool64_t vm, const int8_t *rs1, 
size_t vl)
   and vle8_v_u8mf8_m(vbool64_t vm, const uint8_t *rs1, size_t vl), encode the 
pointer type
3. In order to distinguish vfadd_vv_f32mf2_rm(vfloat32mf2_t vs2, vfloat32mf2_t 
vs1, size_t vl)
   and vfadd_vv_f32mf2(vfloat32mf2_t vs2, vfloat32mf2_t vs1, size_t vl), encode 
the number of
   parameters. The same goes for the vxrm intrinsics.
 
PR target/113420
 
gcc/ChangeLog:
 
* config/riscv/riscv-vector-builtins.cc (has_vxrm_or_frm_p): remove.
(registered_function::overloaded_hash): refactor.
 
gcc/testsuite/ChangeLog:
 
* gcc.target/riscv/rvv/base/pr113420.c: New test.
---
gcc/config/riscv/riscv-vector-builtins.cc | 88 +++
.../gcc.target/riscv/rvv/base/pr113420.c  | 30 +++
2 files changed, 43 insertions(+), 75 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
 
diff --git a/gcc/config/riscv/riscv-vector-builtins.cc 
b/gcc/config/riscv/riscv-vector-builtins.cc
index 25e0b6e56de..5240f9e1f02 100644
--- a/gcc/config/riscv/riscv-vector-builtins.cc
+++ b/gcc/config/riscv/riscv-vector-builtins.cc
@@ -4271,24 +4271,22 @@ registered_function::overloaded_hash () const
: TYPE_UNSIGNED (type);
   mode_p = POINTER_TYPE_P (type) ? TYPE_MODE (TREE_TYPE (type))
 : TYPE_MODE (type);
-  h.add_int (unsigned_p);
-  h.add_int (mode_p);
+  if (POINTER_TYPE_P (type) || lookup_vector_type_attribute (type))
+ {
+   h.add_int (unsigned_p);
+   h.add_int (mode_p);
+ }
+  else if (instance.base->may_require_vxrm_p ()
+|| instance.base->may_require_frm_p ())
+ {
+   h.add_int (argument_types.length ());
+   break;
+ }
 }
   return h.end ();
}
-bool
-has_vxrm_or_frm_p (function_instance , const vec 
)
-{
-  if (instance.base->may_require_vxrm_p ()
-  || (instance.base->may_require_frm_p ()
-   && (TREE_CODE (TREE_TYPE (arglist[arglist.length () - 2]))
-   == INTEGER_TYPE)))
-return true;
-  return false;
-}
-
hashval_t
registered_function::overloaded_hash (const vec )
{
@@ -4296,68 +4294,8 @@ registered_function::overloaded_hash (const vec )
   unsigned int len = arglist.length ();
   for (unsigned int i = 0; i < len; i++)
-{
-  /* vint8m1_t __riscv_vget_i8m1(vint8m2_t src, size_t index);
-  When the user calls vget intrinsic, the __riscv_vget_i8m1(src, 1)
-   form is used. The compiler recognizes that the parameter index is signed
-   int, which is inconsistent with size_t, so the index is converted to
-   size_t type in order to get correct hash value. vint8m2_t
-   __riscv_vset(vint8m2_t dest, size_t index, vint8m1_t value); The reason
-   is the same as above. */
-  if ((instance.base == bases::vget && (i == (len - 1)))
-   || ((instance.base == bases::vset
-   || instance.shape == shapes::crypto_vi)
- && (i == (len - 2
- argument_types.safe_push (size_type_node);
-  /* Vector fixed-point arithmetic instructions requiring argument vxrm.
-  For example: vuint32m4_t __riscv_vaaddu(vuint32m4_t vs2,
-  vuint32m4_t vs1, unsigned int vxrm, size_t vl); The user calls vaaddu
-  intrinsic in the form of __riscv_vaaddu(vs2, vs1, 2, vl). The compiler
-  recognizes that the parameter vxrm is a signed int, which is inconsistent
-  with the parameter unsigned int vxrm declared by intrinsic, so the
-  parameter vxrm is converted to an unsigned int type in order to get
-  correct hash value.
-
-  Vector Floating-Point Instructions requiring argument frm.
-  DEF_RVV_FUNCTION (vfadd, alu, full_preds, f_vvv_ops)
-  DEF_RVV_FUNCTION (vfadd_frm, alu_frm, full_preds, f_vvv_ops)
-  Taking vfadd as an example, theoretically we can add base or shape to the
-  hash value to distinguish whether the frm parameter is required.
-  vfloat32m1_t __riscv_vfadd(vfloat32m1_t vs2, float32_t rs1, size_t vl);
-  vfloat32m1_t __riscv_vfadd(vfloat32m1_t vs2, vfloat32m1_t vs1, unsigned 
int
-  frm, size_t vl);
-
- However, the current registration mechanism