On Wed, Oct 16, 2024 at 5:39 PM Stephen Hemminger
<step...@networkplumber.org> wrote:
>
> On Wed, 16 Oct 2024 17:32:27 +0200
> David Marchand <david.march...@redhat.com> wrote:
>
> > On Wed, Oct 16, 2024 at 5:04 PM Stephen Hemminger
> > <step...@networkplumber.org> wrote:
> > >
> > > On Wed, 16 Oct 2024 15:54:11 +0200
> > > Mattias Rönnblom <mattias.ronnb...@ericsson.com> wrote:
> > >
> > > > Extend checkpatches.sh to detect the use of __builtin_(ffs|ffsll).
> > > >
> > > > These intrinsics are not available in MSVC, and there are perfectly
> > > > serviceable alternatives in <rte_bitops.h>.
> > > >
> > > > Signed-off-by: Mattias Rönnblom <mattias.ronnb...@ericsson.com>
> > > > Suggested-by: David Marchand <david.march...@redhat.com>
> > >
> > > Shouldn't this apply to all _builtin_ functions.
> > >
> > > There are a lot of drivers still doing this.
> >
> > - The updated check here is about use of __builtin_XXX instead of DPDK
> > bit count ops.
> > So when you refer to all builtin functions, there may be other
> > builtins related to bit count we are missing, but at least the current
> > patch is better than before.
> >
> >
> > - On the larger topic of refusing *any* __builtin_, some use of them
> > are "legit" under compiler checks.
> > So the check would have to skip headers where wrappers are provided to
> > the rest of DPDK.
> > This seems possible, but a larger and probably more tricky change for now.
> >
> > Are you ok if we go with this patch as is, in a first step?
> >
> >
>
> Examples of direct use of builtin
> app/dumpcap/main.c: log2 = sizeof(size) * 8 - __builtin_clzl(size
> - 1);
> app/test-dma-perf/benchmark.c:
> __builtin_ia32_clflush(data + offset);
> drivers/bus/fslmc/qbman/include/compat.h:#define likely(x)
> __builtin_expect(!!(x), 1)
> drivers/bus/fslmc/qbman/include/compat.h:#define unlikely(x)
> __builtin_expect(!!(x), 0)
>
> drivers/common/nfp/nfp_platform.h:#define __bf_shf(x)
> (__builtin_ffsll(x) - 1)
> drivers/crypto/ccp/ccp_dev.c: first_zero = __builtin_ffsl(~word);
> app/test/test_memcpy_perf.c: if (__builtin_constant_p(n))
>
> drivers/net/bonding/rte_eth_bond_pmd.c: if
> (__builtin_popcountl(link_speeds) != 2) {
>
>
> Think that direct use of __builitin_expect should be flagged.
> And the lots of __builtin_in mlx5 should also be flagged.
Yes, we should fix those, but that's beyond the bitops check.
I'll go ahead with current series for now.
--
David Marchand
