Re: [PATCH 2/2] devtools: forbid the use of ffs compiler builtins

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 16 Oct 2024 17:32:27 +0200
David Marchand <david.marchand@redhat.com> wrote:

> On Wed, Oct 16, 2024 at 5:04 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Wed, 16 Oct 2024 15:54:11 +0200
> > Mattias Rönnblom <mattias.ronnblom@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.ronnblom@ericsson.com>
> > > Suggested-by: David Marchand <david.marchand@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.