public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
@ 2024-01-19  7:44 Li Xu
  2024-01-19  7:53 ` juzhe.zhong
  0 siblings, 1 reply; 4+ messages in thread
From: Li Xu @ 2024-01-19  7:44 UTC (permalink / raw)
  To: gcc-patches; +Cc: kito.cheng, palmer, juzhe.zhong, zhengyu, pan2.li, xuli

From: xuli <xuli1@eswincomputing.com>

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 &instance, const vec<tree, va_gc> &arglist)
-{
-  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<tree, va_gc> &arglist)
 {
@@ -4296,68 +4294,8 @@ registered_function::overloaded_hash (const vec<tree, va_gc> &arglist)
   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 of overloaded intinsic for gcc
-      limits the intrinsic obtained by entering the hook to always be vfadd, not
-      vfadd_frm. Therefore, the correct hash value cannot be obtained through the
-      parameter list and overload name, base or shape.
-      +--------+---------------------------+-------------------+
-      | index  | name                      | kind              |
-      +--------+---------------------------+-------------------+
-      | 124733 | __riscv_vfadd             | Overloaded        | <- Hook fun code
-      +--------+---------------------------+-------------------+
-      | 124735 |  __riscv_vfadd_vv_f32m1   | Non-overloaded    |
-      +--------+---------------------------+-------------------+
-      | 124737 | __riscv_vfadd             | Placeholder       |
-      +--------+---------------------------+-------------------+
-      | ...                                                    |
-      +--------+---------------------------+-------------------+
-      | ...                                                    |
-      +--------+---------------------------+-------------------+
-      | 125739 | __riscv_vfadd             | Overloaded        |
-      +--------+---------------------------+-------------------+
-      | 125741 | __riscv_vfadd_vv_f32m1_rm | Non-overloaded    |
-      +--------+---------------------------+-------------------+
-      | 125743 | __riscv_vfadd             | Placeholder       |
-      +--------+---------------------------+-------------------+
-
-	    Therefore, the hash value cannot be added with base or shape, and needs
-      to be distinguished by whether the penultimate parameter is INTEGER_TYPE. */
-      else if (has_vxrm_or_frm_p (instance, arglist) && (i == (len - 2)))
-	argument_types.safe_push (unsigned_type_node);
-      else
-	argument_types.safe_push (TREE_TYPE (arglist[i]));
-    }
+    argument_types.safe_push (TREE_TYPE (arglist[i]));
+
   return overloaded_hash ();
 }
 
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
new file mode 100644
index 00000000000..d17f22804ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
+
+#include "riscv_vector.h"
+
+void
+matrix_transpose_intrinsics (float *dst, float *src, size_t n)
+{
+  for (size_t row_id = 0; row_id < n; ++row_id)
+    { // input row-index
+      size_t avl = n;
+      // source pointer to row_id-th row
+      float *row_src = src + row_id * n;
+      // destination pointer to row_id-th column
+      float *row_dst = dst + row_id;
+      while (avl > 0)
+	{
+	  size_t vl = __riscv_vsetvl_e32m1 (avl);
+	  vfloat32m1_t row = __riscv_vle32_v_f32m1 (row_src, vl);
+	  __riscv_vsse32 (row_dst, sizeof (float) * n, row, vl);
+	  // updating application vector length
+	  avl -= vl;
+	  // updating source and destination pointers
+	  row_src += vl;
+	  row_dst += vl * n;
+	}
+    }
+}
+
+/* { dg-final { scan-assembler-times {vsse32\.v} 1 } } */
-- 
2.17.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
  2024-01-19  7:44 [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420] Li Xu
@ 2024-01-19  7:53 ` juzhe.zhong
  2024-01-19  8:04   ` Li Xu
  0 siblings, 1 reply; 4+ messages in thread
From: juzhe.zhong @ 2024-01-19  7:53 UTC (permalink / raw)
  To: Li Xu, gcc-patches; +Cc: kito.cheng, palmer, zhengyu, pan2.li, Li Xu

[-- Attachment #1: Type: text/plain, Size: 8402 bytes --]

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.zhong@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 <xuli1@eswincomputing.com>
 
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 &instance, const vec<tree, va_gc> &arglist)
-{
-  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<tree, va_gc> &arglist)
{
@@ -4296,68 +4294,8 @@ registered_function::overloaded_hash (const vec<tree, va_gc> &arglist)
   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 of overloaded intinsic for gcc
-      limits the intrinsic obtained by entering the hook to always be vfadd, not
-      vfadd_frm. Therefore, the correct hash value cannot be obtained through the
-      parameter list and overload name, base or shape.
-      +--------+---------------------------+-------------------+
-      | index  | name                      | kind              |
-      +--------+---------------------------+-------------------+
-      | 124733 | __riscv_vfadd             | Overloaded        | <- Hook fun code
-      +--------+---------------------------+-------------------+
-      | 124735 |  __riscv_vfadd_vv_f32m1   | Non-overloaded    |
-      +--------+---------------------------+-------------------+
-      | 124737 | __riscv_vfadd             | Placeholder       |
-      +--------+---------------------------+-------------------+
-      | ...                                                    |
-      +--------+---------------------------+-------------------+
-      | ...                                                    |
-      +--------+---------------------------+-------------------+
-      | 125739 | __riscv_vfadd             | Overloaded        |
-      +--------+---------------------------+-------------------+
-      | 125741 | __riscv_vfadd_vv_f32m1_rm | Non-overloaded    |
-      +--------+---------------------------+-------------------+
-      | 125743 | __riscv_vfadd             | Placeholder       |
-      +--------+---------------------------+-------------------+
-
-     Therefore, the hash value cannot be added with base or shape, and needs
-      to be distinguished by whether the penultimate parameter is INTEGER_TYPE. */
-      else if (has_vxrm_or_frm_p (instance, arglist) && (i == (len - 2)))
- argument_types.safe_push (unsigned_type_node);
-      else
- argument_types.safe_push (TREE_TYPE (arglist[i]));
-    }
+    argument_types.safe_push (TREE_TYPE (arglist[i]));
+
   return overloaded_hash ();
}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
new file mode 100644
index 00000000000..d17f22804ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
+
+#include "riscv_vector.h"
+
+void
+matrix_transpose_intrinsics (float *dst, float *src, size_t n)
+{
+  for (size_t row_id = 0; row_id < n; ++row_id)
+    { // input row-index
+      size_t avl = n;
+      // source pointer to row_id-th row
+      float *row_src = src + row_id * n;
+      // destination pointer to row_id-th column
+      float *row_dst = dst + row_id;
+      while (avl > 0)
+ {
+   size_t vl = __riscv_vsetvl_e32m1 (avl);
+   vfloat32m1_t row = __riscv_vle32_v_f32m1 (row_src, vl);
+   __riscv_vsse32 (row_dst, sizeof (float) * n, row, vl);
+   // updating application vector length
+   avl -= vl;
+   // updating source and destination pointers
+   row_src += vl;
+   row_dst += vl * n;
+ }
+    }
+}
+
+/* { dg-final { scan-assembler-times {vsse32\.v} 1 } } */
-- 
2.17.1
 
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
  2024-01-19  7:53 ` juzhe.zhong
