* [PATCH] bpf: Use enum for resolved overloaded builtins
@ 2022-11-07 22:48 David Faust
2022-11-08 8:02 ` Jose E. Marchesi
0 siblings, 1 reply; 2+ messages in thread
From: David Faust @ 2022-11-07 22:48 UTC (permalink / raw)
To: gcc-patches; +Cc: jose.marchesi
Change several places in the eBPF backend dealing with overloaded
built-in functions to consistently use the enum bpf_builtins type,
rather than variously using integer constants or booleans. The result is
eaiser to read and extend.
Tested on bpf-unknown-none, no known regressions.
OK to push?
Thanks.
gcc/
* config/bpf/bpf.cc (struct core_walk_data): Add field `which'...
(bpf_resolve_overloaded_builtin): ... set it here. Use values of enum
bpf_builtins for error checks.
(bpf_core_walk): Use values of enum bpf_builtins.
(bpf_core_newdecl): Likewise.
(bpf_expand_builtin): Likewise.
---
gcc/config/bpf/bpf.cc | 106 +++++++++++++++++++++++-------------------
1 file changed, 59 insertions(+), 47 deletions(-)
diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
index fd4003c2bfc..16af2412bf6 100644
--- a/gcc/config/bpf/bpf.cc
+++ b/gcc/config/bpf/bpf.cc
@@ -1222,7 +1222,7 @@ bpf_expand_builtin (tree exp, rtx target ATTRIBUTE_UNUSED,
return gen_rtx_REG (ops[0].mode, BPF_R0);
}
- else if (code == -1)
+ else if (code == -BPF_BUILTIN_PRESERVE_ACCESS_INDEX)
{
/* A resolved overloaded __builtin_preserve_access_index. */
tree arg = CALL_EXPR_ARG (exp, 0);
@@ -1249,7 +1249,7 @@ bpf_expand_builtin (tree exp, rtx target ATTRIBUTE_UNUSED,
return expand_normal (arg);
}
- else if (code == -2)
+ else if (code == -BPF_BUILTIN_PRESERVE_FIELD_INFO)
{
/* A resolved overloaded __builtin_preserve_field_info. */
tree src = CALL_EXPR_ARG (exp, 0);
@@ -1444,28 +1444,37 @@ bpf_core_get_index (const tree node)
__builtin_preserve_access_index. */
static tree
-bpf_core_newdecl (tree type, bool is_pai)
+bpf_core_newdecl (tree type, enum bpf_builtins which)
{
tree rettype;
char name[80];
static unsigned long pai_count = 0;
static unsigned long pfi_count = 0;
- if (is_pai)
+ switch (which)
{
- rettype = build_function_type_list (type, type, NULL);
- int len = snprintf (name, sizeof (name), "%s", "__builtin_pai_");
- len = snprintf (name + len, sizeof (name) - len, "%lu", pai_count++);
- }
- else
- {
- rettype = build_function_type_list (unsigned_type_node, type,
- unsigned_type_node, NULL);
- int len = snprintf (name, sizeof (name), "%s", "__builtin_pfi_");
- len = snprintf (name + len, sizeof (name) - len, "%lu", pfi_count++);
+ case BPF_BUILTIN_PRESERVE_ACCESS_INDEX:
+ {
+ rettype = build_function_type_list (type, type, NULL);
+ int len = snprintf (name, sizeof (name), "%s", "__builtin_pai_");
+ len = snprintf (name + len, sizeof (name) - len, "%lu", pai_count++);
+ }
+ break;
+
+ case BPF_BUILTIN_PRESERVE_FIELD_INFO:
+ {
+ rettype = build_function_type_list (unsigned_type_node, type,
+ unsigned_type_node, NULL);
+ int len = snprintf (name, sizeof (name), "%s", "__builtin_pfi_");
+ len = snprintf (name + len, sizeof (name) - len, "%lu", pfi_count++);
+ }
+ break;
+
+ default:
+ gcc_unreachable ();
}
- return add_builtin_function_ext_scope (name, rettype, is_pai ? -1 : -2,
+ return add_builtin_function_ext_scope (name, rettype, -which,
BUILT_IN_MD, NULL, NULL_TREE);
}
@@ -1492,6 +1501,7 @@ bpf_core_is_maybe_aggregate_access (tree expr)
struct core_walk_data {
location_t loc;
+ enum bpf_builtins which;
tree arg;
};
@@ -1501,7 +1511,6 @@ static tree
bpf_core_walk (tree *tp, int *walk_subtrees, void *data)
{
struct core_walk_data *dat = (struct core_walk_data *) data;
- bool is_pai = dat->arg == NULL_TREE;
/* If this is a type, don't do anything. */
if (TYPE_P (*tp))
@@ -1510,19 +1519,21 @@ bpf_core_walk (tree *tp, int *walk_subtrees, void *data)
return NULL_TREE;
}
- /* Build a new function call to a resolved builtin for the desired operation.
- If this is a preserve_field_info call, pass along the argument to the
- resolved builtin call. */
- if (bpf_core_is_maybe_aggregate_access (*tp))
- {
- tree newdecl = bpf_core_newdecl (TREE_TYPE (*tp), is_pai);
- tree newcall;
- if (is_pai)
- newcall = build_call_expr_loc (dat->loc, newdecl, 1, *tp);
- else
- newcall = build_call_expr_loc (dat->loc, newdecl, 2, *tp, dat->arg);
+ /* Build a new function call to a type-resolved temporary builtin for the
+ desired operation, and pass along args as necessary. */
+ tree newdecl = bpf_core_newdecl (TREE_TYPE (*tp), dat->which);
- *tp = newcall;
+ if (dat->which == BPF_BUILTIN_PRESERVE_ACCESS_INDEX)
+ {
+ if (bpf_core_is_maybe_aggregate_access (*tp))
+ {
+ *tp = build_call_expr_loc (dat->loc, newdecl, 1, *tp);
+ *walk_subtrees = 0;
+ }
+ }
+ else
+ {
+ *tp = build_call_expr_loc (dat->loc, newdecl, 2, *tp, dat->arg);
*walk_subtrees = 0;
}
@@ -1572,32 +1583,26 @@ bpf_is_valid_preserve_field_info_arg (tree expr)
/* Implement TARGET_RESOLVE_OVERLOADED_BUILTIN (see gccint manual section
Target Macros::Misc.).
- We use this for the __builtin_preserve_access_index builtin for CO-RE
- support.
+ Used for CO-RE support builtins such as __builtin_preserve_access_index
+ and __builtin_preserve_field_info.
FNDECL is the declaration of the builtin, and ARGLIST is the list of
- arguments passed to it, and is really a vec<tree,_> *.
-
- In this case, the 'operation' implemented by the builtin is a no-op;
- the builtin is just a marker. So, the result is simply the argument. */
+ arguments passed to it, and is really a vec<tree,_> *. */
static tree
bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist)
{
- bool is_pai = DECL_MD_FUNCTION_CODE (fndecl)
- == BPF_BUILTIN_PRESERVE_ACCESS_INDEX;
- bool is_pfi = DECL_MD_FUNCTION_CODE (fndecl)
- == BPF_BUILTIN_PRESERVE_FIELD_INFO;
+ enum bpf_builtins which = (enum bpf_builtins) DECL_MD_FUNCTION_CODE (fndecl);
- if (!is_pai && !is_pfi)
+ if (which < BPF_BUILTIN_PRESERVE_ACCESS_INDEX
+ || which >= BPF_BUILTIN_MAX)
return NULL_TREE;
- /* We only expect one argument, but it may be an arbitrarily-complicated
- statement-expression. */
vec<tree, va_gc> *params = static_cast<vec<tree, va_gc> *> (arglist);
unsigned n_params = params ? params->length() : 0;
- if ((is_pai && n_params != 1) || (is_pfi && n_params != 2))
+ if (!(which == BPF_BUILTIN_PRESERVE_ACCESS_INDEX && n_params == 1)
+ && n_params != 2)
{
error_at (loc, "wrong number of arguments");
return error_mark_node;
@@ -1605,12 +1610,15 @@ bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist)
tree param = (*params)[0];
- /* If not generating BPF_CORE information, preserve_access_index does nothing,
- and simply "resolves to" the argument. */
- if (!TARGET_BPF_CORE && is_pai)
+ /* If not generating BPF_CORE information, preserve_access_index does
+ nothing, and simply "resolves to" the argument. */
+ if (which == BPF_BUILTIN_PRESERVE_ACCESS_INDEX && !TARGET_BPF_CORE)
return param;
- if (is_pfi && !bpf_is_valid_preserve_field_info_arg (param))
+ /* For __builtin_preserve_field_info, enforce that the parameter is exactly a
+ field access and not a more complex expression. */
+ else if (which == BPF_BUILTIN_PRESERVE_FIELD_INFO
+ && !bpf_is_valid_preserve_field_info_arg (param))
{
error_at (EXPR_LOC_OR_LOC (param, loc),
"argument is not a field access");
@@ -1642,7 +1650,11 @@ bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist)
struct core_walk_data data;
data.loc = loc;
- data.arg = is_pai ? NULL_TREE : (*params)[1];
+ data.which = which;
+ if (which == BPF_BUILTIN_PRESERVE_ACCESS_INDEX)
+ data.arg = NULL_TREE;
+ else
+ data.arg = (*params)[1];
walk_tree (¶m, bpf_core_walk, (void *) &data, NULL);
--
2.37.2
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] bpf: Use enum for resolved overloaded builtins
2022-11-07 22:48 [PATCH] bpf: Use enum for resolved overloaded builtins David Faust
@ 2022-11-08 8:02 ` Jose E. Marchesi
0 siblings, 0 replies; 2+ messages in thread
From: Jose E. Marchesi @ 2022-11-08 8:02 UTC (permalink / raw)
To: David Faust; +Cc: gcc-patches
Hi David.
> Change several places in the eBPF backend dealing with overloaded
> built-in functions to consistently use the enum bpf_builtins type,
> rather than variously using integer constants or booleans. The result is
> eaiser to read and extend.
>
> Tested on bpf-unknown-none, no known regressions.
> OK to push?
OK.
Thanks for the patch.
>
> Thanks.
>
> gcc/
>
> * config/bpf/bpf.cc (struct core_walk_data): Add field `which'...
> (bpf_resolve_overloaded_builtin): ... set it here. Use values of enum
> bpf_builtins for error checks.
> (bpf_core_walk): Use values of enum bpf_builtins.
> (bpf_core_newdecl): Likewise.
> (bpf_expand_builtin): Likewise.
> ---
> gcc/config/bpf/bpf.cc | 106 +++++++++++++++++++++++-------------------
> 1 file changed, 59 insertions(+), 47 deletions(-)
>
> diff --git a/gcc/config/bpf/bpf.cc b/gcc/config/bpf/bpf.cc
> index fd4003c2bfc..16af2412bf6 100644
> --- a/gcc/config/bpf/bpf.cc
> +++ b/gcc/config/bpf/bpf.cc
> @@ -1222,7 +1222,7 @@ bpf_expand_builtin (tree exp, rtx target ATTRIBUTE_UNUSED,
> return gen_rtx_REG (ops[0].mode, BPF_R0);
> }
>
> - else if (code == -1)
> + else if (code == -BPF_BUILTIN_PRESERVE_ACCESS_INDEX)
> {
> /* A resolved overloaded __builtin_preserve_access_index. */
> tree arg = CALL_EXPR_ARG (exp, 0);
> @@ -1249,7 +1249,7 @@ bpf_expand_builtin (tree exp, rtx target ATTRIBUTE_UNUSED,
> return expand_normal (arg);
> }
>
> - else if (code == -2)
> + else if (code == -BPF_BUILTIN_PRESERVE_FIELD_INFO)
> {
> /* A resolved overloaded __builtin_preserve_field_info. */
> tree src = CALL_EXPR_ARG (exp, 0);
> @@ -1444,28 +1444,37 @@ bpf_core_get_index (const tree node)
> __builtin_preserve_access_index. */
>
> static tree
> -bpf_core_newdecl (tree type, bool is_pai)
> +bpf_core_newdecl (tree type, enum bpf_builtins which)
> {
> tree rettype;
> char name[80];
> static unsigned long pai_count = 0;
> static unsigned long pfi_count = 0;
>
> - if (is_pai)
> + switch (which)
> {
> - rettype = build_function_type_list (type, type, NULL);
> - int len = snprintf (name, sizeof (name), "%s", "__builtin_pai_");
> - len = snprintf (name + len, sizeof (name) - len, "%lu", pai_count++);
> - }
> - else
> - {
> - rettype = build_function_type_list (unsigned_type_node, type,
> - unsigned_type_node, NULL);
> - int len = snprintf (name, sizeof (name), "%s", "__builtin_pfi_");
> - len = snprintf (name + len, sizeof (name) - len, "%lu", pfi_count++);
> + case BPF_BUILTIN_PRESERVE_ACCESS_INDEX:
> + {
> + rettype = build_function_type_list (type, type, NULL);
> + int len = snprintf (name, sizeof (name), "%s", "__builtin_pai_");
> + len = snprintf (name + len, sizeof (name) - len, "%lu", pai_count++);
> + }
> + break;
> +
> + case BPF_BUILTIN_PRESERVE_FIELD_INFO:
> + {
> + rettype = build_function_type_list (unsigned_type_node, type,
> + unsigned_type_node, NULL);
> + int len = snprintf (name, sizeof (name), "%s", "__builtin_pfi_");
> + len = snprintf (name + len, sizeof (name) - len, "%lu", pfi_count++);
> + }
> + break;
> +
> + default:
> + gcc_unreachable ();
> }
>
> - return add_builtin_function_ext_scope (name, rettype, is_pai ? -1 : -2,
> + return add_builtin_function_ext_scope (name, rettype, -which,
> BUILT_IN_MD, NULL, NULL_TREE);
> }
>
> @@ -1492,6 +1501,7 @@ bpf_core_is_maybe_aggregate_access (tree expr)
>
> struct core_walk_data {
> location_t loc;
> + enum bpf_builtins which;
> tree arg;
> };
>
> @@ -1501,7 +1511,6 @@ static tree
> bpf_core_walk (tree *tp, int *walk_subtrees, void *data)
> {
> struct core_walk_data *dat = (struct core_walk_data *) data;
> - bool is_pai = dat->arg == NULL_TREE;
>
> /* If this is a type, don't do anything. */
> if (TYPE_P (*tp))
> @@ -1510,19 +1519,21 @@ bpf_core_walk (tree *tp, int *walk_subtrees, void *data)
> return NULL_TREE;
> }
>
> - /* Build a new function call to a resolved builtin for the desired operation.
> - If this is a preserve_field_info call, pass along the argument to the
> - resolved builtin call. */
> - if (bpf_core_is_maybe_aggregate_access (*tp))
> - {
> - tree newdecl = bpf_core_newdecl (TREE_TYPE (*tp), is_pai);
> - tree newcall;
> - if (is_pai)
> - newcall = build_call_expr_loc (dat->loc, newdecl, 1, *tp);
> - else
> - newcall = build_call_expr_loc (dat->loc, newdecl, 2, *tp, dat->arg);
> + /* Build a new function call to a type-resolved temporary builtin for the
> + desired operation, and pass along args as necessary. */
> + tree newdecl = bpf_core_newdecl (TREE_TYPE (*tp), dat->which);
>
> - *tp = newcall;
> + if (dat->which == BPF_BUILTIN_PRESERVE_ACCESS_INDEX)
> + {
> + if (bpf_core_is_maybe_aggregate_access (*tp))
> + {
> + *tp = build_call_expr_loc (dat->loc, newdecl, 1, *tp);
> + *walk_subtrees = 0;
> + }
> + }
> + else
> + {
> + *tp = build_call_expr_loc (dat->loc, newdecl, 2, *tp, dat->arg);
> *walk_subtrees = 0;
> }
>
> @@ -1572,32 +1583,26 @@ bpf_is_valid_preserve_field_info_arg (tree expr)
>
> /* Implement TARGET_RESOLVE_OVERLOADED_BUILTIN (see gccint manual section
> Target Macros::Misc.).
> - We use this for the __builtin_preserve_access_index builtin for CO-RE
> - support.
> + Used for CO-RE support builtins such as __builtin_preserve_access_index
> + and __builtin_preserve_field_info.
>
> FNDECL is the declaration of the builtin, and ARGLIST is the list of
> - arguments passed to it, and is really a vec<tree,_> *.
> -
> - In this case, the 'operation' implemented by the builtin is a no-op;
> - the builtin is just a marker. So, the result is simply the argument. */
> + arguments passed to it, and is really a vec<tree,_> *. */
>
> static tree
> bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist)
> {
> - bool is_pai = DECL_MD_FUNCTION_CODE (fndecl)
> - == BPF_BUILTIN_PRESERVE_ACCESS_INDEX;
> - bool is_pfi = DECL_MD_FUNCTION_CODE (fndecl)
> - == BPF_BUILTIN_PRESERVE_FIELD_INFO;
> + enum bpf_builtins which = (enum bpf_builtins) DECL_MD_FUNCTION_CODE (fndecl);
>
> - if (!is_pai && !is_pfi)
> + if (which < BPF_BUILTIN_PRESERVE_ACCESS_INDEX
> + || which >= BPF_BUILTIN_MAX)
> return NULL_TREE;
>
> - /* We only expect one argument, but it may be an arbitrarily-complicated
> - statement-expression. */
> vec<tree, va_gc> *params = static_cast<vec<tree, va_gc> *> (arglist);
> unsigned n_params = params ? params->length() : 0;
>
> - if ((is_pai && n_params != 1) || (is_pfi && n_params != 2))
> + if (!(which == BPF_BUILTIN_PRESERVE_ACCESS_INDEX && n_params == 1)
> + && n_params != 2)
> {
> error_at (loc, "wrong number of arguments");
> return error_mark_node;
> @@ -1605,12 +1610,15 @@ bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist)
>
> tree param = (*params)[0];
>
> - /* If not generating BPF_CORE information, preserve_access_index does nothing,
> - and simply "resolves to" the argument. */
> - if (!TARGET_BPF_CORE && is_pai)
> + /* If not generating BPF_CORE information, preserve_access_index does
> + nothing, and simply "resolves to" the argument. */
> + if (which == BPF_BUILTIN_PRESERVE_ACCESS_INDEX && !TARGET_BPF_CORE)
> return param;
>
> - if (is_pfi && !bpf_is_valid_preserve_field_info_arg (param))
> + /* For __builtin_preserve_field_info, enforce that the parameter is exactly a
> + field access and not a more complex expression. */
> + else if (which == BPF_BUILTIN_PRESERVE_FIELD_INFO
> + && !bpf_is_valid_preserve_field_info_arg (param))
> {
> error_at (EXPR_LOC_OR_LOC (param, loc),
> "argument is not a field access");
> @@ -1642,7 +1650,11 @@ bpf_resolve_overloaded_builtin (location_t loc, tree fndecl, void *arglist)
>
> struct core_walk_data data;
> data.loc = loc;
> - data.arg = is_pai ? NULL_TREE : (*params)[1];
> + data.which = which;
> + if (which == BPF_BUILTIN_PRESERVE_ACCESS_INDEX)
> + data.arg = NULL_TREE;
> + else
> + data.arg = (*params)[1];
>
> walk_tree (¶m, bpf_core_walk, (void *) &data, NULL);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-11-08 7:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-07 22:48 [PATCH] bpf: Use enum for resolved overloaded builtins David Faust
2022-11-08 8:02 ` Jose E. Marchesi
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).