On Tue, Aug 20, 2024 at 6:41 AM Qing Zhao <qing.z...@oracle.com> wrote:
> > On Aug 20, 2024, at 05:58, Richard Biener <richard.guent...@gmail.com>
> > wrote:
> >
> > On Tue, Aug 13, 2024 at 5:34 PM Qing Zhao <qing.z...@oracle.com> wrote:
> >>
> >> 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 alloc(P, FAM, COUNT) ({ \
> >> size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> >> kmalloc(__size, GFP); \
> >> })
> >>
> >> 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_get_counted_by will be very useful
> >> to be added to help the adoption of the counted-by attribute.
> >>
> >> -- Built-in Function: TYPE __builtin_get_counted_by (PTR)
> >> The built-in function '__builtin_get_counted_by' 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 a pointer type pointing to the
> >> SIZE_T in case of a NULL pointer being returned.
> >>
> >> With this new builtin, the central allocator could be updated to:
> >>
> >> #define alloc(P, FAM, COUNT) ({ \
> >> typeof(P) __p; \
> >> size_t __size = sizeof(*P) + sizeof(*P->FAM) * COUNT; \
> >> __p = kmalloc(__size, GFP); \
> >> if (__builtin_get_counted_by (__p->FAM)) \
> >> *(__builtin_get_counted_by(__p->FAM)) = COUNT; \
> >> __p; \
> >> })
> >>
> >> 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.
> >
> > Did you consider a __builtin_set_counted_by (PTR, VALUE)?
>
> Yes, that’s the initial request from Kees. -)
>
> The title of PR116016 is: add __builtin_set_counted_by(P->FAM, COUNT) or
> equivalent
>
> After extensive discussion (Martin Uecker raised the initial idea in
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116016#c24, more discussions
> followed, till comments #31). we decided to provide
> __builtin_get_counted_by(PTR) instead of __builtin_set_counted_by(PTR, VALUE)
> due to the following two reasons:
>
> 1. __builtin_get_counted_by should be enough to provide the functionality,
> and even simpler;
> 2. More flexible to be used by the programmer to be able to both WRITE and
> READ the counted-by field.
>
> >
> > Note that __builtin_get_counted_by to me suggests it returns the
> > value and not a pointer to the value.
>
> The syntax of __builtin_get_counted_by is:
>
> TYPE __builtin_get_counted_by (PTR)
>
> The returned value is:
>
> 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 a pointer type pointing to the
> SIZE_T in case of a NULL pointer being returned.
>
>
> > A more proper language extension might involve a keyword
> > like __real, so __counted_by X would produce an lvalue, selecting
> > the counted-by member.
>
> Yes, if the returned value could be a LVALUE instead of a Pointer, that’s
> even simpler and cleaner.
> However, then as you mentioned below, another builtin
> “__builtin_has_attribute(PTR, counted_by)” need
> to be queried first to make sure the counted_by field exists.
>
> We have discussed this approach, and I preferred this approach too.
>
> However, the main reason we gave up on that direction is:
>
> There is NO __builtin_has_attribute (PTR, counted_by) been supported by
> CLANG, and not sure how difficult for CLANG to add this new builtin.
>
> In order to provide consistent interface to Linux kernel from both CLANG and
> GCC, I finally decided to provide the current interface in order to be
> consistent with CLANG.
>
> Bill, could you please provide a little bit more info on the possibility of a
> new builtin __builtin_has_attribute() in CLANG?
>
>From what I gathered, it would require some moderate surgery to the
Clang front-end to support "__builtin_has_attribute()". Parsing of
attributes is done only in the context of the "__attribute__" keyword
and would have to be refactored. That in itself would probably be a
nice clean-up to the Clang front-end. The bigger issue is how to
support an attribute as a node in the AST. We don't currently have a
node like that in our AST, because attributes are associated with an
AST node and aren't nodes in and of themselves. So it's a lot more
work than it appears on the surface.
That said, Clang probably *should* support __builtin_has_attribute().
The question is whether we should do it for this feature or wait? If
we do it for this feature, I imagine that Clang's implementation of
__builtin_get_counted_by() will be significantly delayed, but if it's
the correct thing to do then we should bite the bullet.
-bw
> > You'd need another way to query whether
> > 'X' has a counted-by member - the apparent runtime test in your
> > example is odd for a statically typed language, type selection
> > like with generics
>
> Could you please explain this a little bit more? (Not quite understood, a
> small example will be better, -:) thanks a lot.
> > or __builtin_classify_type instead of overloading
> > the value producing builtin might be more fit here?
>
> You mean the following lines from the testing case:
>
> + if (__builtin_get_counted_by (__p->FAM)) \
> + *(__builtin_get_counted_by(__p->FAM)) = COUNT; \
>
> How to improve it? (Thanks a lot for your suggestion).
>
> Qing