@ 2024-01-19  8:04   ` Li Xu
  2024-01-19  8:06     ` juzhe.zhong
  0 siblings, 1 reply; 4+ messages in thread
From: Li Xu @ 2024-01-19  8:04 UTC (permalink / raw)
  To: juzhe.zhong, gcc-patches; +Cc: kito.cheng, palmer, zhengyu, pan2.li

[-- Attachment #1: Type: text/plain, Size: 8843 bytes --]

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.



xuli1@eswincomputing.com
 
From: juzhe.zhong@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.zhong@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 <xuli1@eswincomputing.com>
 
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 &instance, const vec<tree, va_gc> &arglist)
-{
-  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<tree, va_gc> &arglist)
{
@@ -4296,68 +4294,8 @@ registered_function::overloaded_hash (const vec<tree, va_gc> &arglist)
   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 of overloaded intinsic for gcc
-      limits the intrinsic obtained by entering the hook to always be vfadd, not
-      vfadd_frm. Therefore, the correct hash value cannot be obtained through the
-      parameter list and overload name, base or shape.
-      +--------+---------------------------+-------------------+
-      | index  | name                      | kind              |
-      +--------+---------------------------+-------------------+
-      | 124733 | __riscv_vfadd             | Overloaded        | <- Hook fun code
-      +--------+---------------------------+-------------------+
-      | 124735 |  __riscv_vfadd_vv_f32m1   | Non-overloaded    |
-      +--------+---------------------------+-------------------+
-      | 124737 | __riscv_vfadd             | Placeholder       |
-      +--------+---------------------------+-------------------+
-      | ...                                                    |
-      +--------+---------------------------+-------------------+
-      | ...                                                    |
-      +--------+---------------------------+-------------------+
-      | 125739 | __riscv_vfadd             | Overloaded        |
-      +--------+---------------------------+-------------------+
-      | 125741 | __riscv_vfadd_vv_f32m1_rm | Non-overloaded    |
-      +--------+---------------------------+-------------------+
-      | 125743 | __riscv_vfadd             | Placeholder       |
-      +--------+---------------------------+-------------------+
-
-     Therefore, the hash value cannot be added with base or shape, and needs
-      to be distinguished by whether the penultimate parameter is INTEGER_TYPE. */
-      else if (has_vxrm_or_frm_p (instance, arglist) && (i == (len - 2)))
- argument_types.safe_push (unsigned_type_node);
-      else
- argument_types.safe_push (TREE_TYPE (arglist[i]));
-    }
+    argument_types.safe_push (TREE_TYPE (arglist[i]));
+
   return overloaded_hash ();
}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
new file mode 100644
index 00000000000..d17f22804ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
+
+#include "riscv_vector.h"
+
+void
+matrix_transpose_intrinsics (float *dst, float *src, size_t n)
+{
+  for (size_t row_id = 0; row_id < n; ++row_id)
+    { // input row-index
+      size_t avl = n;
+      // source pointer to row_id-th row
+      float *row_src = src + row_id * n;
+      // destination pointer to row_id-th column
+      float *row_dst = dst + row_id;
+      while (avl > 0)
+ {
+   size_t vl = __riscv_vsetvl_e32m1 (avl);
+   vfloat32m1_t row = __riscv_vle32_v_f32m1 (row_src, vl);
+   __riscv_vsse32 (row_dst, sizeof (float) * n, row, vl);
+   // updating application vector length
+   avl -= vl;
+   // updating source and destination pointers
+   row_src += vl;
+   row_dst += vl * n;
+ }
+    }
+}
+
+/* { dg-final { scan-assembler-times {vsse32\.v} 1 } } */
-- 
2.17.1
 
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
  2024-01-19  8:04   ` Li Xu
@ 2024-01-19  8:06     ` juzhe.zhong
  0 siblings, 0 replies; 4+ messages in thread
