[dpdk-dev] [PATCH v8 01/14] build: alias default build as generic

[Juraj Linkeš]

> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli at arm.com>
> Sent: Saturday, November 7, 2020 12:53 AM
> To: Juraj Linkeš <juraj.linkes at pantheon.tech>; bruce.richardson at intel.com;
> Ruifeng Wang <Ruifeng.Wang at arm.com>; Phil Yang <Phil.Yang at arm.com>;
> vcchunga at amazon.com; Dharmik Thakkar <Dharmik.Thakkar at arm.com>;
> jerinjacobk at gmail.com; hemant.agrawal at nxp.com; Ajit Khaparde
> (ajit.khaparde at broadcom.com) <ajit.khaparde at broadcom.com>;
> ferruh.yigit at intel.com; aconole at redhat.com
> Cc: dev at dpdk.org; nd <nd at arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli at arm.com>; nd <nd at arm.com>
> Subject: RE: [PATCH v8 01/14] build: alias default build as generic
> 
> <snip>
> 
> >
> > The current machine='default' build name is not descriptive. The
> > actual default build is machine='native'. Add an alternative string
> > which does the same build and better describes what we're building:
> > machine='generic'. Leave machine='default' for backwards compatibility.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes at pantheon.tech>
> > ---
> >  config/arm/meson.build                    | 5 +++--
> >  config/meson.build                        | 9 +++++----
> >  doc/guides/prog_guide/build-sdk-meson.rst | 4 ++--
> >  meson_options.txt                         | 2 +-
> >  4 files changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 42b4e43c7..d4066ade8 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -1,12 +1,13 @@
> >  # SPDX-License-Identifier: BSD-3-Clause  # Copyright(c) 2017 Intel
> > Corporation.
> >  # Copyright(c) 2017 Cavium, Inc
> > +# Copyright(c) 2020 PANTHEON.tech s.r.o.
> >
> >  # for checking defines we need to use the correct compiler flags
> > march_opt =
> > '-march=@0@'.format(machine)
> >
> >  arm_force_native_march = false
> > -arm_force_default_march = (machine == 'default')
> > +arm_force_generic_march = (machine == 'generic')
> >
> >  flags_common_default = [
> >  	# Accelarate rte_memcpy. Be sure to run unit test
> > (memcpy_perf_autotest) @@ -148,7 +149,7 @@ else
> >  	cmd_generic = ['generic', '', '', 'default', '']
> >  	cmd_output = cmd_generic # Set generic by default
> >  	machine_args = [] # Clear previous machine args
> > -	if arm_force_default_march and not meson.is_cross_build()
> > +	if arm_force_generic_march and not meson.is_cross_build()
> >  		machine = impl_generic
> >  		impl_pn = 'default'
> >  	elif not meson.is_cross_build()
> > diff --git a/config/meson.build b/config/meson.build index
> > 258b01d06..c7f7aa6e2 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -68,13 +68,14 @@ else
> >  	machine = get_option('machine')
> >  endif
> >
> > -# machine type 'default' is special, it defaults to the per arch
> > agreed common -# minimal baseline needed for DPDK.
> > +# machine type 'generic' is special, it defaults to the per arch
> > +agreed common # minimal baseline needed for DPDK. Machine type
> > +'default' is also supported # with the same meaning for backwards
> compatibility.
> >  # That might not be the most optimized, but the most portable version
> > while # still being able to support the CPU features required for DPDK.
> >  # This can be bumped up by the DPDK project, but it can never be an
> > # invariant like 'native'
> > -if machine == 'default'
> > +if machine == 'default' or machine == 'generic'
> >  	if host_machine.cpu_family().startswith('x86')
> >  		# matches the old pre-meson build systems default
> >  		machine = 'corei7'
> > @@ -82,7 +83,7 @@ if machine == 'default'
> >  		machine = 'armv7-a'
> >  	elif host_machine.cpu_family().startswith('aarch')
> >  		# arm64 manages defaults in config/arm/meson.build
> > -		machine = 'default'
> > +		machine = 'generic'
> >  	elif host_machine.cpu_family().startswith('ppc')
> >  		machine = 'power8'
> >  	endif
> > diff --git a/doc/guides/prog_guide/build-sdk-meson.rst
> > b/doc/guides/prog_guide/build-sdk-meson.rst
> > index 3429e2647..c7e12eedf 100644
> > --- a/doc/guides/prog_guide/build-sdk-meson.rst
> > +++ b/doc/guides/prog_guide/build-sdk-meson.rst
> > @@ -85,7 +85,7 @@ Project-specific options are passed used -
> > Doption=value::
> >
> >  	meson -Denable_docs=true fullbuild  # build and install docs
> >
> > -	meson -Dmachine=default  # use builder-independent baseline -
> > march
> > +	meson -Dmachine=generic  # use builder-independent baseline -
> > march
> >
> >  	meson -Ddisable_drivers=event/*,net/tap  # disable tap driver and all
> >  					# eventdev PMDs for a smaller build
> @@ -114,7 +114,7 @@ Examples
> > of setting some of the same options using meson configure::
> >          re-scan from meson.
> >
> >  .. note::
> > -        machine=default uses a config that works on all supported architectures
> > +        machine=generic uses a config that works on all supported
> > + architectures
> >          regardless of the capabilities of the machine where the build
> > is happening.
> >
> >  As well as those settings taken from ``meson configure``, other
> > options diff -- git a/meson_options.txt b/meson_options.txt index
> > 9bf18ab6b..ce23289e3
> > 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -23,7 +23,7 @@ option('kernel_dir', type: 'string', value: '',
> > option('lib_musdk_dir', type: 'string', value: '',
> >  	description: 'path to the MUSDK library installation directory')
> > option('machine', type: 'string', value: 'native',
> > -	description: 'set the target machine type')
> > +	description: 'set the target machine type. Set to generic for a
> > +build usable on most machines of the build machine architecture, set
> > +to
>                         ^^^^ all

I don't think we've settled this with Bruce. According to him, the build with "machine=generic" on x86 won't necessarily produce a build usable on all machines, so I went with most machines.

> > +native to let the compiler choose the best fit for the build
> > +machine.')
> How about: native to let the compiler choose the attributes of the build machine
> 

I haven't looked at what the docs before, so now here's what GCC docs say: https://gcc.gnu.org/onlinedocs/gcc-8.4.0/gcc/AArch64-Options.html#AArch64-Options:
The value ‘native’ is available on native AArch64 GNU/Linux and causes the compiler to pick the architecture of the host system.

Best fit is kinda vague and choosing the attibutes is probably not using the right verb (it's always going to use the same subset of attributes of the build machine in question, so it's not really a choice), I like use the attributes more.

So it would be either "use the attributes of the build machine" or "pick the architecture of the build machine" (I've changed host system to build machine to match meson language). Both look fine to be, but I'm always inclined to use wording from documentation, even if it's from one compiler. Which one do you like more?

> Otherwise, it looks good to me.
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli at arm.com>
> 
> >  option('max_ethports', type: 'integer', value: 32,
> >  	description: 'maximum number of Ethernet devices')
> > option('max_lcores', type: 'integer', value: 128,
> > --
> > 2.20.1