Re: [dpdk-dev] [RFC] toolchain specific macro expansion

[Tyler Retzlaff <roretzla@linux.microsoft.com>]

On Thu, Jun 24, 2021 at 08:54:49AM +0200, Thomas Monjalon wrote:
> 23/06/2021 20:26, Tyler Retzlaff:
> > today rte_common.h defines common macros for use by dpdk and consuming
> > applications. most expansions are specific to the gcc toolchain.
> > 
> > example
> > // lib/eal/include/rte_common.h
> > 
> > /**
> >  * Hint never returning function
> >  */
> > #define __rte_noreturn __attribute__((noreturn))
> > 
> > there is an anticipated need rte_common.h to expose alternate
> > expansions for non-gcc toolchains in the future and would like
> > comments on how to achieve this in an agreeable manner.
> > 
> > 
> > option 1 add conditional compilation directly to rte_common.h
> > 
> > example
> > // lib/eal/include/rte_common.h
> > 
> > /**
> >  * Hint never returning function
> >  */
> > #ifdef RTE_TOOLCHAIN_GCC
> > #define __rte_noreturn __attribute__((noreturn))
> > #endif
> > 
> > #ifdef RTE_TOOLCHAIN_FOO
> > #define __rte_noreturn __foo_expansion_for_noreturn
> > #endif
> > 
> > represents the simplest approach technically but may be tedious to
> > maintain due to the number of macros and number of conditional
> > expansions per macro.
> 
> Macros are simple so the option 1 is not that bad.

our preference for option 2 is based mainly on experience in platform
conditional compile where reviewers of patches ask us to reduce
or attempt to hide as much visible conditional compilation.  the testpmd
patch is a recent example where we get asked to reorganize code to avoid
frequency of conditional compile for windows execenv.

either way if the community prefers option 1 we'll do it that way we
look forward to seeing additional responses.

> 
> 
> > option 2 add toolchain specific files (follow existing platform/os
> >          includes pattern)
> > 
> > example:
> > // lib/eal/gcc/rte_toolchain_common.h
> > #define __rte_noreturn __attribute__((noreturn))
> 
> We should keep a macro in rte_common.h which triggers an explicit error

i think that's relatively trivial to do. rte_common.h could after
toolchain specific include do a simple test.

#ifndef __rte_no_return
#error no __rte_no_return defined for toolchain
#endif

what is harder to catch is if there is an addition of a new macro to a
toolchain specific header where there is no corresponding test added to
rte_common.h this is somewhat mitigated by standing up more ci
automation that covers the set of toolchains that may be used and to
some degree would need help from patch reviewers to catch but on balance
it is no worse than what we already have.