I prefer all vector related function registration should be in the same
function groups.
like aarch64:
/* A list of all SVE ACLE functions. */
static CONSTEXPR const function_group_info function_groups[] = {
#define DEF_SVE_FUNCTION_GS(NAME, SHAPE, TYPES, GROUPS, PREDS) \
{ #NAME, &functions::NAME, &shapes::SHAPE, types_##TYPES, groups_##GROUPS, \
preds_##PREDS, REQUIRED_EXTENSIONS },
#define DEF_SME_ZA_FUNCTION_GS(NAME, SHAPE, TYPES, GROUPS, PREDS) \
{ #NAME, &functions::NAME##_za, &shapes::SHAPE, types_##TYPES, \
groups_##GROUPS, preds_##PREDS, (REQUIRED_EXTENSIONS | AARCH64_FL_ZA_ON) },
#include "aarch64-sve-builtins.def"
};
all SVE and SME are in the same function_groups.
Come back to your previous commit:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=ce7e66787b5b4ad385b21756da5a89171d233ddc
I think you should send another separate patch to tweak current
DEF_RVV_FUNCTION:
1. +#define DEF_RVV_FUNCTION(NAME, SHAPE, PREDS, OPS_INFO, AVAIL)
Recover it back to -#define DEF_RVV_FUNCTION(NAME, SHAPE, PREDS, OPS_INFO)
2. Recover it :
-#define DEF_RVV_FUNCTION(NAME, SHAPE, PREDS, OPS_INFO)
\
- {#NAME, &bases::NAME, &shapes::SHAPE, PREDS, OPS_INFO},
+#define DEF_RVV_FUNCTION(NAME, SHAPE, PREDS, OPS_INFO, ...)
\
+ {#NAME, &bases::NAME, &shapes::SHAPE, PREDS, OPS_INFO,\
+ __VA_ARGS__},
into:
static function_group_info function_groups[] = {
+#define DEF_RVV_FUNCTION(NAME, SHAPE, PREDS, OPS_INFO)
\
+ {#NAME, &bases::NAME, &shapes::SHAPE, PREDS, OPS_INFO},
3. Recover all DEF_RVV_FUNCTION back to the original.
4. In the following vector crypto intrinsic, you should add like
DEF_RVV_CRYPTO_FUNCTION like aarch64 does for SME.
juzhe.zh...@rivai.ai
From: Feng Wang
Date: 2023-12-13 21:00
To: juzhe.zh...@rivai.ai; gcc-patches
CC: kito.cheng; Jeff Law; zhusonghe; panciyan
Subject: Re: Re: [PATCH v3 2/4] RISC-V: Add crypto vector builtin function.
2023-12-13 18:18 juzhe.zhong <juzhe.zh...@rivai.ai> wrote:
>
>
>+ multiple_p (GET_MODE_BITSIZE (e.arg_mode (0)),
>+ GET_MODE_BITSIZE (e.arg_mode (1)), &nunits);
>
>Change it into gcc_assert (multiple_p (...))
>
>+/* A list of all Vector Crypto intrinsic functions. */
>+static function_group_info cryoto_function_groups[] = {
>+#define DEF_RVV_FUNCTION(NAME, SHAPE, PREDS, OPS_INFO, AVAIL) \
>+ {#NAME, &bases::NAME, &shapes::SHAPE, PREDS, OPS_INFO,\
>+ riscv_vector_avail_ ## AVAIL},
>+#include "riscv-vector-crypto-builtins-functions.def"
>+};
>Why do you add this ? I think it should belong to function_groups.
The original intention of this modification was to make the processing flow of
the crypto vector more clearer.
If you think it should merge into V extension, I will do it.
Thanks.
Feng Wang
>
>+ /* Dfine the crypto vector builtin functions. */
>+ for (unsigned int i = 0; i < ARRAY_SIZE (cryoto_function_groups); ++i)
>+ {
>+ function_group_info *f = &cryoto_function_groups[i];
>+ if (f->avail && f->avail ())
>+ builder.register_function_group (cryoto_function_groups[i]);
>+ }
>
>
>I think it should be:
>
>for (unsigned int i = 0; i < ARRAY_SIZE (function_groups); ++i)
> if (avail)
> builder.register_function_group (function_groups[i]);
>
>
>
>
>juzhe.zh...@rivai.ai
>