From: juzhe.zhong @ 2024-01-19  8:06 UTC (permalink / raw)
  To: Li Xu, gcc-patches; +Cc: kito.cheng, palmer, zhengyu, pan2.li

[-- Attachment #1: Type: text/plain, Size: 9196 bytes --]

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.zhong@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.



xuli1@eswincomputing.com
 
From: juzhe.zhong@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.zhong@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 <xuli1@eswincomputing.com>
 
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 &instance, const vec<tree, va_gc> &arglist)
-{
-  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<tree, va_gc> &arglist)
{
@@ -4296,68 +4294,8 @@ registered_function::overloaded_hash (const vec<tree, va_gc> &arglist)
   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 of overloaded intinsic for gcc
-      limits the intrinsic obtained by entering the hook to always be vfadd, not
-      vfadd_frm. Therefore, the correct hash value cannot be obtained through the
-      parameter list and overload name, base or shape.
-      +--------+---------------------------+-------------------+
-      | index  | name                      | kind              |
-      +--------+---------------------------+-------------------+
-      | 124733 | __riscv_vfadd             | Overloaded        | <- Hook fun code
-      +--------+---------------------------+-------------------+
-      | 124735 |  __riscv_vfadd_vv_f32m1   | Non-overloaded    |
-      +--------+---------------------------+-------------------+
-      | 124737 | __riscv_vfadd             | Placeholder       |
-      +--------+---------------------------+-------------------+
-      | ...                                                    |
-      +--------+---------------------------+-------------------+
-      | ...                                                    |
-      +--------+---------------------------+-------------------+
-      | 125739 | __riscv_vfadd             | Overloaded        |
-      +--------+---------------------------+-------------------+
-      | 125741 | __riscv_vfadd_vv_f32m1_rm | Non-overloaded    |
-      +--------+---------------------------+-------------------+
-      | 125743 | __riscv_vfadd             | Placeholder       |
-      +--------+---------------------------+-------------------+
-
-     Therefore, the hash value cannot be added with base or shape, and needs
-      to be distinguished by whether the penultimate parameter is INTEGER_TYPE. */
-      else if (has_vxrm_or_frm_p (instance, arglist) && (i == (len - 2)))
- argument_types.safe_push (unsigned_type_node);
-      else
- argument_types.safe_push (TREE_TYPE (arglist[i]));
-    }
+    argument_types.safe_push (TREE_TYPE (arglist[i]));
+
   return overloaded_hash ();
}
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c b/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
new file mode 100644
index 00000000000..d17f22804ff
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr113420.c
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3" } */
+
+#include "riscv_vector.h"
+
+void
+matrix_transpose_intrinsics (float *dst, float *src, size_t n)
+{
+  for (size_t row_id = 0; row_id < n; ++row_id)
+    { // input row-index
+      size_t avl = n;
+      // source pointer to row_id-th row
+      float *row_src = src + row_id * n;
+      // destination pointer to row_id-th column
+      float *row_dst = dst + row_id;
+      while (avl > 0)
+ {
+   size_t vl = __riscv_vsetvl_e32m1 (avl);
+   vfloat32m1_t row = __riscv_vle32_v_f32m1 (row_src, vl);
+   __riscv_vsse32 (row_dst, sizeof (float) * n, row, vl);
+   // updating application vector length
+   avl -= vl;
+   // updating source and destination pointers
+   row_src += vl;
+   row_dst += vl * n;
+ }
+    }
+}
+
+/* { dg-final { scan-assembler-times {vsse32\.v} 1 } } */
-- 
2.17.1
 
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-01-19  8:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-19  7:44 [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420] Li Xu
2024-01-19  7:53 ` juzhe.zhong
2024-01-19  8:04   ` Li Xu
2024-01-19  8:06     ` juzhe.zhong

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).