Am Mittwoch, dem 21.08.2024 um 15:24 +0000 schrieb Qing Zhao:
> >
> > But if we changed it to return a void pointer, we could make this
> > a compile-time check:
> >
> > auto ret = __builtin_get_counted_by(__p->FAM);
> >
> > _Generic(ret, void*: (void)0, default: *ret = COUNT);
>
> Is there any benefit to return a void pointer than a SIZE_T pointer for
> the NULL pointer?
Yes! You can test with _Generic (or __builtin_types_compatible_p)
at compile-time based on the type whether you can set *ret to COUNT
or not as in the example above.
So it is not a weird run-time test which needs to be optimized
away.
>
> >
> >
> > >
> > > Yes, I do feel that the approach __builtin_get_counted_by is not very
> > > good.
> > > Maybe it’s better to provide
> > > A. __builtin_set_counted_by
> > > or
> > > B. The unary operator __counted_by(PTR) to return a Lvalue, in this case,
> > > we need a __builtin_has_attribute first to check whether PTR has the
> > > counted_by attribute first.
> >
> > You could potentially do the same __counted_by and test for type void.
> >
> > _Generic(typeof(__counted_by(PTR)), void: (void)0, __counted_by(PTR) =
> > COUNT);
>
> Oh, so, is there any benefit for the unary operator __counted_by(PTR) than
> the current __builtin_get_counted_by?
I don't know. You suggested it ;-)
It probably makes it harder to test the type because you need the
typeof / C2Y Generic combination, but maybe there are other ways
to test.
Martin
>
> Thanks.
>
> Qing
> >
> > Martin
> >
> > >
> > > Any suggestion?
> > >
> > > thanks.
> > >
> > > Qing
> > >
> > >
> > > >
> > > > Richard.
> > > >
> > > > >
> > > > > Qing
> > > > >
> > > > > >
> > > > > > No objection to the patch but I wanted to share my thoughts here.
> > > > > >
> > > > > > Richard.
> > > > > >
> > > > > > > Bootstrapped and regression tested on both X86 and aarch64, no
> > > > > > > issue.
> > > > > > >
> > > > > > > Okay for trunk?
> > > > > > >
> > > > > > > thanks.
> > > > > > >
> > > > > > > Qing.
> > > > > > >
> > > > > > >
> > > > > > > PR c/116016
> > > > > > >
> > > > > > > gcc/c-family/ChangeLog:
> > > > > > >
> > > > > > > * c-common.cc: Add new __builtin_get_counted_by.
> > > > > > > * c-common.h (enum rid): Add RID_BUILTIN_GET_COUNTED_BY.
> > > > > > >
> > > > > > > gcc/c/ChangeLog:
> > > > > > >
> > > > > > > * c-decl.cc (names_builtin_p): Add
> > > > > > > RID_BUILTIN_GET_COUNTED_BY.
> > > > > > > * c-parser.cc (has_counted_by_object): New routine.
> > > > > > > (get_counted_by_ref): New routine.
> > > > > > > (c_parser_postfix_expression): Handle New
> > > > > > > RID_BUILTIN_GET_COUNTED_BY.
> > > > > > >
> > > > > > > gcc/ChangeLog:
> > > > > > >
> > > > > > > * doc/extend.texi: Add documentation for
> > > > > > > __builtin_get_counted_by.
> > > > > > >
> > > > > > > gcc/testsuite/ChangeLog:
> > > > > > >
> > > > > > > * gcc.dg/builtin-get-counted-by-1.c: New test.
> > > > > > > * gcc.dg/builtin-get-counted-by.c: New test.
> > > > > > > ---
> > > > > > > gcc/c-family/c-common.cc | 1 +
> > > > > > > gcc/c-family/c-common.h | 1 +
> > > > > > > gcc/c/c-decl.cc | 1 +
> > > > > > > gcc/c/c-parser.cc | 72 +++++++++++++++
> > > > > > > gcc/doc/extend.texi | 55 +++++++++++
> > > > > > > .../gcc.dg/builtin-get-counted-by-1.c | 91
> > > > > > > +++++++++++++++++++
> > > > > > > gcc/testsuite/gcc.dg/builtin-get-counted-by.c | 54 +++++++++++
> > > > > > > 7 files changed, 275 insertions(+)
> > > > > > > create mode 100644 gcc/testsuite/gcc.dg/builtin-get-counted-by-1.c
> > > > > > > create mode 100644 gcc/testsuite/gcc.dg/builtin-get-counted-by.c
> > > > > > >
> > > > > > > diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> > > > > > > index e7e371fd26f..4b27c6bfeeb 100644
> > > > > > > --- a/gcc/c-family/c-common.cc
> > > > > > > +++ b/gcc/c-family/c-common.cc
> > > > > > > @@ -430,6 +430,7 @@ const struct c_common_resword
> > > > > > > c_common_reswords[] =
> > > > > > > { "__builtin_choose_expr", RID_CHOOSE_EXPR, D_CONLY },
> > > > > > > { "__builtin_complex", RID_BUILTIN_COMPLEX, D_CONLY },
> > > > > > > { "__builtin_convertvector", RID_BUILTIN_CONVERTVECTOR, 0 },
> > > > > > > + { "__builtin_get_counted_by", RID_BUILTIN_GET_COUNTED_BY,
> > > > > > > D_CONLY },
> > > > > > > { "__builtin_has_attribute", RID_BUILTIN_HAS_ATTRIBUTE, 0 },
> > > > > > > { "__builtin_launder", RID_BUILTIN_LAUNDER, D_CXXONLY },
> > > > > > > { "__builtin_shuffle", RID_BUILTIN_SHUFFLE, 0 },
> > > > > > > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> > > > > > > index 2510ee4dbc9..5d5a297012f 100644
> > > > > > > --- a/gcc/c-family/c-common.h
> > > > > > > +++ b/gcc/c-family/c-common.h
> > > > > > > @@ -110,6 +110,7 @@ enum rid
> > > > > > > RID_TYPES_COMPATIBLE_P, RID_BUILTIN_COMPLEX,
> > > > > > > RID_BUILTIN_SHUFFLE,
> > > > > > > RID_BUILTIN_SHUFFLEVECTOR, RID_BUILTIN_CONVERTVECTOR,
> > > > > > > RID_BUILTIN_TGMATH,
> > > > > > > RID_BUILTIN_HAS_ATTRIBUTE, RID_BUILTIN_ASSOC_BARRIER,
> > > > > > > RID_BUILTIN_STDC,
> > > > > > > + RID_BUILTIN_GET_COUNTED_BY,
> > > > > > > RID_DFLOAT32, RID_DFLOAT64, RID_DFLOAT128,
> > > > > > >
> > > > > > > /* TS 18661-3 keywords, in the same sequence as the TI_* values.
> > > > > > > */
> > > > > > > diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
> > > > > > > index 8cef8f2c289..40bc808878b 100644
> > > > > > > --- a/gcc/c/c-decl.cc
> > > > > > > +++ b/gcc/c/c-decl.cc
> > > > > > > @@ -11739,6 +11739,7 @@ names_builtin_p (const char *name)
> > > > > > > case RID_BUILTIN_SHUFFLE:
> > > > > > > case RID_BUILTIN_SHUFFLEVECTOR:
> > > > > > > case RID_BUILTIN_STDC:
> > > > > > > + case RID_BUILTIN_GET_COUNTED_BY:
> > > > > > > case RID_CHOOSE_EXPR:
> > > > > > > case RID_OFFSETOF:
> > > > > > > case RID_TYPES_COMPATIBLE_P:
> > > > > > > diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
> > > > > > > index 9b9284b1ba4..7b179449fb0 100644
> > > > > > > --- a/gcc/c/c-parser.cc
> > > > > > > +++ b/gcc/c/c-parser.cc
> > > > > > > @@ -10646,6 +10646,35 @@ c_parser_predefined_identifier (c_parser
> > > > > > > *parser)
> > > > > > > return expr;
> > > > > > > }
> > > > > > >
> > > > > > > +/* Check whether the ARRAY_REF has an counted-by object
> > > > > > > associated with it
> > > > > > > + through the "counted_by" attribute. */
> > > > > > > +static bool
> > > > > > > +has_counted_by_object (tree array_ref)
> > > > > > > +{
> > > > > > > + /* Currently, only when the array_ref is an indirect_ref to a
> > > > > > > call to the
> > > > > > > + .ACCESS_WITH_SIZE, return true.
> > > > > > > + More cases can be included later when the counted_by
> > > > > > > attribute is
> > > > > > > + extended to other situations. */
> > > > > > > + if ((TREE_CODE (array_ref) == INDIRECT_REF)
> > > > > > > + && is_access_with_size_p (TREE_OPERAND (array_ref, 0)))
> > > > > > > + return true;
> > > > > > > + return false;
> > > > > > > +}
> > > > > > > +
> > > > > > > +/* Get the reference to the counted-by object associated with
> > > > > > > the ARRAY_REF. */
> > > > > > > +static tree
> > > > > > > +get_counted_by_ref (tree array_ref)
> > > > > > > +{
> > > > > > > + /* Currently, only when the array_ref is an indirect_ref to a
> > > > > > > call to the
> > > > > > > + .ACCESS_WITH_SIZE, get the corresponding counted_by ref.
> > > > > > > + More cases can be included later when the counted_by
> > > > > > > attribute is
> > > > > > > + extended to other situations. */
> > > > > > > + if ((TREE_CODE (array_ref) == INDIRECT_REF)
> > > > > > > + && is_access_with_size_p (TREE_OPERAND (array_ref, 0)))
> > > > > > > + return CALL_EXPR_ARG (TREE_OPERAND (array_ref, 0), 1);
> > > > > > > + return NULL_TREE;
> > > > > > > +}
> > > > > > > +
> > > > > > > /* Parse a postfix expression (C90 6.3.1-6.3.2, C99 6.5.1-6.5.2,
> > > > > > > C11 6.5.1-6.5.2). Compound literals aren't handled here;
> > > > > > > callers have to
> > > > > > > call c_parser_postfix_expression_after_paren_type on
> > > > > > > encountering them.
> > > > > > > @@ -11740,6 +11769,49 @@ c_parser_postfix_expression (c_parser
> > > > > > > *parser)
> > > > > > > set_c_expr_source_range (&expr, loc, close_paren_loc);
> > > > > > > break;
> > > > > > > }
> > > > > > > + case RID_BUILTIN_GET_COUNTED_BY:
> > > > > > > + {
> > > > > > > + vec<c_expr_t, va_gc> *cexpr_list;
> > > > > > > + c_expr_t *e_p;
> > > > > > > + location_t close_paren_loc;
> > > > > > > +
> > > > > > > + c_parser_consume_token (parser);
> > > > > > > + if (!c_parser_get_builtin_args (parser,
> > > > > > > +
> > > > > > > "__builtin_get_counted_by",
> > > > > > > + &cexpr_list, false,
> > > > > > > + &close_paren_loc))
> > > > > > > + {
> > > > > > > + expr.set_error ();
> > > > > > > + break;
> > > > > > > + }
> > > > > > > + if (vec_safe_length (cexpr_list) != 1)
> > > > > > > + {
> > > > > > > + error_at (loc, "wrong number of arguments to "
> > > > > > > + "%<__builtin_get_counted_by%>");
> > > > > > > + expr.set_error ();
> > > > > > > + break;
> > > > > > > + }
> > > > > > > +
> > > > > > > + e_p = &(*cexpr_list)[0];
> > > > > > > +
> > > > > > > + if (TREE_CODE (TREE_TYPE (e_p->value)) != ARRAY_TYPE)
> > > > > > > + {
> > > > > > > + error_at (loc, "the argument must be an array"
> > > > > > > + "%<__builtin_get_counted_by%>");
> > > > > > > + expr.set_error ();
> > > > > > > + break;
> > > > > > > + }
> > > > > > > +
> > > > > > > + else if (has_counted_by_object
> > > > > > > ((*cexpr_list)[0].value))
> > > > > > > + expr.value
> > > > > > > + = get_counted_by_ref ((*cexpr_list)[0].value);
> > > > > > > + else
> > > > > > > + expr.value
> > > > > > > + = build_int_cst (build_pointer_type
> > > > > > > (size_type_node), 0);
> > > > > > > +
> > > > > > > + set_c_expr_source_range (&expr, loc, close_paren_loc);
> > > > > > > + break;
> > > > > > > + }
> > > > > > > case RID_BUILTIN_SHUFFLE:
> > > > > > > {
> > > > > > > vec<c_expr_t, va_gc> *cexpr_list;
> > > > > > > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
> > > > > > > index 48b27ff9f39..485f6f8dd8d 100644
> > > > > > > --- a/gcc/doc/extend.texi
> > > > > > > +++ b/gcc/doc/extend.texi
> > > > > > > @@ -15198,6 +15198,61 @@ initializers of variables usable in
> > > > > > > constant expressions. For more details
> > > > > > > refer to the latest revision of the C++ standard.
> > > > > > > @enddefbuiltin
> > > > > > >
> > > > > > > +@defbuiltin{@var{type} __builtin_get_counted_by (@var{ptr})}
> > > > > > > +The built-in function @code{__builtin_get_counted_by} checks
> > > > > > > whether the array
> > > > > > > +object pointed by the pointer @var{ptr} has another object
> > > > > > > associated with it
> > > > > > > +that represents the number of elements in the array object
> > > > > > > through the
> > > > > > > +@code{counted_by} attribute (i.e. the counted-by object). If so,
> > > > > > > returns a
> > > > > > > +pointer to the corresponding counted-by object.
> > > > > > > +If such counted-by object does not exist, returns a NULL pointer.
> > > > > > > +
> > > > > > > +This built-in function is only available in C for now.
> > > > > > > +
> > > > > > > +The argument @var{ptr} must be a pointer to an array.
> > > > > > > +The @var{type} of the returned value must be a pointer type
> > > > > > > pointing to the
> > > > > > > +corresponding type of the counted-by object or a pointer type
> > > > > > > pointing to
> > > > > > > +the @var{size_t} in case of a NULL pointer being returned.
> > > > > > > +
> > > > > > > +For example:
> > > > > > > +
> > > > > > > +@smallexample
> > > > > > > +struct foo1 @{
> > > > > > > + int counter;
> > > > > > > + struct bar1 array[] __attribute__((counted_by (counter)));
> > > > > > > +@} *p;
> > > > > > > +
> > > > > > > +struct foo2 @{
> > > > > > > + int other;
> > > > > > > + struct bar2 array[];
> > > > > > > +@} *q;
> > > > > > > +@end smallexample
> > > > > > > +
> > > > > > > +@noindent
> > > > > > > +the following call to the built-in
> > > > > > > +
> > > > > > > +@smallexample
> > > > > > > +__builtin_get_counted_by (p->array)
> > > > > > > +@end smallexample
> > > > > > > +
> > > > > > > +@noindent
> > > > > > > +returns:
> > > > > > > +
> > > > > > > +@smallexample
> > > > > > > +&p->counter with type @code{int *}
> > > > > > > +@end smallexample
> > > > > > > +
> > > > > > > +@noindent
> > > > > > > +However, the following call to the built-in
> > > > > > > +
> > > > > > > +@smallexample
> > > > > > > +__builtin_get_counted_by (q->array)
> > > > > > > +@end smallexample
> > > > > > > +
> > > > > > > +@noindent
> > > > > > > +returns a NULL pointer with type @code{size_t *}.
> > > > > > > +
> > > > > > > +@enddefbuiltin
> > > > > > > +
> > > > > > > @defbuiltin{void __builtin_clear_padding (@var{ptr})}
> > > > > > > The built-in function @code{__builtin_clear_padding} function
> > > > > > > clears
> > > > > > > padding bits inside of the object representation of object
> > > > > > > pointed by
> > > > > > > diff --git a/gcc/testsuite/gcc.dg/builtin-get-counted-by-1.c
> > > > > > > b/gcc/testsuite/gcc.dg/builtin-get-counted-by-1.c
> > > > > > > new file mode 100644
> > > > > > > index 00000000000..fb195f03970
> > > > > > > --- /dev/null
> > > > > > > +++ b/gcc/testsuite/gcc.dg/builtin-get-counted-by-1.c
> > > > > > > @@ -0,0 +1,91 @@
> > > > > > > +/* Test the code generation for the new
> > > > > > > __builtin_get_counted_by. */
> > > > > > > +/* { dg-do run } */
> > > > > > > +/* { dg-options "-O2" } */
> > > > > > > +#include <stdio.h>
> > > > > > > +
> > > > > > > +struct annotated {
> > > > > > > + char b;
> > > > > > > + int c[] __attribute ((counted_by (b)));
> > > > > > > +} *array_annotated;
> > > > > > > +
> > > > > > > +struct flex {
> > > > > > > + short b;
> > > > > > > + int c[];
> > > > > > > +} *array_flex;
> > > > > > > +
> > > > > > > +struct nested_annotated {
> > > > > > > + struct {
> > > > > > > + union {
> > > > > > > + int b;
> > > > > > > + float f;
> > > > > > > + };
> > > > > > > + int n;
> > > > > > > + };
> > > > > > > + char c[] __attribute__ ((counted_by (b)));
> > > > > > > +} *array_nested_annotated;
> > > > > > > +
> > > > > > > +struct nested_flex {
> > > > > > > + struct {
> > > > > > > + union {
> > > > > > > + unsigned int b;
> > > > > > > + float f;
> > > > > > > + };
> > > > > > > + int n;
> > > > > > > + };
> > > > > > > + char c[];
> > > > > > > +} *array_nested_flex;
> > > > > > > +
> > > > > > > +#define MY_ALLOC(P, FAM, COUNT) ({ \
> > > > > > > + typeof(P) __p; \
> > > > > > > + size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> > > > > > > + __p = (typeof(P)) __builtin_malloc(__size); \
> > > > > > > + __builtin_memset(__p, 0, __size); \
> > > > > > > + if (__builtin_get_counted_by (__p->FAM)) \
> > > > > > > + *(__builtin_get_counted_by(__p->FAM)) = COUNT; \
> > > > > > > + P = __p; \
> > > > > > > +})
> > > > > > > +
> > > > > > > +int count;
> > > > > > > +
> > > > > > > +int main(int argc, char *argv[])
> > > > > > > +{
> > > > > > > + MY_ALLOC(array_annotated, c, 10);
> > > > > > > + if (array_annotated->b != 10)
> > > > > > > + __builtin_abort ();
> > > > > > > +
> > > > > > > + MY_ALLOC(array_flex, c, 20);
> > > > > > > + if (array_flex->b == 20)
> > > > > > > + __builtin_abort ();
> > > > > > > +
> > > > > > > + MY_ALLOC(array_nested_annotated, c, 30);
> > > > > > > + if (array_nested_annotated->b != 30)
> > > > > > > + __builtin_abort ();
> > > > > > > +
> > > > > > > + MY_ALLOC(array_nested_flex, c, 40);
> > > > > > > + if (array_nested_flex->b == 40)
> > > > > > > + __builtin_abort ();
> > > > > > > +
> > > > > > > + count = array_annotated->b * 2 + array_nested_annotated->b * 3;
> > > > > > > + struct annotated * annotated_p;
> > > > > > > + struct flex * flex_p;
> > > > > > > + struct nested_annotated * nested_annotated_p;
> > > > > > > + struct nested_flex * nested_flex_p;
> > > > > > > +
> > > > > > > + MY_ALLOC(annotated_p, c, count);
> > > > > > > + if (annotated_p->b != count)
> > > > > > > + __builtin_abort ();
> > > > > > > +
> > > > > > > + MY_ALLOC(flex_p, c, count * 2);
> > > > > > > + if (flex_p->b == count * 2)
> > > > > > > + __builtin_abort ();
> > > > > > > +
> > > > > > > + MY_ALLOC(nested_annotated_p, c, count * 3);
> > > > > > > + if (nested_annotated_p->b != count * 3)
> > > > > > > + __builtin_abort ();
> > > > > > > +
> > > > > > > + MY_ALLOC(nested_flex_p, c, count * 4);
> > > > > > > + if (nested_flex_p->b == count * 4)
> > > > > > > + __builtin_abort ();
> > > > > > > +
> > > > > > > + return 0;
> > > > > > > +}
> > > > > > > diff --git a/gcc/testsuite/gcc.dg/builtin-get-counted-by.c
> > > > > > > b/gcc/testsuite/gcc.dg/builtin-get-counted-by.c
> > > > > > > new file mode 100644
> > > > > > > index 00000000000..5eca12bc992
> > > > > > > --- /dev/null
> > > > > > > +++ b/gcc/testsuite/gcc.dg/builtin-get-counted-by.c
> > > > > > > @@ -0,0 +1,54 @@
> > > > > > > +/* Testing the correct usage of the new
> > > > > > > __builtin_get_counted_by. */
> > > > > > > +/* { dg-do compile } */
> > > > > > > +/* { dg-options "-O" } */
> > > > > > > +
> > > > > > > +#include <stdio.h>
> > > > > > > +
> > > > > > > +struct annotated {
> > > > > > > + size_t b;
> > > > > > > + int other;
> > > > > > > + int c[] __attribute ((counted_by (b)));
> > > > > > > +} *array_annotated;
> > > > > > > +
> > > > > > > +struct flex {
> > > > > > > + size_t b;
> > > > > > > + int other;
> > > > > > > + int c[];
> > > > > > > +} *array_flex;
> > > > > > > +
> > > > > > > +#define MY_ALLOC(P, FAM, COUNT) ({ \
> > > > > > > + typeof(P) __p; \
> > > > > > > + size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> > > > > > > + __p = (typeof(P)) __builtin_malloc(__size); \
> > > > > > > + if (__builtin_get_counted_by (__p->FAM)) \
> > > > > > > + *(__builtin_get_counted_by(__p->FAM)) = COUNT; \
> > > > > > > + __p; \
> > > > > > > +})
> > > > > > > +
> > > > > > > +extern char c_count;
> > > > > > > +extern short s_count;
> > > > > > > +extern int i_count;
> > > > > > > +extern long l_count;
> > > > > > > +extern float f_count;
> > > > > > > +
> > > > > > > +extern int * foo ();
> > > > > > > +
> > > > > > > +int main(int argc, char *argv[])
> > > > > > > +{
> > > > > > > + /* The good usages. */
> > > > > > > + MY_ALLOC(array_annotated, c, 10);
> > > > > > > + MY_ALLOC(array_flex, c, 20);
> > > > > > > + MY_ALLOC(array_annotated, c, c_count);
> > > > > > > + MY_ALLOC(array_flex, c, i_count);
> > > > > > > + MY_ALLOC(array_annotated, c, l_count);
> > > > > > > + MY_ALLOC(array_flex, c, c_count * 3);
> > > > > > > + MY_ALLOC(array_annotated, c, l_count * i_count);
> > > > > > > +
> > > > > > > + /* The bad usages, issue errors. */
> > > > > > > + __builtin_get_counted_by (); /* { dg-error "wrong number of
> > > > > > > arguments to" } */
> > > > > > > + __builtin_get_counted_by (array_annotated->c, 10); /* {
> > > > > > > dg-error "wrong number of arguments to" } */
> > > > > > > + __builtin_get_counted_by (array_annotated->other); /* {
> > > > > > > dg-error "the argument must be an array" } */
> > > > > > > + __builtin_get_counted_by (foo()); /* { dg-error "the argument
> > > > > > > must be an array" } */
> > > > > > > +
> > > > > > > + return 0;
> > > > > > > +}
> > > > > > > --
> > > > > > > 2.31.1
> > >
> > >
> >
> > --
> > Univ.-Prof. Dr. rer. nat. Martin Uecker
> > Graz University of Technology
> > Institute of Biomedical Imaging
>
>
--
Univ.-Prof. Dr. rer. nat. Martin Uecker
Graz University of Technology
Institute of Biomedical Imaging
