Re: Re: [PATCH] RISC-V: Bugfix for resolve_overloaded_builtin[PR113420]
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]
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]
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