Re: [dpdk-dev] [PATCH 4/4] net: replace ifdefs with runtime branches

[Thomas Monjalon <thomas@monjalon.net>]

29/05/2019 17:41, Bruce Richardson:
> Use the flag checking functions and a couple of empty stubs to remove the
> ifdefs from the middle of the C code, and replace them with more readable
> regular if statements. Other ifdefs at the top of the file are kept, but
> are not mixed with C code, so there is a clean separation.
> 
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/librte_net/rte_net_crc.c | 38 ++++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 13 deletions(-)

The result is more lines of code :)

> --- a/lib/librte_net/rte_net_crc.c
> +++ b/lib/librte_net/rte_net_crc.c
> @@ -18,8 +18,17 @@
>  
>  #ifdef X86_64_SSE42_PCLMULQDQ
>  #include <net_crc_sse.h>
> -#elif defined ARM64_NEON_PMULL
> +#else
> +/* define stubs for the SSE functions to avoid compiler errors */
> +#define handlers_sse42 handlers_scalar
> +#define rte_net_crc_sse42_init() do { } while(0)
> +#endif
> +
> +#ifdef ARM64_NEON_PMULL
>  #include <net_crc_neon.h>
> +#else
> +#define handlers_neon handlers_scalar
> +#define rte_net_crc_neon_init() do { } while(0)
>  #endif

Looking at the need for stubs, I don't see the benefit.

>  rte_net_crc_set_alg(enum rte_net_crc_alg alg)
>  {
>  	switch (alg) {
> -#ifdef X86_64_SSE42_PCLMULQDQ
>  	case RTE_NET_CRC_SSE42:
> -		handlers = handlers_sse42;
> -		break;
> -#elif defined ARM64_NEON_PMULL
> +		if (rte_cpu_get_flagname_enabled(rte_cpu_arch_x86,
> +				"RTE_CPUFLAG_SSE4_2")) {

Why the CPU flags strings are prefixed with RTE_CPUFLAG_?