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

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jul 01, 2019 at 09:30:02PM +0200, Thomas Monjalon wrote:
> 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.
>
Yes, one needs stubs, but those are placed in a single place, and the main
C-code itself is free of ifdefs running through it. I'd view this as a good
thing in limiting the scope of any ifdef-ery, since it annoys me looking at
#ifdefs in the middle of functions (since it messes up the indentation flow
of the code if nothing else!).

If you don't see this as a big benefit, then there is not a lot else to
commend this set for you, sadly. It just seemed a nice improvement to me -
irrespective of net lines of code.

/Bruce