> On Sep 11, 2024, at 18:16, Bill Wendling <isanb...@gmail.com> wrote:
>
> On Wed, Sep 11, 2024 at 2:13 PM Qing Zhao <qing.z...@oracle.com> wrote:
>>
>> compared to the 4th version, the changes are (address Jacub's concerns):
>>
>> 1. change the global "in_builtin_counted_by_ref" from a boolean to an int;
>> 2. delete the label for the error handling code, and decress the global
>> "in_builtin_counted_by_ref" before each break;
>>
>> the 4th version compared to the 3rd version, the only change is the
>> size calculation in the testing case.
>>
>> The 3rd version compared to the 2nd version, the major change is:
>> the update in testing cases per Martin's suggestions.
>>
>> when the 2nd version is compared to the first version, the major changes are:
>>
>> 1. change the name of the builtin from __builtin_get_counted_by to
>> __builtin_counted_by_ref in order to reflect the fact that the returned
>> value of it is a reference to the object.
>>
>> 2. make typeof(__builtin_counted_by_ref) working.
>>
>> 3. update the testing case to use the new builtin inside _Generic.
>>
>> bootstrapped and regress tested on both X86 and aarch64. no issue.
>>
>> Okay for the trunk?
>>
>> thanks.
>>
>> Qing.
>>
>> ==============================================
>>
>> With the addition of the 'counted_by' attribute and its wide roll-out
>> within the Linux kernel, a use case has been found that would be very
>> nice to have for object allocators: being able to set the counted_by
>> counter variable without knowing its name.
>>
>> For example, given:
>>
>> struct foo {
>> ...
>> int counter;
>> ...
>> struct bar array[] __attribute__((counted_by (counter)));
>> } *p;
>>
>> The existing Linux object allocators are roughly:
>>
>> #define MAX(A, B) (A > B) ? (A) : (B)
>> #define alloc(P, FAM, COUNT) ({ \
>> __auto_type __p = &(P); \
>> size_t __size = MAX (sizeof(*P),
>> __builtin_offsetof (__typeof(*P), FAM)
>> + sizeof (*(P->FAM)) * COUNT); \
>> *__p = kmalloc(__size); \
>> })
>>
>> Right now, any addition of a counted_by annotation must also
>> include an open-coded assignment of the counter variable after
>> the allocation:
>>
>> p = alloc(p, array, how_many);
>> p->counter = how_many;
>>
>> In order to avoid the tedious and error-prone work of manually adding
>> the open-coded counted-by intializations everywhere in the Linux
>> kernel, a new GCC builtin __builtin_counted_by_ref will be very useful
>> to be added to help the adoption of the counted-by attribute.
>>
>> -- Built-in Function: TYPE __builtin_counted_by_ref (PTR)
>> The built-in function '__builtin_counted_by_ref' checks whether the
>> array object pointed by the pointer PTR has another object
>> associated with it that represents the number of elements in the
>> array object through the '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 PTR must be a pointer to an array. The TYPE of the
>> returned value must be a pointer type pointing to the corresponding
>> type of the counted-by object or VOID pointer type in case of a
>> NULL pointer being returned.
>>
>> With this new builtin, the central allocator could be updated to:
>>
>> #define MAX(A, B) (A > B) ? (A) : (B)
>> #define alloc(P, FAM, COUNT) ({ \
>> __auto_type __p = &(P); \
>> __auto_type __c = (COUNT); \
>> size_t __size = MAX (sizeof (*(*__p)),\
>> __builtin_offsetof (__typeof(*(*__p)),FAM) \
>> + sizeof (*((*__p)->FAM)) * __c); \
>> if ((*__p = kmalloc(__size))) { \
>> __auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
>> *_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
>> } \
>> })
>>
>> And then structs can gain the counted_by attribute without needing
>> additional open-coded counter assignments for each struct, and
>> unannotated structs could still use the same allocator.
>>
>> PR c/116016
>>
>> gcc/c-family/ChangeLog:
>>
>> * c-common.cc: Add new __builtin_counted_by_ref.
>> * c-common.h (enum rid): Add RID_BUILTIN_COUNTED_BY_REF.
>>
>> gcc/c/ChangeLog:
>>
>> * c-decl.cc (names_builtin_p): Add RID_BUILTIN_COUNTED_BY_REF.
>> * c-parser.cc (has_counted_by_object): New routine.
>> (get_counted_by_ref): New routine.
>> (c_parser_postfix_expression): Handle New RID_BUILTIN_COUNTED_BY_REF.
>> * c-tree.h: New global in_builtin_counted_by_ref.
>> * c-typeck.cc (build_component_ref): Enable generating
>> .ACCESS_WITH_SIZE inside typeof when inside builtin_counted_by_ref.
>>
>> gcc/ChangeLog:
>>
>> * doc/extend.texi: Add documentation for __builtin_counted_by_ref.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * gcc.dg/builtin-counted-by-ref-1.c: New test.
>> * gcc.dg/builtin-counted-by-ref.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 | 78 +++++++++++++++
>> gcc/c/c-tree.h | 1 +
>> gcc/c/c-typeck.cc | 7 +-
>> gcc/doc/extend.texi | 55 +++++++++++
>> .../gcc.dg/builtin-counted-by-ref-1.c | 97 +++++++++++++++++++
>> gcc/testsuite/gcc.dg/builtin-counted-by-ref.c | 61 ++++++++++++
>> 9 files changed, 301 insertions(+), 1 deletion(-)
>> create mode 100644 gcc/testsuite/gcc.dg/builtin-counted-by-ref-1.c
>> create mode 100644 gcc/testsuite/gcc.dg/builtin-counted-by-ref.c
>>
>> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
>> index e7e371fd26f..15b90bae8b5 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_counted_by_ref", RID_BUILTIN_COUNTED_BY_REF, 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 027f077d51b..4400d2c5d43 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_COUNTED_BY_REF,
>> 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 aa7f69d1b7b..1145dde9bb1 100644
>> --- a/gcc/c/c-decl.cc
>> +++ b/gcc/c/c-decl.cc
>> @@ -11788,6 +11788,7 @@ names_builtin_p (const char *name)
>> case RID_BUILTIN_SHUFFLE:
>> case RID_BUILTIN_SHUFFLEVECTOR:
>> case RID_BUILTIN_STDC:
>> + case RID_BUILTIN_COUNTED_BY_REF:
>> 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 aff5af17430..91bd91f4be1 100644
>> --- a/gcc/c/c-parser.cc
>> +++ b/gcc/c/c-parser.cc
>> @@ -10647,6 +10647,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.
>> @@ -11741,6 +11770,55 @@ c_parser_postfix_expression (c_parser *parser)
>> set_c_expr_source_range (&expr, loc, close_paren_loc);
>> break;
>> }
>> + case RID_BUILTIN_COUNTED_BY_REF:
>> + {
>> + vec<c_expr_t, va_gc> *cexpr_list;
>> + c_expr_t *e_p;
>> + location_t close_paren_loc;
>> +
>> + in_builtin_counted_by_ref++;
>> +
>> + c_parser_consume_token (parser);
>> + if (!c_parser_get_builtin_args (parser,
>> + "__builtin_counted_by_ref",
>> + &cexpr_list, false,
>> + &close_paren_loc))
>> + {
>> + expr.set_error ();
>> + in_builtin_counted_by_ref--;
>> + break;
>> + }
>> + if (vec_safe_length (cexpr_list) != 1)
>> + {
>> + error_at (loc, "wrong number of arguments to "
>> + "%<__builtin_counted_by_ref%>");
>> + expr.set_error ();
>> + in_builtin_counted_by_ref--;
>> + 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_counted_by_ref%>");
>> + expr.set_error ();
>> + in_builtin_counted_by_ref--;
>> + break;
>> + }
>> +
>> + 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 (void_type_node), 0);
>> +
>> + set_c_expr_source_range (&expr, loc, close_paren_loc);
>> + in_builtin_counted_by_ref--;
>> + break;
>> + }
>> case RID_BUILTIN_SHUFFLE:
>> {
>> vec<c_expr_t, va_gc> *cexpr_list;
>> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
>> index 57befb94c08..6f46b2ff5a0 100644
>> --- a/gcc/c/c-tree.h
>> +++ b/gcc/c/c-tree.h
>> @@ -737,6 +737,7 @@ extern int c_type_dwarf_attribute (const_tree, int);
>> extern int in_alignof;
>> extern int in_sizeof;
>> extern int in_typeof;
>> +extern int in_builtin_counted_by_ref;
>> extern bool c_in_omp_for;
>> extern bool c_omp_array_section_p;
>>
>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>> index 58b2724b39e..8cb2cb60f6a 100644
>> --- a/gcc/c/c-typeck.cc
>> +++ b/gcc/c/c-typeck.cc
>> @@ -74,6 +74,9 @@ int in_sizeof;
>> /* The level of nesting inside "typeof". */
>> int in_typeof;
>>
>> +/* The level of nesting inside "builtin_counted_by_ref". */
>> +int in_builtin_counted_by_ref;
>> +
>> /* True when parsing OpenMP loop expressions. */
>> bool c_in_omp_for;
>>
>> @@ -2848,7 +2851,9 @@ build_component_ref (location_t loc, tree datum, tree
>> component,
>> bool use_datum_quals;
>> tree counted_by_type = NULL_TREE;
>> /* Do not handle counted_by when in typeof and alignof operator. */
>> - handle_counted_by = handle_counted_by && !in_typeof && !in_alignof;
>> + handle_counted_by = handle_counted_by
>> + && !(in_typeof && !in_builtin_counted_by_ref)
>> + && !in_alignof;
>> tree counted_by_ref = handle_counted_by
>> ? build_counted_by_ref (datum, subdatum,
>> &counted_by_type)
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index 2d795ba7e59..268e50677d7 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -15287,6 +15287,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_counted_by_ref (@var{ptr})}
>> +The built-in function @code{__builtin_counted_by_ref} 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 VOID pointer type in case
>> +of a NULL pointer being returned.
>> +
> Having the return type default to a VOID pointer restricts it from this use:
>
> if (__builtin_counted_by_ref(p->FAM))
> *__builtin_counted_by_ref(p->FAM) = 42;
>
> The second line will emit a diagnostic.
We have discussion on this during the 1st version of the patch. at that
version, the __builtin_counted_by_ref return a size_t * void pointer if no
counted_by attribute for the above usage, however, Richard pointed out that
"the above usage is odd for a statically typed languages, on the other hand,
type selection like generics or __builtin_classify_type instead of overloading
the value producing builtin might be more fit”. please see the following for
more details:
https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660871.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660908.html
https://gcc.gnu.org/pipermail/gcc-patches/2024-August/660995.html
> Is VOID pointer just to make
> using _Generic a bit cleaner? Is that now the prefered way to use this
> builtin?
Yes, I think so. __Generic or __builtin_choose_expr is the preferred way to use
this builtin.
Qing
>
> -bw
>
>> +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_counted_by_ref (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_counted_by_ref (q->array)
>> +@end smallexample
>> +
>> +@noindent
>> +returns a void NULL pointer.
>> +
>> +@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-counted-by-ref-1.c
>> b/gcc/testsuite/gcc.dg/builtin-counted-by-ref-1.c
>> new file mode 100644
>> index 00000000000..d055cf1defc
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/builtin-counted-by-ref-1.c
>> @@ -0,0 +1,97 @@
>> +/* Test the code generation for the new __builtin_counted_by_ref. */
>> +/* { 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 MAX(A, B) (A > B) ? (A) : (B)
>> +#define MY_ALLOC(P, FAM, COUNT) ({ \
>> + __auto_type __p = &(P); \
>> + __auto_type __c = (COUNT); \
>> + size_t __size = MAX (sizeof (*(*__p)),\
>> + __builtin_offsetof (__typeof(*(*__p)),FAM) \
>> + + sizeof (*((*__p)->FAM)) * __c); \
>> + if ((*__p = __builtin_malloc(__size))) { \
>> + __builtin_memset(*__p, 0, __size); \
>> + __auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
>> + *_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
>> + if (sizeof (__builtin_counted_by_ref ((*__p)->FAM)) != sizeof (char *))
>> \
>> + __builtin_abort (); \
>> + } \
>> +})
>> +
>> +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-counted-by-ref.c
>> b/gcc/testsuite/gcc.dg/builtin-counted-by-ref.c
>> new file mode 100644
>> index 00000000000..4aa57ffb916
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/builtin-counted-by-ref.c
>> @@ -0,0 +1,61 @@
>> +/* Testing the correct usage of the new __builtin_counted_by_ref. */
>> +/* { 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 MAX(A, B) (A > B) ? (A) : (B)
>> +#define MY_ALLOC(P, FAM, COUNT) ({ \
>> + __auto_type __p = &(P); \
>> + __auto_type __c = (COUNT); \
>> + size_t __size = MAX (sizeof (*(*__p)),\
>> + __builtin_offsetof (__typeof(*(*__p)),FAM) \
>> + + sizeof (*((*__p)->FAM)) * __c); \
>> + if ((*__p = __builtin_malloc(__size))) { \
>> + __builtin_memset(*__p, 0, __size); \
>> + __auto_type ret = __builtin_counted_by_ref((*__p)->FAM); \
>> + *_Generic(ret, void *: &(size_t){0}, default: ret) = __c; \
>> + if (sizeof (__builtin_counted_by_ref ((*__p)->FAM)) != sizeof (char *))
>> \
>> + __builtin_abort (); \
>> + } \
>> +})
>> +
>> +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_counted_by_ref (); /* { dg-error "wrong number of arguments to"
>> } */
>> + __builtin_counted_by_ref (array_annotated->c, 10); /* { dg-error "wrong
>> number of arguments to" } */
>> + __builtin_counted_by_ref (array_annotated->other); /* { dg-error "the
>> argument must be an array" } */
>> + __builtin_counted_by_ref (foo()); /* { dg-error "the argument must be an
>> array" } */
>> +
>> + return 0;
>> +}
>> --
>> 2.31.1
