> On Aug 21, 2024, at 11:43, Martin Uecker <uec...@tugraz.at> wrote: > > 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. Okay, I see. Yes, this makes good sense to me. > > >> >>> >>> >>>> >>>> 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.
For the unary operator __counted_by(PTR), “PTR” must have a counted_by attribute, if not, there will be a compilation time error. Then the user could write the following code: If __builtin_has_attriubtes (PTR,counted_by) __counted_by(PTR) = COUNT; From the design point of view, I think this might be the cleanest solution. However, currently, CLANG doesn’t have __builtin_has_attributes. In order to provide a consistent interface, __builtin_get_counted_by(PTR) is fine. Thanks. Qing > > > 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