Re: [PATCH v3 3/5] xen/bitops: Implement hweight32() in terms of hweightl()

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2024-09-06 20:50, Andrew Cooper wrote:
> On 06/09/2024 1:27 am, Andrew Cooper wrote:
> > On 06/09/2024 12:08 am, Stefano Stabellini wrote:
> > > On Wed, 4 Sep 2024, Andrew Cooper wrote:
> > > > ... and drop generic_hweight32().
> > > >
> > > > As noted previously, the only two users of hweight32() are in __init 
> > > > paths.
> > > > The int-optimised form of generic_hweight() is only two instructions 
> > > > shorter
> > > > than the long-optimised form, and even then only on architectures 
> > > > which lack
> > > > fast multiplication, so there's no point providing an int-optimised 
> > > > form.
> > > > No functional change.
> > > >
> > > > Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > > Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> > > The patch is OK:
> > >
> > > Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Thanks.
> >
> > > I was looking at docs/misra/C-language-toolchain.rst to make sure
> > > everything is listed there. We have attr_const as "__const__" noted
> > > among "Non-standard tokens".
> > >
> > > Looks like we need to add __always_inline__ ?
> > Luckily, no.
> >
> > First, the __const__ referred to there is GCC's alternative spelling 
> > of
> > the regular C 'const' keyword, and not the function attribute by the
> > same name.
> >
> > But, non-standard tokens are about things which survive full
> > preprocessing and are still not standard C.  In this case, it's the
> > __attribute__ that matters, not what's in it, and this is why we don't
> > have a hundreds of rows in that table for __attribute__((foo, bar, 
> > baz)).
> > That said, I think f96e2f64576cdbb147391c7cb399d393385719a9 probably
> > should have removed the entire row including __const__ seeing as 
> > AFAICT
> > the last user in Xen was dropped in 1aa3c54a31a5 in 2008
> Hmm.  Eclair disagrees.
>
> https://gitlab.com/xen-project/people/andyhhp/xen/-/jobs/7765315981
>
> Now I'm completely confused as to why __const__ matters and everything
> else inside an __attribute__(()) apparently doesn't.
>
> CC-ing some folk.  Any ideas?
>
> ~Andrew
The reason why __attribute__((__const__)) is reported after removing 
from the configuration of STD.tokenext the __const__ token is that:
- __const__ is just an alternate keyword for const, and as such needs to 
be special-cased in the configuration, as it was originally;
- the __const__ inside the attribute is still a non-standard token, no 
matter the surrounding context, and the kind of token is determined 
right after preprocessing as being a keyword (albeit non-standard), as 
far as I'm aware;
- this only happens due to the overlap with the alternate keyword 
__const__; in other words, other attributes such as 
__transparent_union__ in __attribute__((__transparent_union__)) do not 
need to be listed as permitted non-standard tokens because they aren't 
either standard or non-standard keywords, but just ordinary identifiers.
Therefore, I suggest keeping the __const__ inside the configuration.

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)