DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [RFC PATCH v1] build: add platform meson option
@ 2020-11-26 15:47 Juraj Linkeš
  2020-11-26 16:02 ` Bruce Richardson
  2021-01-04 11:52 ` [dpdk-dev] [RFC PATCH v2] " Juraj Linkeš
  0 siblings, 2 replies; 16+ messages in thread
From: Juraj Linkeš @ 2020-11-26 15:47 UTC (permalink / raw)
  To: thomas, bruce.richardson, Honnappa.Nagarahalli; +Cc: dev, Juraj Linkeš

The current meson option 'machine' should only specify the ISA, which is
not sufficient for Arm, where setting ISA implies other setting as well.
Add a new meson option, 'platform', which differentiates the type of the
build (native/generic) and sets machine accordingly, unless the user
chooses to override it.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 config/arm/meson.build |  2 +-
 config/meson.build     | 14 +++++++++++++-
 meson_options.txt      |  6 ++++--
 3 files changed, 18 insertions(+), 4 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 42b4e43c7..ac680956f 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -6,7 +6,7 @@
 march_opt = '-march=@0@'.format(machine)
 
 arm_force_native_march = false
-arm_force_default_march = (machine == 'default')
+arm_force_default_march = (platform == 'generic')
 
 flags_common_default = [
 	# Accelarate rte_memcpy. Be sure to run unit test (memcpy_perf_autotest)
diff --git a/config/meson.build b/config/meson.build
index c02802c18..41d32e63e 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -63,6 +63,8 @@ if not is_windows
 			pmd_subdir_opt)
 endif
 
+platform = get_option('platform')
+
 # set the machine type and cflags for it
 if meson.is_cross_build()
 	machine = host_machine.cpu()
@@ -70,13 +72,23 @@ else
 	machine = get_option('machine')
 endif
 
+if platform == 'native'
+	if machine == 'auto'
+		machine = 'native'
+	endif
+elif platform == 'generic'
+	if machine == 'auto'
+		machine = 'default'
+	endif
+endif
+
+if machine == 'default'
 # machine type 'default' is special, it defaults to the per arch agreed common
 # minimal baseline needed for DPDK.
 # 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 host_machine.cpu_family().startswith('x86')
 		# matches the old pre-meson build systems default
 		machine = 'corei7'
diff --git a/meson_options.txt b/meson_options.txt
index e384e6dbb..1a5e47fd3 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
 	description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
 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')
+option('machine', type: 'string', value: 'auto',
+	description: 'set the target machine type/ISA')
 option('max_ethports', type: 'integer', value: 32,
 	description: 'maximum number of Ethernet devices')
 option('max_lcores', type: 'integer', value: 128,
 	description: 'maximum number of cores/threads supported by EAL')
 option('max_numa_nodes', type: 'integer', value: 4,
 	description: 'maximum number of NUMA nodes supported by EAL')
+option('platform', type: 'string', value: 'generic',
+	description: 'Platform to build for, either "native" or "generic".')
 option('enable_trace_fp', type: 'boolean', value: false,
 	description: 'enable fast path trace points.')
 option('tests', type: 'boolean', value: true,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: add platform meson option
  2020-11-26 15:47 [dpdk-dev] [RFC PATCH v1] build: add platform meson option Juraj Linkeš
@ 2020-11-26 16:02 ` Bruce Richardson
  2020-11-27  8:31   ` Juraj Linkeš
  2021-01-04 11:52 ` [dpdk-dev] [RFC PATCH v2] " Juraj Linkeš
  1 sibling, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2020-11-26 16:02 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: thomas, Honnappa.Nagarahalli, dev

On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> The current meson option 'machine' should only specify the ISA, which is
> not sufficient for Arm, where setting ISA implies other setting as well.
> Add a new meson option, 'platform', which differentiates the type of the
> build (native/generic) and sets machine accordingly, unless the user
> chooses to override it.
> 
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>  config/arm/meson.build |  2 +-
>  config/meson.build     | 14 +++++++++++++-
>  meson_options.txt      |  6 ++++--
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 42b4e43c7..ac680956f 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -6,7 +6,7 @@
>  march_opt = '-march=@0@'.format(machine)
>  
>  arm_force_native_march = false
> -arm_force_default_march = (machine == 'default')
> +arm_force_default_march = (platform == 'generic')
>  
>  flags_common_default = [
>  	# Accelarate rte_memcpy. Be sure to run unit test (memcpy_perf_autotest)
> diff --git a/config/meson.build b/config/meson.build
> index c02802c18..41d32e63e 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -63,6 +63,8 @@ if not is_windows
>  			pmd_subdir_opt)
>  endif
>  
> +platform = get_option('platform')
> +
>  # set the machine type and cflags for it
>  if meson.is_cross_build()
>  	machine = host_machine.cpu()
> @@ -70,13 +72,23 @@ else
>  	machine = get_option('machine')
>  endif
>  
> +if platform == 'native'
> +	if machine == 'auto'
> +		machine = 'native'
> +	endif
> +elif platform == 'generic'
> +	if machine == 'auto'
> +		machine = 'default'
> +	endif
> +endif
> +
> +if machine == 'default'
>  # machine type 'default' is special, it defaults to the per arch agreed common
>  # minimal baseline needed for DPDK.
>  # 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 host_machine.cpu_family().startswith('x86')
>  		# matches the old pre-meson build systems default
>  		machine = 'corei7'
> diff --git a/meson_options.txt b/meson_options.txt
> index e384e6dbb..1a5e47fd3 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
>  	description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
>  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')
> +option('machine', type: 'string', value: 'auto',
> +	description: 'set the target machine type/ISA')
>  option('max_ethports', type: 'integer', value: 32,
>  	description: 'maximum number of Ethernet devices')
>  option('max_lcores', type: 'integer', value: 128,
>  	description: 'maximum number of cores/threads supported by EAL')
>  option('max_numa_nodes', type: 'integer', value: 4,
>  	description: 'maximum number of NUMA nodes supported by EAL')
> +option('platform', type: 'string', value: 'generic',
> +	description: 'Platform to build for, either "native" or "generic".')

As well as this short description option, I think we need more
comprehensive coverage of this option in the docs. Presumably for ARM
systems this will have other options for various SOC's rather than just
generic/native?

/Bruce

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: add platform meson option
  2020-11-26 16:02 ` Bruce Richardson
@ 2020-11-27  8:31   ` Juraj Linkeš
  2020-11-27 14:07     ` Bruce Richardson
  0 siblings, 1 reply; 16+ messages in thread
From: Juraj Linkeš @ 2020-11-27  8:31 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: thomas, Honnappa.Nagarahalli, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, November 26, 2020 5:03 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v1] build: add platform meson option
> 
> On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> > The current meson option 'machine' should only specify the ISA, which
> > is not sufficient for Arm, where setting ISA implies other setting as well.
> > Add a new meson option, 'platform', which differentiates the type of
> > the build (native/generic) and sets machine accordingly, unless the
> > user chooses to override it.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > ---
> >  config/arm/meson.build |  2 +-
> >  config/meson.build     | 14 +++++++++++++-
> >  meson_options.txt      |  6 ++++--
> >  3 files changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > 42b4e43c7..ac680956f 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -6,7 +6,7 @@
> >  march_opt = '-march=@0@'.format(machine)
> >
> >  arm_force_native_march = false
> > -arm_force_default_march = (machine == 'default')
> > +arm_force_default_march = (platform == 'generic')
> >
> >  flags_common_default = [
> >  	# Accelarate rte_memcpy. Be sure to run unit test
> > (memcpy_perf_autotest) diff --git a/config/meson.build
> > b/config/meson.build index c02802c18..41d32e63e 100644
> > --- a/config/meson.build
> > +++ b/config/meson.build
> > @@ -63,6 +63,8 @@ if not is_windows
> >  			pmd_subdir_opt)
> >  endif
> >
> > +platform = get_option('platform')
> > +
> >  # set the machine type and cflags for it  if meson.is_cross_build()
> >  	machine = host_machine.cpu()
> > @@ -70,13 +72,23 @@ else
> >  	machine = get_option('machine')
> >  endif
> >
> > +if platform == 'native'
> > +	if machine == 'auto'
> > +		machine = 'native'
> > +	endif
> > +elif platform == 'generic'
> > +	if machine == 'auto'
> > +		machine = 'default'
> > +	endif
> > +endif
> > +
> > +if machine == 'default'
> >  # machine type 'default' is special, it defaults to the per arch
> > agreed common  # minimal baseline needed for DPDK.
> >  # 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 host_machine.cpu_family().startswith('x86')
> >  		# matches the old pre-meson build systems default
> >  		machine = 'corei7'
> > diff --git a/meson_options.txt b/meson_options.txt index
> > e384e6dbb..1a5e47fd3 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
> >  	description: 'Path to the kernel for building kernel modules.
> > Headers must be in $kernel_dir/build. Modules will be installed in
> > $DEST_DIR/$kernel_dir/extra/dpdk.')
> >  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')
> > +option('machine', type: 'string', value: 'auto',
> > +	description: 'set the target machine type/ISA')
> >  option('max_ethports', type: 'integer', value: 32,
> >  	description: 'maximum number of Ethernet devices')
> > option('max_lcores', type: 'integer', value: 128,
> >  	description: 'maximum number of cores/threads supported by EAL')
> > option('max_numa_nodes', type: 'integer', value: 4,
> >  	description: 'maximum number of NUMA nodes supported by EAL')
> > +option('platform', type: 'string', value: 'generic',
> > +	description: 'Platform to build for, either "native" or "generic".')
> 
> As well as this short description option, I think we need more comprehensive
> coverage of this option in the docs.

Where should this be documented? In doc/guides/linux_gsg/build_dpdk.rst or somewhere else?

> Presumably for ARM systems this will have
> other options for various SOC's rather than just generic/native?
> 

Yes, I'm planning on using the platform option for this. Since we've separated the changes into their own patch sets, only this small change is in this patch set. It'll affect how we're doing the automatic numa/core detection as well as how we're choosic which Arm SoC to build for, but those changes are in different patch sets.

I mainly want feedback on naming - I like 'platform' and I don't think we need to rename 'machine', maybe just document it better - and on the overall idea. I think this is in line with what you had in mind, right?

> /Bruce


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: add platform meson option
  2020-11-27  8:31   ` Juraj Linkeš
@ 2020-11-27 14:07     ` Bruce Richardson
  2020-12-23 11:23       ` Juraj Linkeš
  0 siblings, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2020-11-27 14:07 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: thomas, Honnappa.Nagarahalli, dev

On Fri, Nov 27, 2020 at 08:31:47AM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Thursday, November 26, 2020 5:03 PM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> > Subject: Re: [RFC PATCH v1] build: add platform meson option
> > 
> > On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> > > The current meson option 'machine' should only specify the ISA, which
> > > is not sufficient for Arm, where setting ISA implies other setting as well.
> > > Add a new meson option, 'platform', which differentiates the type of
> > > the build (native/generic) and sets machine accordingly, unless the
> > > user chooses to override it.
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > ---
> > >  config/arm/meson.build |  2 +-
> > >  config/meson.build     | 14 +++++++++++++-
> > >  meson_options.txt      |  6 ++++--
> > >  3 files changed, 18 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > 42b4e43c7..ac680956f 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -6,7 +6,7 @@
> > >  march_opt = '-march=@0@'.format(machine)
> > >
> > >  arm_force_native_march = false
> > > -arm_force_default_march = (machine == 'default')
> > > +arm_force_default_march = (platform == 'generic')
> > >
> > >  flags_common_default = [
> > >  	# Accelarate rte_memcpy. Be sure to run unit test
> > > (memcpy_perf_autotest) diff --git a/config/meson.build
> > > b/config/meson.build index c02802c18..41d32e63e 100644
> > > --- a/config/meson.build
> > > +++ b/config/meson.build
> > > @@ -63,6 +63,8 @@ if not is_windows
> > >  			pmd_subdir_opt)
> > >  endif
> > >
> > > +platform = get_option('platform')
> > > +
> > >  # set the machine type and cflags for it  if meson.is_cross_build()
> > >  	machine = host_machine.cpu()
> > > @@ -70,13 +72,23 @@ else
> > >  	machine = get_option('machine')
> > >  endif
> > >
> > > +if platform == 'native'
> > > +	if machine == 'auto'
> > > +		machine = 'native'
> > > +	endif
> > > +elif platform == 'generic'
> > > +	if machine == 'auto'
> > > +		machine = 'default'
> > > +	endif
> > > +endif
> > > +
> > > +if machine == 'default'
> > >  # machine type 'default' is special, it defaults to the per arch
> > > agreed common  # minimal baseline needed for DPDK.
> > >  # 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 host_machine.cpu_family().startswith('x86')
> > >  		# matches the old pre-meson build systems default
> > >  		machine = 'corei7'
> > > diff --git a/meson_options.txt b/meson_options.txt index
> > > e384e6dbb..1a5e47fd3 100644
> > > --- a/meson_options.txt
> > > +++ b/meson_options.txt
> > > @@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
> > >  	description: 'Path to the kernel for building kernel modules.
> > > Headers must be in $kernel_dir/build. Modules will be installed in
> > > $DEST_DIR/$kernel_dir/extra/dpdk.')
> > >  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')
> > > +option('machine', type: 'string', value: 'auto',
> > > +	description: 'set the target machine type/ISA')
> > >  option('max_ethports', type: 'integer', value: 32,
> > >  	description: 'maximum number of Ethernet devices')
> > > option('max_lcores', type: 'integer', value: 128,
> > >  	description: 'maximum number of cores/threads supported by EAL')
> > > option('max_numa_nodes', type: 'integer', value: 4,
> > >  	description: 'maximum number of NUMA nodes supported by EAL')
> > > +option('platform', type: 'string', value: 'generic',
> > > +	description: 'Platform to build for, either "native" or "generic".')
> > 
> > As well as this short description option, I think we need more comprehensive
> > coverage of this option in the docs.
> 
> Where should this be documented? In doc/guides/linux_gsg/build_dpdk.rst or somewhere else?
> 
I'll need to look into this, but I think that we need to expand that
section to cover in more detail all build options as we rework them.

> > Presumably for ARM systems this will have
> > other options for various SOC's rather than just generic/native?
> > 
> 
> Yes, I'm planning on using the platform option for this. Since we've separated the changes into their own patch sets, only this small change is in this patch set. It'll affect how we're doing the automatic numa/core detection as well as how we're choosic which Arm SoC to build for, but those changes are in different patch sets.
> 
> I mainly want feedback on naming - I like 'platform' and I don't think we need to rename 'machine', maybe just document it better - and on the overall idea. I think this is in line with what you had in mind, right?
> 
Yes this is in line with what I had in mind, thanks for doing the work.

I'm still of a mind to rename "machine", since it's a very ambiguous term.
I think something like "cpu_instruction_set" would be a lot clearer. With
choice of a good name, the amount of documentation needed around it is much
reduced.

We should probably get feedback from a number of people before making a
decision.

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v1] build: add platform meson option
  2020-11-27 14:07     ` Bruce Richardson
@ 2020-12-23 11:23       ` Juraj Linkeš
  0 siblings, 0 replies; 16+ messages in thread
From: Juraj Linkeš @ 2020-12-23 11:23 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: thomas, Honnappa.Nagarahalli, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Friday, November 27, 2020 3:07 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v1] build: add platform meson option
> 
> On Fri, Nov 27, 2020 at 08:31:47AM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Thursday, November 26, 2020 5:03 PM
> > > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com;
> dev@dpdk.org
> > > Subject: Re: [RFC PATCH v1] build: add platform meson option
> > >
> > > On Thu, Nov 26, 2020 at 04:47:29PM +0100, Juraj Linkeš wrote:
> > > > The current meson option 'machine' should only specify the ISA,
> > > > which is not sufficient for Arm, where setting ISA implies other setting as
> well.
> > > > Add a new meson option, 'platform', which differentiates the type
> > > > of the build (native/generic) and sets machine accordingly, unless
> > > > the user chooses to override it.
> > > >
> > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > ---
> > > >  config/arm/meson.build |  2 +-
> > > >  config/meson.build     | 14 +++++++++++++-
> > > >  meson_options.txt      |  6 ++++--
> > > >  3 files changed, 18 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > > 42b4e43c7..ac680956f 100644
> > > > --- a/config/arm/meson.build
> > > > +++ b/config/arm/meson.build
> > > > @@ -6,7 +6,7 @@
> > > >  march_opt = '-march=@0@'.format(machine)
> > > >
> > > >  arm_force_native_march = false
> > > > -arm_force_default_march = (machine == 'default')
> > > > +arm_force_default_march = (platform == 'generic')
> > > >
> > > >  flags_common_default = [
> > > >  	# Accelarate rte_memcpy. Be sure to run unit test
> > > > (memcpy_perf_autotest) diff --git a/config/meson.build
> > > > b/config/meson.build index c02802c18..41d32e63e 100644
> > > > --- a/config/meson.build
> > > > +++ b/config/meson.build
> > > > @@ -63,6 +63,8 @@ if not is_windows
> > > >  			pmd_subdir_opt)
> > > >  endif
> > > >
> > > > +platform = get_option('platform')
> > > > +
> > > >  # set the machine type and cflags for it  if meson.is_cross_build()
> > > >  	machine = host_machine.cpu()
> > > > @@ -70,13 +72,23 @@ else
> > > >  	machine = get_option('machine')
> > > >  endif
> > > >
> > > > +if platform == 'native'
> > > > +	if machine == 'auto'
> > > > +		machine = 'native'
> > > > +	endif
> > > > +elif platform == 'generic'
> > > > +	if machine == 'auto'
> > > > +		machine = 'default'
> > > > +	endif
> > > > +endif
> > > > +
> > > > +if machine == 'default'
> > > >  # machine type 'default' is special, it defaults to the per arch
> > > > agreed common  # minimal baseline needed for DPDK.
> > > >  # 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 host_machine.cpu_family().startswith('x86')
> > > >  		# matches the old pre-meson build systems default
> > > >  		machine = 'corei7'
> > > > diff --git a/meson_options.txt b/meson_options.txt index
> > > > e384e6dbb..1a5e47fd3 100644
> > > > --- a/meson_options.txt
> > > > +++ b/meson_options.txt
> > > > @@ -20,14 +20,16 @@ option('kernel_dir', type: 'string', value: '',
> > > >  	description: 'Path to the kernel for building kernel modules.
> > > > Headers must be in $kernel_dir/build. Modules will be installed in
> > > > $DEST_DIR/$kernel_dir/extra/dpdk.')
> > > >  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')
> > > > +option('machine', type: 'string', value: 'auto',
> > > > +	description: 'set the target machine type/ISA')
> > > >  option('max_ethports', type: 'integer', value: 32,
> > > >  	description: 'maximum number of Ethernet devices')
> > > > option('max_lcores', type: 'integer', value: 128,
> > > >  	description: 'maximum number of cores/threads supported by EAL')
> > > > option('max_numa_nodes', type: 'integer', value: 4,
> > > >  	description: 'maximum number of NUMA nodes supported by EAL')
> > > > +option('platform', type: 'string', value: 'generic',
> > > > +	description: 'Platform to build for, either "native" or
> > > > +"generic".')
> > >
> > > As well as this short description option, I think we need more
> > > comprehensive coverage of this option in the docs.
> >
> > Where should this be documented? In doc/guides/linux_gsg/build_dpdk.rst or
> somewhere else?
> >
> I'll need to look into this, but I think that we need to expand that section to
> cover in more detail all build options as we rework them.
> 

Any news on this? If we want to document this in just doc/guides/linux_gsg/build_dpdk.rst I can propose some changes.

> > > Presumably for ARM systems this will have other options for various
> > > SOC's rather than just generic/native?
> > >
> >
> > Yes, I'm planning on using the platform option for this. Since we've separated
> the changes into their own patch sets, only this small change is in this patch set.
> It'll affect how we're doing the automatic numa/core detection as well as how
> we're choosic which Arm SoC to build for, but those changes are in different
> patch sets.
> >
> > I mainly want feedback on naming - I like 'platform' and I don't think we need
> to rename 'machine', maybe just document it better - and on the overall idea. I
> think this is in line with what you had in mind, right?
> >
> Yes this is in line with what I had in mind, thanks for doing the work.
> 
> I'm still of a mind to rename "machine", since it's a very ambiguous term.
> I think something like "cpu_instruction_set" would be a lot clearer. With choice
> of a good name, the amount of documentation needed around it is much
> reduced.
> 

I wanted to keep machine mainly for backwards compatibilty, but we can do that anyway, as in keep machine for a release a two with a deprecation notice and then remove it while using a differently named (but same in function) option.

What's were doing with the option is setting the appropriate "machine args", as the variable  is called. For x86, that means setting -march (which is just the instruction set according to the docs [0]), for ppc it sets -mcpu and -mtune (architecture type, register usage, and instruction scheduling parameters according to [1]) and we're not using it to set machine args for arm at all (we are setting machine args based on other criteria; when this new option along with platform are in effect, we can think about using cpu_instruction_set somehow).

With this in mind, cpu_instruction_set seems like the best candidate. I'll submit a new version with both cpu_instruction_set and machine if there aren't any objections in some time.

[0] https://gcc.gnu.org/onlinedocs/gcc-8.4.0/gcc/x86-Options.html#x86-Options
[1] https://gcc.gnu.org/onlinedocs/gcc-8.4.0/gcc/RS_002f6000-and-PowerPC-Options.html#RS_002f6000-and-PowerPC-Options

> We should probably get feedback from a number of people before making a
> decision.
> 

Unfortunately we didn't get any feedback. Do you have anyone in particular in mind?

> Regards,
> /Bruce


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2020-11-26 15:47 [dpdk-dev] [RFC PATCH v1] build: add platform meson option Juraj Linkeš
  2020-11-26 16:02 ` Bruce Richardson
@ 2021-01-04 11:52 ` Juraj Linkeš
  2021-01-04 11:59   ` Juraj Linkeš
  1 sibling, 1 reply; 16+ messages in thread
From: Juraj Linkeš @ 2021-01-04 11:52 UTC (permalink / raw)
  To: thomas, bruce.richardson, Honnappa.Nagarahalli; +Cc: dev, Juraj Linkeš

The current meson option 'machine' should only specify the ISA, which is
not sufficient for Arm, where setting ISA implies other setting as well.
Add a new meson option, 'platform', which differentiates the type of the
build (native/generic) and sets machine accordingly, unless the user
chooses to override it.
The 'machine' option also doesn't describe very well what it sets, so
introduce a new option 'cpu_instruction_set', but keep 'machine' for
backward compatibility.
These two new variables, taken together, achieve what 'machine' was
setting per architecture - setting the ISA in x86 build and setting
native/default 'build type' in aarch64 build - is now properly being set
for all architectures in a uniform manner.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 config/arm/meson.build        |  4 +--
 config/meson.build            | 47 +++++++++++++++++++++++++----------
 devtools/test-meson-builds.sh |  9 ++++---
 meson_options.txt             |  8 ++++--
 4 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 42b4e43c7..6b09a74a7 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -3,10 +3,10 @@
 # Copyright(c) 2017 Cavium, Inc
 
 # for checking defines we need to use the correct compiler flags
-march_opt = '-march=@0@'.format(machine)
+march_opt = '-march=@0@'.format(cpu_instruction_set)
 
 arm_force_native_march = false
-arm_force_default_march = (machine == 'default')
+arm_force_default_march = (platform == 'generic')
 
 flags_common_default = [
 	# Accelarate rte_memcpy. Be sure to run unit test (memcpy_perf_autotest)
diff --git a/config/meson.build b/config/meson.build
index a3154e29c..647116513 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -63,42 +63,63 @@ if not is_windows
 			pmd_subdir_opt)
 endif
 
-# set the machine type and cflags for it
+platform = get_option('platform')
+
+# set the cpu_instruction_set type and cflags for it
 if meson.is_cross_build()
-	machine = host_machine.cpu()
+	cpu_instruction_set = host_machine.cpu()
 else
-	machine = get_option('machine')
+	cpu_instruction_set = get_option('cpu_instruction_set')
+	if get_option('machine') != 'auto'
+		warning('The "machine" option is deprecated. ' +
+		        'Please use "cpu_instruction_set" instead.')
+		if cpu_instruction_set != 'auto'
+			error('Setting both "machine" and ' +
+			      '"cpu_instruction_set" is unsupported.')
+		endif
+		cpu_instruction_set = get_option('machine')
+	endif
+endif
+
+if platform == 'native'
+	if cpu_instruction_set == 'auto'
+		cpu_instruction_set = 'native'
+	endif
+elif platform == 'generic'
+	if cpu_instruction_set == 'auto'
+		cpu_instruction_set = 'default'
+	endif
 endif
 
-# machine type 'default' is special, it defaults to the per arch agreed common
+if cpu_instruction_set == 'default'
+# cpu_instruction_set type 'default' is special, it defaults to the per arch agreed common
 # minimal baseline needed for DPDK.
 # 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 host_machine.cpu_family().startswith('x86')
 		# matches the old pre-meson build systems default
-		machine = 'corei7'
+		cpu_instruction_set = 'corei7'
 	elif host_machine.cpu_family().startswith('arm')
-		machine = 'armv7-a'
+		cpu_instruction_set = 'armv7-a'
 	elif host_machine.cpu_family().startswith('aarch')
 		# arm64 manages defaults in config/arm/meson.build
-		machine = 'default'
+		cpu_instruction_set = 'default'
 	elif host_machine.cpu_family().startswith('ppc')
-		machine = 'power8'
+		cpu_instruction_set = 'power8'
 	endif
 endif
 
-dpdk_conf.set('RTE_MACHINE', machine)
+dpdk_conf.set('RTE_MACHINE', cpu_instruction_set)
 machine_args = []
 
 # ppc64 does not support -march= at all, use -mcpu and -mtune for that
 if host_machine.cpu_family().startswith('ppc')
-	machine_args += '-mcpu=' + machine
-	machine_args += '-mtune=' + machine
+	machine_args += '-mcpu=' + cpu_instruction_set
+	machine_args += '-mtune=' + cpu_instruction_set
 else
-	machine_args += '-march=' + machine
+	machine_args += '-march=' + cpu_instruction_set
 endif
 
 toolchain = cc.get_id()
diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 7280b7a93..33845d998 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -211,11 +211,12 @@ done
 # test compilation with minimal x86 instruction set
 # Set the install path for libraries to "lib" explicitly to prevent problems
 # with pkg-config prefixes if installed in "lib/x86_64-linux-gnu" later.
-default_machine='nehalem'
-if ! check_cc_flags "-march=$default_machine" ; then
-	default_machine='corei7'
+default_isa='nehalem'
+if ! check_cc_flags "-march=$default_isa" ; then
+	default_isa='corei7'
 fi
-build build-x86-default cc -Dlibdir=lib -Dmachine=$default_machine $use_shared
+build build-x86-default cc -Dlibdir=lib -Dcpu_instruction_set=$default_isa \
+      $use_shared
 
 # 32-bit with default compiler
 if check_cc_flags '-m32' ; then
diff --git a/meson_options.txt b/meson_options.txt
index e384e6dbb..9db03a87d 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -1,5 +1,7 @@
 # Please keep these options sorted alphabetically.
 
+option('cpu_instruction_set', type: 'string', value: 'auto',
+	description: 'Set the target machine ISA (instruction set architecture)/')
 option('disable_drivers', type: 'string', value: '',
 	description: 'Comma-separated list of drivers to explicitly disable.')
 option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
@@ -20,14 +22,16 @@ option('kernel_dir', type: 'string', value: '',
 	description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
 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')
+option('machine', type: 'string', value: 'auto',
+	description: 'set the target machine type/ISA')
 option('max_ethports', type: 'integer', value: 32,
 	description: 'maximum number of Ethernet devices')
 option('max_lcores', type: 'integer', value: 128,
 	description: 'maximum number of cores/threads supported by EAL')
 option('max_numa_nodes', type: 'integer', value: 4,
 	description: 'maximum number of NUMA nodes supported by EAL')
+option('platform', type: 'string', value: 'generic',
+	description: 'Platform to build for, either "native" or "generic".')
 option('enable_trace_fp', type: 'boolean', value: false,
 	description: 'enable fast path trace points.')
 option('tests', type: 'boolean', value: true,
-- 
2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-01-04 11:52 ` [dpdk-dev] [RFC PATCH v2] " Juraj Linkeš
@ 2021-01-04 11:59   ` Juraj Linkeš
  2021-01-05 22:17     ` David Christensen
  0 siblings, 1 reply; 16+ messages in thread
From: Juraj Linkeš @ 2021-01-04 11:59 UTC (permalink / raw)
  To: Juraj Linkeš, thomas, bruce.richardson, Honnappa.Nagarahalli, drc
  Cc: dev

Apologies, I meant to add David as well.

> -----Original Message-----
> From: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Sent: Monday, January 4, 2021 12:52 PM
> To: thomas@monjalon.net; bruce.richardson@intel.com;
> Honnappa.Nagarahalli@arm.com
> Cc: dev@dpdk.org; Juraj Linkeš <juraj.linkes@pantheon.tech>
> Subject: [RFC PATCH v2] build: add platform meson option
> 
> The current meson option 'machine' should only specify the ISA, which is not
> sufficient for Arm, where setting ISA implies other setting as well.
> Add a new meson option, 'platform', which differentiates the type of the build
> (native/generic) and sets machine accordingly, unless the user chooses to
> override it.
> The 'machine' option also doesn't describe very well what it sets, so introduce a
> new option 'cpu_instruction_set', but keep 'machine' for backward
> compatibility.
> These two new variables, taken together, achieve what 'machine' was setting
> per architecture - setting the ISA in x86 build and setting native/default 'build
> type' in aarch64 build - is now properly being set for all architectures in a
> uniform manner.
> 
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
>  config/arm/meson.build        |  4 +--
>  config/meson.build            | 47 +++++++++++++++++++++++++----------
>  devtools/test-meson-builds.sh |  9 ++++---
>  meson_options.txt             |  8 ++++--
>  4 files changed, 47 insertions(+), 21 deletions(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> 42b4e43c7..6b09a74a7 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -3,10 +3,10 @@
>  # Copyright(c) 2017 Cavium, Inc
> 
>  # for checking defines we need to use the correct compiler flags -march_opt = '-
> march=@0@'.format(machine)
> +march_opt = '-march=@0@'.format(cpu_instruction_set)
> 
>  arm_force_native_march = false
> -arm_force_default_march = (machine == 'default')
> +arm_force_default_march = (platform == 'generic')
> 
>  flags_common_default = [
>  	# Accelarate rte_memcpy. Be sure to run unit test
> (memcpy_perf_autotest) diff --git a/config/meson.build b/config/meson.build
> index a3154e29c..647116513 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -63,42 +63,63 @@ if not is_windows
>  			pmd_subdir_opt)
>  endif
> 
> -# set the machine type and cflags for it
> +platform = get_option('platform')
> +
> +# set the cpu_instruction_set type and cflags for it
>  if meson.is_cross_build()
> -	machine = host_machine.cpu()
> +	cpu_instruction_set = host_machine.cpu()
>  else
> -	machine = get_option('machine')
> +	cpu_instruction_set = get_option('cpu_instruction_set')
> +	if get_option('machine') != 'auto'
> +		warning('The "machine" option is deprecated. ' +
> +		        'Please use "cpu_instruction_set" instead.')
> +		if cpu_instruction_set != 'auto'
> +			error('Setting both "machine" and ' +
> +			      '"cpu_instruction_set" is unsupported.')
> +		endif
> +		cpu_instruction_set = get_option('machine')
> +	endif
> +endif
> +
> +if platform == 'native'
> +	if cpu_instruction_set == 'auto'
> +		cpu_instruction_set = 'native'
> +	endif
> +elif platform == 'generic'
> +	if cpu_instruction_set == 'auto'
> +		cpu_instruction_set = 'default'
> +	endif
>  endif
> 
> -# machine type 'default' is special, it defaults to the per arch agreed common
> +if cpu_instruction_set == 'default'
> +# cpu_instruction_set type 'default' is special, it defaults to the per
> +arch agreed common
>  # minimal baseline needed for DPDK.
>  # 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 host_machine.cpu_family().startswith('x86')
>  		# matches the old pre-meson build systems default
> -		machine = 'corei7'
> +		cpu_instruction_set = 'corei7'
>  	elif host_machine.cpu_family().startswith('arm')
> -		machine = 'armv7-a'
> +		cpu_instruction_set = 'armv7-a'
>  	elif host_machine.cpu_family().startswith('aarch')
>  		# arm64 manages defaults in config/arm/meson.build
> -		machine = 'default'
> +		cpu_instruction_set = 'default'
>  	elif host_machine.cpu_family().startswith('ppc')
> -		machine = 'power8'
> +		cpu_instruction_set = 'power8'
>  	endif
>  endif
> 
> -dpdk_conf.set('RTE_MACHINE', machine)
> +dpdk_conf.set('RTE_MACHINE', cpu_instruction_set)
>  machine_args = []
> 
>  # ppc64 does not support -march= at all, use -mcpu and -mtune for that  if
> host_machine.cpu_family().startswith('ppc')
> -	machine_args += '-mcpu=' + machine
> -	machine_args += '-mtune=' + machine
> +	machine_args += '-mcpu=' + cpu_instruction_set
> +	machine_args += '-mtune=' + cpu_instruction_set
>  else
> -	machine_args += '-march=' + machine
> +	machine_args += '-march=' + cpu_instruction_set
>  endif
> 
>  toolchain = cc.get_id()
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh index
> 7280b7a93..33845d998 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -211,11 +211,12 @@ done
>  # test compilation with minimal x86 instruction set  # Set the install path for
> libraries to "lib" explicitly to prevent problems  # with pkg-config prefixes if
> installed in "lib/x86_64-linux-gnu" later.
> -default_machine='nehalem'
> -if ! check_cc_flags "-march=$default_machine" ; then
> -	default_machine='corei7'
> +default_isa='nehalem'
> +if ! check_cc_flags "-march=$default_isa" ; then
> +	default_isa='corei7'
>  fi
> -build build-x86-default cc -Dlibdir=lib -Dmachine=$default_machine
> $use_shared
> +build build-x86-default cc -Dlibdir=lib -Dcpu_instruction_set=$default_isa \
> +      $use_shared
> 
>  # 32-bit with default compiler
>  if check_cc_flags '-m32' ; then
> diff --git a/meson_options.txt b/meson_options.txt index
> e384e6dbb..9db03a87d 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -1,5 +1,7 @@
>  # Please keep these options sorted alphabetically.
> 
> +option('cpu_instruction_set', type: 'string', value: 'auto',
> +	description: 'Set the target machine ISA (instruction set
> +architecture)/')
>  option('disable_drivers', type: 'string', value: '',
>  	description: 'Comma-separated list of drivers to explicitly disable.')
> option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>',
> @@ -20,14 +22,16 @@ option('kernel_dir', type: 'string', value: '',
>  	description: 'Path to the kernel for building kernel modules. Headers
> must be in $kernel_dir/build. Modules will be installed in
> $DEST_DIR/$kernel_dir/extra/dpdk.')
>  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')
> +option('machine', type: 'string', value: 'auto',
> +	description: 'set the target machine type/ISA')
>  option('max_ethports', type: 'integer', value: 32,
>  	description: 'maximum number of Ethernet devices')
> option('max_lcores', type: 'integer', value: 128,
>  	description: 'maximum number of cores/threads supported by EAL')
> option('max_numa_nodes', type: 'integer', value: 4,
>  	description: 'maximum number of NUMA nodes supported by EAL')
> +option('platform', type: 'string', value: 'generic',
> +	description: 'Platform to build for, either "native" or "generic".')
>  option('enable_trace_fp', type: 'boolean', value: false,
>  	description: 'enable fast path trace points.')  option('tests', type:
> 'boolean', value: true,
> --
> 2.20.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-01-04 11:59   ` Juraj Linkeš
@ 2021-01-05 22:17     ` David Christensen
  2021-01-06 14:42       ` Bruce Richardson
  0 siblings, 1 reply; 16+ messages in thread
From: David Christensen @ 2021-01-05 22:17 UTC (permalink / raw)
  To: Juraj Linkeš, thomas, bruce.richardson, Honnappa.Nagarahalli; +Cc: dev

>> The current meson option 'machine' should only specify the ISA, which is not
>> sufficient for Arm, where setting ISA implies other setting as well.
>> Add a new meson option, 'platform', which differentiates the type of the build
>> (native/generic) and sets machine accordingly, unless the user chooses to
>> override it.
>> The 'machine' option also doesn't describe very well what it sets, so introduce a
>> new option 'cpu_instruction_set', but keep 'machine' for backward
>> compatibility.
>> These two new variables, taken together, achieve what 'machine' was setting
>> per architecture - setting the ISA in x86 build and setting native/default 'build
>> type' in aarch64 build - is now properly being set for all architectures in a
>> uniform manner.
>>
>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>> ---
>>   config/arm/meson.build        |  4 +--
>>   config/meson.build            | 47 +++++++++++++++++++++++++----------
>>   devtools/test-meson-builds.sh |  9 ++++---
>>   meson_options.txt             |  8 ++++--
>>   4 files changed, 47 insertions(+), 21 deletions(-)
>>
>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>> 42b4e43c7..6b09a74a7 100644
>> --- a/config/arm/meson.build
>> +++ b/config/arm/meson.build
>> @@ -3,10 +3,10 @@
>>   # Copyright(c) 2017 Cavium, Inc
>>
>>   # for checking defines we need to use the correct compiler flags -march_opt = '-
>> march=@0@'.format(machine)
>> +march_opt = '-march=@0@'.format(cpu_instruction_set)
>>
>>   arm_force_native_march = false
>> -arm_force_default_march = (machine == 'default')
>> +arm_force_default_march = (platform == 'generic')
>>
>>   flags_common_default = [
>>   	# Accelarate rte_memcpy. Be sure to run unit test
>> (memcpy_perf_autotest) diff --git a/config/meson.build b/config/meson.build
>> index a3154e29c..647116513 100644
>> --- a/config/meson.build
>> +++ b/config/meson.build
>> @@ -63,42 +63,63 @@ if not is_windows
>>   			pmd_subdir_opt)
>>   endif
>>
>> -# set the machine type and cflags for it
>> +platform = get_option('platform')
>> +
>> +# set the cpu_instruction_set type and cflags for it
>>   if meson.is_cross_build()
>> -	machine = host_machine.cpu()
>> +	cpu_instruction_set = host_machine.cpu()
>>   else
>> -	machine = get_option('machine')
>> +	cpu_instruction_set = get_option('cpu_instruction_set')
>> +	if get_option('machine') != 'auto'
>> +		warning('The "machine" option is deprecated. ' +
>> +		        'Please use "cpu_instruction_set" instead.')
>> +		if cpu_instruction_set != 'auto'
>> +			error('Setting both "machine" and ' +
>> +			      '"cpu_instruction_set" is unsupported.')
>> +		endif
>> +		cpu_instruction_set = get_option('machine')
>> +	endif
>> +endif
>> +
>> +if platform == 'native'
>> +	if cpu_instruction_set == 'auto'
>> +		cpu_instruction_set = 'native'
>> +	endif
>> +elif platform == 'generic'
>> +	if cpu_instruction_set == 'auto'
>> +		cpu_instruction_set = 'default'
>> +	endif
>>   endif
>>
>> -# machine type 'default' is special, it defaults to the per arch agreed common
>> +if cpu_instruction_set == 'default'
>> +# cpu_instruction_set type 'default' is special, it defaults to the per
>> +arch agreed common
>>   # minimal baseline needed for DPDK.
>>   # 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 host_machine.cpu_family().startswith('x86')
>>   		# matches the old pre-meson build systems default
>> -		machine = 'corei7'
>> +		cpu_instruction_set = 'corei7'
>>   	elif host_machine.cpu_family().startswith('arm')
>> -		machine = 'armv7-a'
>> +		cpu_instruction_set = 'armv7-a'
>>   	elif host_machine.cpu_family().startswith('aarch')
>>   		# arm64 manages defaults in config/arm/meson.build
>> -		machine = 'default'
>> +		cpu_instruction_set = 'default'
>>   	elif host_machine.cpu_family().startswith('ppc')
>> -		machine = 'power8'
>> +		cpu_instruction_set = 'power8'
>>   	endif
>>   endif

This change forces the build on a P9 system to use the P8 instruction 
set.  Prior to this change the "native" machine type was used which 
resulted in P9 instructions when built on a P9 system.  How do I force 
the build to use the power9 instruction set in this case?

Dave

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-01-05 22:17     ` David Christensen
@ 2021-01-06 14:42       ` Bruce Richardson
  2021-02-19  9:11         ` Juraj Linkeš
  0 siblings, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2021-01-06 14:42 UTC (permalink / raw)
  To: David Christensen; +Cc: Juraj Linkeš, thomas, Honnappa.Nagarahalli, dev

On Tue, Jan 05, 2021 at 02:17:44PM -0800, David Christensen wrote:
> > > The current meson option 'machine' should only specify the ISA, which is not
> > > sufficient for Arm, where setting ISA implies other setting as well.
> > > Add a new meson option, 'platform', which differentiates the type of the build
> > > (native/generic) and sets machine accordingly, unless the user chooses to
> > > override it.
> > > The 'machine' option also doesn't describe very well what it sets, so introduce a
> > > new option 'cpu_instruction_set', but keep 'machine' for backward
> > > compatibility.
> > > These two new variables, taken together, achieve what 'machine' was setting
> > > per architecture - setting the ISA in x86 build and setting native/default 'build
> > > type' in aarch64 build - is now properly being set for all architectures in a
> > > uniform manner.
> > > 
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > ---
> > >   config/arm/meson.build        |  4 +--
> > >   config/meson.build            | 47 +++++++++++++++++++++++++----------
> > >   devtools/test-meson-builds.sh |  9 ++++---
> > >   meson_options.txt             |  8 ++++--
> > >   4 files changed, 47 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > 42b4e43c7..6b09a74a7 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -3,10 +3,10 @@
> > >   # Copyright(c) 2017 Cavium, Inc
> > > 
> > >   # for checking defines we need to use the correct compiler flags -march_opt = '-
> > > march=@0@'.format(machine)
> > > +march_opt = '-march=@0@'.format(cpu_instruction_set)
> > > 
> > >   arm_force_native_march = false
> > > -arm_force_default_march = (machine == 'default')
> > > +arm_force_default_march = (platform == 'generic')
> > > 
> > >   flags_common_default = [
> > >   	# Accelarate rte_memcpy. Be sure to run unit test
> > > (memcpy_perf_autotest) diff --git a/config/meson.build b/config/meson.build
> > > index a3154e29c..647116513 100644
> > > --- a/config/meson.build
> > > +++ b/config/meson.build
> > > @@ -63,42 +63,63 @@ if not is_windows
> > >   			pmd_subdir_opt)
> > >   endif
> > > 
> > > -# set the machine type and cflags for it
> > > +platform = get_option('platform')
> > > +
> > > +# set the cpu_instruction_set type and cflags for it
> > >   if meson.is_cross_build()
> > > -	machine = host_machine.cpu()
> > > +	cpu_instruction_set = host_machine.cpu()
> > >   else
> > > -	machine = get_option('machine')
> > > +	cpu_instruction_set = get_option('cpu_instruction_set')
> > > +	if get_option('machine') != 'auto'
> > > +		warning('The "machine" option is deprecated. ' +
> > > +		        'Please use "cpu_instruction_set" instead.')
> > > +		if cpu_instruction_set != 'auto'
> > > +			error('Setting both "machine" and ' +
> > > +			      '"cpu_instruction_set" is unsupported.')
> > > +		endif
> > > +		cpu_instruction_set = get_option('machine')
> > > +	endif
> > > +endif
> > > +
> > > +if platform == 'native'
> > > +	if cpu_instruction_set == 'auto'
> > > +		cpu_instruction_set = 'native'
> > > +	endif
> > > +elif platform == 'generic'
> > > +	if cpu_instruction_set == 'auto'
> > > +		cpu_instruction_set = 'default'
> > > +	endif
> > >   endif
> > > 
> > > -# machine type 'default' is special, it defaults to the per arch agreed common
> > > +if cpu_instruction_set == 'default'
> > > +# cpu_instruction_set type 'default' is special, it defaults to the per
> > > +arch agreed common
> > >   # minimal baseline needed for DPDK.
> > >   # 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 host_machine.cpu_family().startswith('x86')
> > >   		# matches the old pre-meson build systems default
> > > -		machine = 'corei7'
> > > +		cpu_instruction_set = 'corei7'
> > >   	elif host_machine.cpu_family().startswith('arm')
> > > -		machine = 'armv7-a'
> > > +		cpu_instruction_set = 'armv7-a'
> > >   	elif host_machine.cpu_family().startswith('aarch')
> > >   		# arm64 manages defaults in config/arm/meson.build
> > > -		machine = 'default'
> > > +		cpu_instruction_set = 'default'
> > >   	elif host_machine.cpu_family().startswith('ppc')
> > > -		machine = 'power8'
> > > +		cpu_instruction_set = 'power8'
> > >   	endif
> > >   endif
> 
> This change forces the build on a P9 system to use the P8 instruction set.
> Prior to this change the "native" machine type was used which resulted in P9
> instructions when built on a P9 system.  How do I force the build to use the
> power9 instruction set in this case?
> 
> Dave

From looking at the patch, setting the "platform" to "native", or the
instruction_set to "native" should do this.
While I consider generic builds a good thing, I wonder if there is an
expectation that "native" is always the default build type for DPDK builds?

/Bruce

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-01-06 14:42       ` Bruce Richardson
@ 2021-02-19  9:11         ` Juraj Linkeš
  2021-02-22 21:25           ` David Christensen
  0 siblings, 1 reply; 16+ messages in thread
From: Juraj Linkeš @ 2021-02-19  9:11 UTC (permalink / raw)
  To: Bruce Richardson, David Christensen; +Cc: thomas, Honnappa.Nagarahalli, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Wednesday, January 6, 2021 3:43 PM
> To: David Christensen <drc@linux.vnet.ibm.com>
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net;
> Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v2] build: add platform meson option
> 
> On Tue, Jan 05, 2021 at 02:17:44PM -0800, David Christensen wrote:
> > > > The current meson option 'machine' should only specify the ISA,
> > > > which is not sufficient for Arm, where setting ISA implies other setting as
> well.
> > > > Add a new meson option, 'platform', which differentiates the type
> > > > of the build
> > > > (native/generic) and sets machine accordingly, unless the user
> > > > chooses to override it.
> > > > The 'machine' option also doesn't describe very well what it sets,
> > > > so introduce a new option 'cpu_instruction_set', but keep
> > > > 'machine' for backward compatibility.
> > > > These two new variables, taken together, achieve what 'machine'
> > > > was setting per architecture - setting the ISA in x86 build and
> > > > setting native/default 'build type' in aarch64 build - is now
> > > > properly being set for all architectures in a uniform manner.
> > > >
> > > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > ---
> > > >   config/arm/meson.build        |  4 +--
> > > >   config/meson.build            | 47 +++++++++++++++++++++++++----------
> > > >   devtools/test-meson-builds.sh |  9 ++++---
> > > >   meson_options.txt             |  8 ++++--
> > > >   4 files changed, 47 insertions(+), 21 deletions(-)
> > > >
> > > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > > 42b4e43c7..6b09a74a7 100644
> > > > --- a/config/arm/meson.build
> > > > +++ b/config/arm/meson.build
> > > > @@ -3,10 +3,10 @@
> > > >   # Copyright(c) 2017 Cavium, Inc
> > > >
> > > >   # for checking defines we need to use the correct compiler flags
> > > > -march_opt = '-
> > > > march=@0@'.format(machine)
> > > > +march_opt = '-march=@0@'.format(cpu_instruction_set)
> > > >
> > > >   arm_force_native_march = false
> > > > -arm_force_default_march = (machine == 'default')
> > > > +arm_force_default_march = (platform == 'generic')
> > > >
> > > >   flags_common_default = [
> > > >   	# Accelarate rte_memcpy. Be sure to run unit test
> > > > (memcpy_perf_autotest) diff --git a/config/meson.build
> > > > b/config/meson.build index a3154e29c..647116513 100644
> > > > --- a/config/meson.build
> > > > +++ b/config/meson.build
> > > > @@ -63,42 +63,63 @@ if not is_windows
> > > >   			pmd_subdir_opt)
> > > >   endif
> > > >
> > > > -# set the machine type and cflags for it
> > > > +platform = get_option('platform')
> > > > +
> > > > +# set the cpu_instruction_set type and cflags for it
> > > >   if meson.is_cross_build()
> > > > -	machine = host_machine.cpu()
> > > > +	cpu_instruction_set = host_machine.cpu()
> > > >   else
> > > > -	machine = get_option('machine')
> > > > +	cpu_instruction_set = get_option('cpu_instruction_set')
> > > > +	if get_option('machine') != 'auto'
> > > > +		warning('The "machine" option is deprecated. ' +
> > > > +		        'Please use "cpu_instruction_set" instead.')
> > > > +		if cpu_instruction_set != 'auto'
> > > > +			error('Setting both "machine" and ' +
> > > > +			      '"cpu_instruction_set" is unsupported.')
> > > > +		endif
> > > > +		cpu_instruction_set = get_option('machine')
> > > > +	endif
> > > > +endif
> > > > +
> > > > +if platform == 'native'
> > > > +	if cpu_instruction_set == 'auto'
> > > > +		cpu_instruction_set = 'native'
> > > > +	endif
> > > > +elif platform == 'generic'
> > > > +	if cpu_instruction_set == 'auto'
> > > > +		cpu_instruction_set = 'default'
> > > > +	endif
> > > >   endif
> > > >
> > > > -# machine type 'default' is special, it defaults to the per arch
> > > > agreed common
> > > > +if cpu_instruction_set == 'default'
> > > > +# cpu_instruction_set type 'default' is special, it defaults to
> > > > +the per arch agreed common
> > > >   # minimal baseline needed for DPDK.
> > > >   # 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 host_machine.cpu_family().startswith('x86')
> > > >   		# matches the old pre-meson build systems default
> > > > -		machine = 'corei7'
> > > > +		cpu_instruction_set = 'corei7'
> > > >   	elif host_machine.cpu_family().startswith('arm')
> > > > -		machine = 'armv7-a'
> > > > +		cpu_instruction_set = 'armv7-a'
> > > >   	elif host_machine.cpu_family().startswith('aarch')
> > > >   		# arm64 manages defaults in config/arm/meson.build
> > > > -		machine = 'default'
> > > > +		cpu_instruction_set = 'default'
> > > >   	elif host_machine.cpu_family().startswith('ppc')
> > > > -		machine = 'power8'
> > > > +		cpu_instruction_set = 'power8'
> > > >   	endif
> > > >   endif
> >
> > This change forces the build on a P9 system to use the P8 instruction set.
> > Prior to this change the "native" machine type was used which resulted
> > in P9 instructions when built on a P9 system.  How do I force the
> > build to use the
> > power9 instruction set in this case?
> >
> > Dave
> 
> From looking at the patch, setting the "platform" to "native", or the
> instruction_set to "native" should do this.
> While I consider generic builds a good thing, I wonder if there is an expectation
> that "native" is always the default build type for DPDK builds?
> 
> /Bruce

I left this patch alone so that people could chime in, but noone did, so let's try to find some agreeable solution.

My current thoughts are as such:
The platform parameter specifies a set of DPDK options that will be used. This is what arm uses for its builds, x86 and ppc don't use this.
The cpu_instruction_set sets just one configuration among the "platform" configuration set.
We want the build to work on most machines of the machine architecture. That implies platform=generic (as in use config options that will work on everything of that architecture) and cpu_instruction_set=generic (as in use ISA that will work on all cpus of the build machine architecture).
Setting cpu_instruction_set=generic changes the build without cmdline options for ppc. Thus, the expectation may be that cpu_instruction_set should be native by default.

For arm, cpu_instruction_set is ignored (and thus the value doen't matter), since we can't use that without other config options (e.g. DPDK config for an SoC (such as RTE_ARM_FEATURE_ATOMICS) used with an invalid cpu_instuction_set). That means the only relevant parameter for Arm is platform and if we want to have a build usable on most machines of the build type, we have to use platform=generic.

For x86 and ppc, there's no difference between native and generic platform (as it's a new argument, the purpose of which is to differentiate DPDK config across platforms, which doesn't exist for x86 and ppc - DPDK config is the same (correct me if I'm wrong)).

So it basically boils down to what should be the default value of cpu_instruction_set when platform=generic (for platform=native, it's obviously native):
1. cpu_instruction_set=native, this would preserve the current behavior, but we won't use the 'auto' default. I think we can fall back to this if we don't agree on anything better.
2. cpu_instruction_set=auto, the same as cpu_instruction_set=generic,
3. cpu_instruction_set=generic, this changes behavior for ppc builds, but we may be able to remedy this:
Similarly to arm (arm is using platform for this, but the idea is the same), if cpu_instruction_set is generic, we can do some discovery for pcc and set the ISA accordingly (either power8 or power9). If I understand it correctly, power8 is a different architecture from power9 (I could be wrong on this), so this is desirable. There's some logic in config/ppc/meson.build, but it doesn't seem sufficient as a discovery mechanism between power8/power9.

I like 3 if we can find a way to discover power8/power9 ppc (that way we would be able to use cpu_instruction_set=auto), if not, we should probably go with 1 (in which case we can't use cpu_instruction_set=auto).

Or maybe we're overthingking this and we really should go with 1. What's (or should be) the difference between cpu_instruction_set=generic and cpu_instruction_set=native for x86 and ppc?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-02-19  9:11         ` Juraj Linkeš
@ 2021-02-22 21:25           ` David Christensen
  2021-02-23  8:45             ` Juraj Linkeš
  0 siblings, 1 reply; 16+ messages in thread
From: David Christensen @ 2021-02-22 21:25 UTC (permalink / raw)
  To: Juraj Linkeš, Bruce Richardson; +Cc: thomas, Honnappa.Nagarahalli, dev



On 2/19/21 1:11 AM, Juraj Linkeš wrote:
> 
> 
>> -----Original Message-----
>> From: Bruce Richardson <bruce.richardson@intel.com>
>> Sent: Wednesday, January 6, 2021 3:43 PM
>> To: David Christensen <drc@linux.vnet.ibm.com>
>> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net;
>> Honnappa.Nagarahalli@arm.com; dev@dpdk.org
>> Subject: Re: [RFC PATCH v2] build: add platform meson option
>>
>> On Tue, Jan 05, 2021 at 02:17:44PM -0800, David Christensen wrote:
>>>>> The current meson option 'machine' should only specify the ISA,
>>>>> which is not sufficient for Arm, where setting ISA implies other setting as
>> well.
>>>>> Add a new meson option, 'platform', which differentiates the type
>>>>> of the build
>>>>> (native/generic) and sets machine accordingly, unless the user
>>>>> chooses to override it.
>>>>> The 'machine' option also doesn't describe very well what it sets,
>>>>> so introduce a new option 'cpu_instruction_set', but keep
>>>>> 'machine' for backward compatibility.
>>>>> These two new variables, taken together, achieve what 'machine'
>>>>> was setting per architecture - setting the ISA in x86 build and
>>>>> setting native/default 'build type' in aarch64 build - is now
>>>>> properly being set for all architectures in a uniform manner.
>>>>>
>>>>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>>>>> ---
>>>>>    config/arm/meson.build        |  4 +--
>>>>>    config/meson.build            | 47 +++++++++++++++++++++++++----------
>>>>>    devtools/test-meson-builds.sh |  9 ++++---
>>>>>    meson_options.txt             |  8 ++++--
>>>>>    4 files changed, 47 insertions(+), 21 deletions(-)
>>>>>
>>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>>>>> 42b4e43c7..6b09a74a7 100644
>>>>> --- a/config/arm/meson.build
>>>>> +++ b/config/arm/meson.build
>>>>> @@ -3,10 +3,10 @@
>>>>>    # Copyright(c) 2017 Cavium, Inc
>>>>>
>>>>>    # for checking defines we need to use the correct compiler flags
>>>>> -march_opt = '-
>>>>> march=@0@'.format(machine)
>>>>> +march_opt = '-march=@0@'.format(cpu_instruction_set)
>>>>>
>>>>>    arm_force_native_march = false
>>>>> -arm_force_default_march = (machine == 'default')
>>>>> +arm_force_default_march = (platform == 'generic')
>>>>>
>>>>>    flags_common_default = [
>>>>>    	# Accelarate rte_memcpy. Be sure to run unit test
>>>>> (memcpy_perf_autotest) diff --git a/config/meson.build
>>>>> b/config/meson.build index a3154e29c..647116513 100644
>>>>> --- a/config/meson.build
>>>>> +++ b/config/meson.build
>>>>> @@ -63,42 +63,63 @@ if not is_windows
>>>>>    			pmd_subdir_opt)
>>>>>    endif
>>>>>
>>>>> -# set the machine type and cflags for it
>>>>> +platform = get_option('platform')
>>>>> +
>>>>> +# set the cpu_instruction_set type and cflags for it
>>>>>    if meson.is_cross_build()
>>>>> -	machine = host_machine.cpu()
>>>>> +	cpu_instruction_set = host_machine.cpu()
>>>>>    else
>>>>> -	machine = get_option('machine')
>>>>> +	cpu_instruction_set = get_option('cpu_instruction_set')
>>>>> +	if get_option('machine') != 'auto'
>>>>> +		warning('The "machine" option is deprecated. ' +
>>>>> +		        'Please use "cpu_instruction_set" instead.')
>>>>> +		if cpu_instruction_set != 'auto'
>>>>> +			error('Setting both "machine" and ' +
>>>>> +			      '"cpu_instruction_set" is unsupported.')
>>>>> +		endif
>>>>> +		cpu_instruction_set = get_option('machine')
>>>>> +	endif
>>>>> +endif
>>>>> +
>>>>> +if platform == 'native'
>>>>> +	if cpu_instruction_set == 'auto'
>>>>> +		cpu_instruction_set = 'native'
>>>>> +	endif
>>>>> +elif platform == 'generic'
>>>>> +	if cpu_instruction_set == 'auto'
>>>>> +		cpu_instruction_set = 'default'
>>>>> +	endif
>>>>>    endif
>>>>>
>>>>> -# machine type 'default' is special, it defaults to the per arch
>>>>> agreed common
>>>>> +if cpu_instruction_set == 'default'
>>>>> +# cpu_instruction_set type 'default' is special, it defaults to
>>>>> +the per arch agreed common
>>>>>    # minimal baseline needed for DPDK.
>>>>>    # 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 host_machine.cpu_family().startswith('x86')
>>>>>    		# matches the old pre-meson build systems default
>>>>> -		machine = 'corei7'
>>>>> +		cpu_instruction_set = 'corei7'
>>>>>    	elif host_machine.cpu_family().startswith('arm')
>>>>> -		machine = 'armv7-a'
>>>>> +		cpu_instruction_set = 'armv7-a'
>>>>>    	elif host_machine.cpu_family().startswith('aarch')
>>>>>    		# arm64 manages defaults in config/arm/meson.build
>>>>> -		machine = 'default'
>>>>> +		cpu_instruction_set = 'default'
>>>>>    	elif host_machine.cpu_family().startswith('ppc')
>>>>> -		machine = 'power8'
>>>>> +		cpu_instruction_set = 'power8'
>>>>>    	endif
>>>>>    endif
>>>
>>> This change forces the build on a P9 system to use the P8 instruction set.
>>> Prior to this change the "native" machine type was used which resulted
>>> in P9 instructions when built on a P9 system.  How do I force the
>>> build to use the
>>> power9 instruction set in this case?
>>>
>>> Dave
>>
>>  From looking at the patch, setting the "platform" to "native", or the
>> instruction_set to "native" should do this.
>> While I consider generic builds a good thing, I wonder if there is an expectation
>> that "native" is always the default build type for DPDK builds?
>>
>> /Bruce
> 
> I left this patch alone so that people could chime in, but noone did, so let's try to find some agreeable solution.
> 
> My current thoughts are as such:
> The platform parameter specifies a set of DPDK options that will be used. This is what arm uses for its builds, x86 and ppc don't use this.
> The cpu_instruction_set sets just one configuration among the "platform" configuration set.
> We want the build to work on most machines of the machine architecture. That implies platform=generic (as in use config options that will work on everything of that architecture) and cpu_instruction_set=generic (as in use ISA that will work on all cpus of the build machine architecture).
> Setting cpu_instruction_set=generic changes the build without cmdline options for ppc. Thus, the expectation may be that cpu_instruction_set should be native by default.
> 
> For arm, cpu_instruction_set is ignored (and thus the value doen't matter), since we can't use that without other config options (e.g. DPDK config for an SoC (such as RTE_ARM_FEATURE_ATOMICS) used with an invalid cpu_instuction_set). That means the only relevant parameter for Arm is platform and if we want to have a build usable on most machines of the build type, we have to use platform=generic.
> 
> For x86 and ppc, there's no difference between native and generic platform (as it's a new argument, the purpose of which is to differentiate DPDK config across platforms, which doesn't exist for x86 and ppc - DPDK config is the same (correct me if I'm wrong)).
> 
> So it basically boils down to what should be the default value of cpu_instruction_set when platform=generic (for platform=native, it's obviously native):
> 1. cpu_instruction_set=native, this would preserve the current behavior, but we won't use the 'auto' default. I think we can fall back to this if we don't agree on anything better.
> 2. cpu_instruction_set=auto, the same as cpu_instruction_set=generic,
> 3. cpu_instruction_set=generic, this changes behavior for ppc builds, but we may be able to remedy this:
> Similarly to arm (arm is using platform for this, but the idea is the same), if cpu_instruction_set is generic, we can do some discovery for pcc and set the ISA accordingly (either power8 or power9). If I understand it correctly, power8 is a different architecture from power9 (I could be wrong on this), so this is desirable. There's some logic in config/ppc/meson.build, but it doesn't seem sufficient as a discovery mechanism between power8/power9.

POWER8 code (Power ISA v2.07) runs on POWER9 CPUs (Power ISA v3.0), so 
setting cpu_instruction_set to GENERIC is a reasonable option. POWER10 
CPUs (Power ISA v3.1) are around the corner so I want to make sure 
developers can tune the application for the platform as easily as 
possible.  I'm fine with supporting GENERIC, just need clear 
instructions on how to build for a particular ISA.

Dave

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-02-22 21:25           ` David Christensen
@ 2021-02-23  8:45             ` Juraj Linkeš
  2021-02-23  9:43               ` Bruce Richardson
  0 siblings, 1 reply; 16+ messages in thread
From: Juraj Linkeš @ 2021-02-23  8:45 UTC (permalink / raw)
  To: David Christensen, Bruce Richardson; +Cc: thomas, Honnappa.Nagarahalli, dev



> -----Original Message-----
> From: David Christensen <drc@linux.vnet.ibm.com>
> Sent: Monday, February 22, 2021 10:25 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
> <bruce.richardson@intel.com>
> Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v2] build: add platform meson option
> 
> 
> 
> On 2/19/21 1:11 AM, Juraj Linkeš wrote:
> >
> >
> >> -----Original Message-----
> >> From: Bruce Richardson <bruce.richardson@intel.com>
> >> Sent: Wednesday, January 6, 2021 3:43 PM
> >> To: David Christensen <drc@linux.vnet.ibm.com>
> >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net;
> >> Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> >> Subject: Re: [RFC PATCH v2] build: add platform meson option
> >>
> >> On Tue, Jan 05, 2021 at 02:17:44PM -0800, David Christensen wrote:
> >>>>> The current meson option 'machine' should only specify the ISA,
> >>>>> which is not sufficient for Arm, where setting ISA implies other
> >>>>> setting as
> >> well.
> >>>>> Add a new meson option, 'platform', which differentiates the type
> >>>>> of the build
> >>>>> (native/generic) and sets machine accordingly, unless the user
> >>>>> chooses to override it.
> >>>>> The 'machine' option also doesn't describe very well what it sets,
> >>>>> so introduce a new option 'cpu_instruction_set', but keep
> >>>>> 'machine' for backward compatibility.
> >>>>> These two new variables, taken together, achieve what 'machine'
> >>>>> was setting per architecture - setting the ISA in x86 build and
> >>>>> setting native/default 'build type' in aarch64 build - is now
> >>>>> properly being set for all architectures in a uniform manner.
> >>>>>
> >>>>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> >>>>> ---
> >>>>>    config/arm/meson.build        |  4 +--
> >>>>>    config/meson.build            | 47 +++++++++++++++++++++++++----------
> >>>>>    devtools/test-meson-builds.sh |  9 ++++---
> >>>>>    meson_options.txt             |  8 ++++--
> >>>>>    4 files changed, 47 insertions(+), 21 deletions(-)
> >>>>>
> >>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> >>>>> 42b4e43c7..6b09a74a7 100644
> >>>>> --- a/config/arm/meson.build
> >>>>> +++ b/config/arm/meson.build
> >>>>> @@ -3,10 +3,10 @@
> >>>>>    # Copyright(c) 2017 Cavium, Inc
> >>>>>
> >>>>>    # for checking defines we need to use the correct compiler
> >>>>> flags -march_opt = '-
> >>>>> march=@0@'.format(machine)
> >>>>> +march_opt = '-march=@0@'.format(cpu_instruction_set)
> >>>>>
> >>>>>    arm_force_native_march = false
> >>>>> -arm_force_default_march = (machine == 'default')
> >>>>> +arm_force_default_march = (platform == 'generic')
> >>>>>
> >>>>>    flags_common_default = [
> >>>>>    	# Accelarate rte_memcpy. Be sure to run unit test
> >>>>> (memcpy_perf_autotest) diff --git a/config/meson.build
> >>>>> b/config/meson.build index a3154e29c..647116513 100644
> >>>>> --- a/config/meson.build
> >>>>> +++ b/config/meson.build
> >>>>> @@ -63,42 +63,63 @@ if not is_windows
> >>>>>    			pmd_subdir_opt)
> >>>>>    endif
> >>>>>
> >>>>> -# set the machine type and cflags for it
> >>>>> +platform = get_option('platform')
> >>>>> +
> >>>>> +# set the cpu_instruction_set type and cflags for it
> >>>>>    if meson.is_cross_build()
> >>>>> -	machine = host_machine.cpu()
> >>>>> +	cpu_instruction_set = host_machine.cpu()
> >>>>>    else
> >>>>> -	machine = get_option('machine')
> >>>>> +	cpu_instruction_set = get_option('cpu_instruction_set')
> >>>>> +	if get_option('machine') != 'auto'
> >>>>> +		warning('The "machine" option is deprecated. ' +
> >>>>> +		        'Please use "cpu_instruction_set" instead.')
> >>>>> +		if cpu_instruction_set != 'auto'
> >>>>> +			error('Setting both "machine" and ' +
> >>>>> +			      '"cpu_instruction_set" is unsupported.')
> >>>>> +		endif
> >>>>> +		cpu_instruction_set = get_option('machine')
> >>>>> +	endif
> >>>>> +endif
> >>>>> +
> >>>>> +if platform == 'native'
> >>>>> +	if cpu_instruction_set == 'auto'
> >>>>> +		cpu_instruction_set = 'native'
> >>>>> +	endif
> >>>>> +elif platform == 'generic'
> >>>>> +	if cpu_instruction_set == 'auto'
> >>>>> +		cpu_instruction_set = 'default'
> >>>>> +	endif
> >>>>>    endif
> >>>>>
> >>>>> -# machine type 'default' is special, it defaults to the per arch
> >>>>> agreed common
> >>>>> +if cpu_instruction_set == 'default'
> >>>>> +# cpu_instruction_set type 'default' is special, it defaults to
> >>>>> +the per arch agreed common
> >>>>>    # minimal baseline needed for DPDK.
> >>>>>    # 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 host_machine.cpu_family().startswith('x86')
> >>>>>    		# matches the old pre-meson build systems default
> >>>>> -		machine = 'corei7'
> >>>>> +		cpu_instruction_set = 'corei7'
> >>>>>    	elif host_machine.cpu_family().startswith('arm')
> >>>>> -		machine = 'armv7-a'
> >>>>> +		cpu_instruction_set = 'armv7-a'
> >>>>>    	elif host_machine.cpu_family().startswith('aarch')
> >>>>>    		# arm64 manages defaults in config/arm/meson.build
> >>>>> -		machine = 'default'
> >>>>> +		cpu_instruction_set = 'default'
> >>>>>    	elif host_machine.cpu_family().startswith('ppc')
> >>>>> -		machine = 'power8'
> >>>>> +		cpu_instruction_set = 'power8'
> >>>>>    	endif
> >>>>>    endif
> >>>
> >>> This change forces the build on a P9 system to use the P8 instruction set.
> >>> Prior to this change the "native" machine type was used which
> >>> resulted in P9 instructions when built on a P9 system.  How do I
> >>> force the build to use the
> >>> power9 instruction set in this case?
> >>>
> >>> Dave
> >>
> >>  From looking at the patch, setting the "platform" to "native", or
> >> the instruction_set to "native" should do this.
> >> While I consider generic builds a good thing, I wonder if there is an
> >> expectation that "native" is always the default build type for DPDK builds?
> >>
> >> /Bruce
> >
> > I left this patch alone so that people could chime in, but noone did, so let's try
> to find some agreeable solution.
> >
> > My current thoughts are as such:
> > The platform parameter specifies a set of DPDK options that will be used. This
> is what arm uses for its builds, x86 and ppc don't use this.
> > The cpu_instruction_set sets just one configuration among the "platform"
> configuration set.
> > We want the build to work on most machines of the machine architecture.
> That implies platform=generic (as in use config options that will work on
> everything of that architecture) and cpu_instruction_set=generic (as in use ISA
> that will work on all cpus of the build machine architecture).
> > Setting cpu_instruction_set=generic changes the build without cmdline options
> for ppc. Thus, the expectation may be that cpu_instruction_set should be native
> by default.
> >
> > For arm, cpu_instruction_set is ignored (and thus the value doen't matter),
> since we can't use that without other config options (e.g. DPDK config for an
> SoC (such as RTE_ARM_FEATURE_ATOMICS) used with an invalid
> cpu_instuction_set). That means the only relevant parameter for Arm is platform
> and if we want to have a build usable on most machines of the build type, we
> have to use platform=generic.
> >
> > For x86 and ppc, there's no difference between native and generic platform (as
> it's a new argument, the purpose of which is to differentiate DPDK config across
> platforms, which doesn't exist for x86 and ppc - DPDK config is the same (correct
> me if I'm wrong)).
> >
> > So it basically boils down to what should be the default value of
> cpu_instruction_set when platform=generic (for platform=native, it's obviously
> native):
> > 1. cpu_instruction_set=native, this would preserve the current behavior, but
> we won't use the 'auto' default. I think we can fall back to this if we don't agree
> on anything better.
> > 2. cpu_instruction_set=auto, the same as cpu_instruction_set=generic,
> > 3. cpu_instruction_set=generic, this changes behavior for ppc builds, but we
> may be able to remedy this:
> > Similarly to arm (arm is using platform for this, but the idea is the same), if
> cpu_instruction_set is generic, we can do some discovery for pcc and set the ISA
> accordingly (either power8 or power9). If I understand it correctly, power8 is a
> different architecture from power9 (I could be wrong on this), so this is
> desirable. There's some logic in config/ppc/meson.build, but it doesn't seem
> sufficient as a discovery mechanism between power8/power9.
> 
> POWER8 code (Power ISA v2.07) runs on POWER9 CPUs (Power ISA v3.0), so
> setting cpu_instruction_set to GENERIC is a reasonable option. POWER10 CPUs
> (Power ISA v3.1) are around the corner so I want to make sure developers can
> tune the application for the platform as easily as possible.  I'm fine with
> supporting GENERIC, just need clear instructions on how to build for a particular
> ISA.
> 

That's good to hear. Setting cpu_instruction_set will set the ISA.

We'll need to document the behavior properly, but I'm not sure where - Bruce? Seems like meson_options.txt doesn't have enough room for that.

The default bahavior is the ISA will be set according to the platform parameter. If cpu_instruction_set is specified, that value will be used.

> Dave


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-02-23  8:45             ` Juraj Linkeš
@ 2021-02-23  9:43               ` Bruce Richardson
  2021-02-25 12:51                 ` Juraj Linkeš
  0 siblings, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2021-02-23  9:43 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: David Christensen, thomas, Honnappa.Nagarahalli, dev

On Tue, Feb 23, 2021 at 08:45:09AM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: David Christensen <drc@linux.vnet.ibm.com>
> > Sent: Monday, February 22, 2021 10:25 PM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
> > <bruce.richardson@intel.com>
> > Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> > Subject: Re: [RFC PATCH v2] build: add platform meson option
> > 
> > 
> > 
> > On 2/19/21 1:11 AM, Juraj Linkeš wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Bruce Richardson <bruce.richardson@intel.com>
> > >> Sent: Wednesday, January 6, 2021 3:43 PM
> > >> To: David Christensen <drc@linux.vnet.ibm.com>
> > >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; thomas@monjalon.net;
> > >> Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> > >> Subject: Re: [RFC PATCH v2] build: add platform meson option
> > >>
> > >> On Tue, Jan 05, 2021 at 02:17:44PM -0800, David Christensen wrote:
> > >>>>> The current meson option 'machine' should only specify the ISA,
> > >>>>> which is not sufficient for Arm, where setting ISA implies other
> > >>>>> setting as
> > >> well.
> > >>>>> Add a new meson option, 'platform', which differentiates the type
> > >>>>> of the build
> > >>>>> (native/generic) and sets machine accordingly, unless the user
> > >>>>> chooses to override it.
> > >>>>> The 'machine' option also doesn't describe very well what it sets,
> > >>>>> so introduce a new option 'cpu_instruction_set', but keep
> > >>>>> 'machine' for backward compatibility.
> > >>>>> These two new variables, taken together, achieve what 'machine'
> > >>>>> was setting per architecture - setting the ISA in x86 build and
> > >>>>> setting native/default 'build type' in aarch64 build - is now
> > >>>>> properly being set for all architectures in a uniform manner.
> > >>>>>
> > >>>>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > >>>>> ---
> > >>>>>    config/arm/meson.build        |  4 +--
> > >>>>>    config/meson.build            | 47 +++++++++++++++++++++++++----------
> > >>>>>    devtools/test-meson-builds.sh |  9 ++++---
> > >>>>>    meson_options.txt             |  8 ++++--
> > >>>>>    4 files changed, 47 insertions(+), 21 deletions(-)
> > >>>>>
> > >>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > >>>>> 42b4e43c7..6b09a74a7 100644
> > >>>>> --- a/config/arm/meson.build
> > >>>>> +++ b/config/arm/meson.build
> > >>>>> @@ -3,10 +3,10 @@
> > >>>>>    # Copyright(c) 2017 Cavium, Inc
> > >>>>>
> > >>>>>    # for checking defines we need to use the correct compiler
> > >>>>> flags -march_opt = '-
> > >>>>> march=@0@'.format(machine)
> > >>>>> +march_opt = '-march=@0@'.format(cpu_instruction_set)
> > >>>>>
> > >>>>>    arm_force_native_march = false
> > >>>>> -arm_force_default_march = (machine == 'default')
> > >>>>> +arm_force_default_march = (platform == 'generic')
> > >>>>>
> > >>>>>    flags_common_default = [
> > >>>>>    	# Accelarate rte_memcpy. Be sure to run unit test
> > >>>>> (memcpy_perf_autotest) diff --git a/config/meson.build
> > >>>>> b/config/meson.build index a3154e29c..647116513 100644
> > >>>>> --- a/config/meson.build
> > >>>>> +++ b/config/meson.build
> > >>>>> @@ -63,42 +63,63 @@ if not is_windows
> > >>>>>    			pmd_subdir_opt)
> > >>>>>    endif
> > >>>>>
> > >>>>> -# set the machine type and cflags for it
> > >>>>> +platform = get_option('platform')
> > >>>>> +
> > >>>>> +# set the cpu_instruction_set type and cflags for it
> > >>>>>    if meson.is_cross_build()
> > >>>>> -	machine = host_machine.cpu()
> > >>>>> +	cpu_instruction_set = host_machine.cpu()
> > >>>>>    else
> > >>>>> -	machine = get_option('machine')
> > >>>>> +	cpu_instruction_set = get_option('cpu_instruction_set')
> > >>>>> +	if get_option('machine') != 'auto'
> > >>>>> +		warning('The "machine" option is deprecated. ' +
> > >>>>> +		        'Please use "cpu_instruction_set" instead.')
> > >>>>> +		if cpu_instruction_set != 'auto'
> > >>>>> +			error('Setting both "machine" and ' +
> > >>>>> +			      '"cpu_instruction_set" is unsupported.')
> > >>>>> +		endif
> > >>>>> +		cpu_instruction_set = get_option('machine')
> > >>>>> +	endif
> > >>>>> +endif
> > >>>>> +
> > >>>>> +if platform == 'native'
> > >>>>> +	if cpu_instruction_set == 'auto'
> > >>>>> +		cpu_instruction_set = 'native'
> > >>>>> +	endif
> > >>>>> +elif platform == 'generic'
> > >>>>> +	if cpu_instruction_set == 'auto'
> > >>>>> +		cpu_instruction_set = 'default'
> > >>>>> +	endif
> > >>>>>    endif
> > >>>>>
> > >>>>> -# machine type 'default' is special, it defaults to the per arch
> > >>>>> agreed common
> > >>>>> +if cpu_instruction_set == 'default'
> > >>>>> +# cpu_instruction_set type 'default' is special, it defaults to
> > >>>>> +the per arch agreed common
> > >>>>>    # minimal baseline needed for DPDK.
> > >>>>>    # 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 host_machine.cpu_family().startswith('x86')
> > >>>>>    		# matches the old pre-meson build systems default
> > >>>>> -		machine = 'corei7'
> > >>>>> +		cpu_instruction_set = 'corei7'
> > >>>>>    	elif host_machine.cpu_family().startswith('arm')
> > >>>>> -		machine = 'armv7-a'
> > >>>>> +		cpu_instruction_set = 'armv7-a'
> > >>>>>    	elif host_machine.cpu_family().startswith('aarch')
> > >>>>>    		# arm64 manages defaults in config/arm/meson.build
> > >>>>> -		machine = 'default'
> > >>>>> +		cpu_instruction_set = 'default'
> > >>>>>    	elif host_machine.cpu_family().startswith('ppc')
> > >>>>> -		machine = 'power8'
> > >>>>> +		cpu_instruction_set = 'power8'
> > >>>>>    	endif
> > >>>>>    endif
> > >>>
> > >>> This change forces the build on a P9 system to use the P8 instruction set.
> > >>> Prior to this change the "native" machine type was used which
> > >>> resulted in P9 instructions when built on a P9 system.  How do I
> > >>> force the build to use the
> > >>> power9 instruction set in this case?
> > >>>
> > >>> Dave
> > >>
> > >>  From looking at the patch, setting the "platform" to "native", or
> > >> the instruction_set to "native" should do this.
> > >> While I consider generic builds a good thing, I wonder if there is an
> > >> expectation that "native" is always the default build type for DPDK builds?
> > >>
> > >> /Bruce
> > >
> > > I left this patch alone so that people could chime in, but noone did, so let's try
> > to find some agreeable solution.
> > >
> > > My current thoughts are as such:
> > > The platform parameter specifies a set of DPDK options that will be used. This
> > is what arm uses for its builds, x86 and ppc don't use this.
> > > The cpu_instruction_set sets just one configuration among the "platform"
> > configuration set.
> > > We want the build to work on most machines of the machine architecture.
> > That implies platform=generic (as in use config options that will work on
> > everything of that architecture) and cpu_instruction_set=generic (as in use ISA
> > that will work on all cpus of the build machine architecture).
> > > Setting cpu_instruction_set=generic changes the build without cmdline options
> > for ppc. Thus, the expectation may be that cpu_instruction_set should be native
> > by default.
> > >
> > > For arm, cpu_instruction_set is ignored (and thus the value doen't matter),
> > since we can't use that without other config options (e.g. DPDK config for an
> > SoC (such as RTE_ARM_FEATURE_ATOMICS) used with an invalid
> > cpu_instuction_set). That means the only relevant parameter for Arm is platform
> > and if we want to have a build usable on most machines of the build type, we
> > have to use platform=generic.
> > >
> > > For x86 and ppc, there's no difference between native and generic platform (as
> > it's a new argument, the purpose of which is to differentiate DPDK config across
> > platforms, which doesn't exist for x86 and ppc - DPDK config is the same (correct
> > me if I'm wrong)).
> > >
> > > So it basically boils down to what should be the default value of
> > cpu_instruction_set when platform=generic (for platform=native, it's obviously
> > native):
> > > 1. cpu_instruction_set=native, this would preserve the current behavior, but
> > we won't use the 'auto' default. I think we can fall back to this if we don't agree
> > on anything better.
> > > 2. cpu_instruction_set=auto, the same as cpu_instruction_set=generic,
> > > 3. cpu_instruction_set=generic, this changes behavior for ppc builds, but we
> > may be able to remedy this:
> > > Similarly to arm (arm is using platform for this, but the idea is the same), if
> > cpu_instruction_set is generic, we can do some discovery for pcc and set the ISA
> > accordingly (either power8 or power9). If I understand it correctly, power8 is a
> > different architecture from power9 (I could be wrong on this), so this is
> > desirable. There's some logic in config/ppc/meson.build, but it doesn't seem
> > sufficient as a discovery mechanism between power8/power9.
> > 
> > POWER8 code (Power ISA v2.07) runs on POWER9 CPUs (Power ISA v3.0), so
> > setting cpu_instruction_set to GENERIC is a reasonable option. POWER10 CPUs
> > (Power ISA v3.1) are around the corner so I want to make sure developers can
> > tune the application for the platform as easily as possible.  I'm fine with
> > supporting GENERIC, just need clear instructions on how to build for a particular
> > ISA.
> > 
> 
> That's good to hear. Setting cpu_instruction_set will set the ISA.
> 
> We'll need to document the behavior properly, but I'm not sure where - Bruce? Seems like meson_options.txt doesn't have enough room for that.
> 

Yes, agreed. I suggest putting in the option help text a link to the
documentation where we explain the options more fully.

/Bruce

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-02-23  9:43               ` Bruce Richardson
@ 2021-02-25 12:51                 ` Juraj Linkeš
  2021-02-25 12:54                   ` Bruce Richardson
  0 siblings, 1 reply; 16+ messages in thread
From: Juraj Linkeš @ 2021-02-25 12:51 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: David Christensen, thomas, Honnappa.Nagarahalli, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Tuesday, February 23, 2021 10:43 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: David Christensen <drc@linux.vnet.ibm.com>; thomas@monjalon.net;
> Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v2] build: add platform meson option
> 
> On Tue, Feb 23, 2021 at 08:45:09AM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: David Christensen <drc@linux.vnet.ibm.com>
> > > Sent: Monday, February 22, 2021 10:25 PM
> > > To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
> > > <bruce.richardson@intel.com>
> > > Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com;
> dev@dpdk.org
> > > Subject: Re: [RFC PATCH v2] build: add platform meson option
> > >
> > >
> > >
> > > On 2/19/21 1:11 AM, Juraj Linkeš wrote:
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: Bruce Richardson <bruce.richardson@intel.com>
> > > >> Sent: Wednesday, January 6, 2021 3:43 PM
> > > >> To: David Christensen <drc@linux.vnet.ibm.com>
> > > >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>;
> > > >> thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> > > >> Subject: Re: [RFC PATCH v2] build: add platform meson option
> > > >>
> > > >> On Tue, Jan 05, 2021 at 02:17:44PM -0800, David Christensen wrote:
> > > >>>>> The current meson option 'machine' should only specify the
> > > >>>>> ISA, which is not sufficient for Arm, where setting ISA
> > > >>>>> implies other setting as
> > > >> well.
> > > >>>>> Add a new meson option, 'platform', which differentiates the
> > > >>>>> type of the build
> > > >>>>> (native/generic) and sets machine accordingly, unless the user
> > > >>>>> chooses to override it.
> > > >>>>> The 'machine' option also doesn't describe very well what it
> > > >>>>> sets, so introduce a new option 'cpu_instruction_set', but
> > > >>>>> keep 'machine' for backward compatibility.
> > > >>>>> These two new variables, taken together, achieve what 'machine'
> > > >>>>> was setting per architecture - setting the ISA in x86 build
> > > >>>>> and setting native/default 'build type' in aarch64 build - is
> > > >>>>> now properly being set for all architectures in a uniform manner.
> > > >>>>>
> > > >>>>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > >>>>> ---
> > > >>>>>    config/arm/meson.build        |  4 +--
> > > >>>>>    config/meson.build            | 47 +++++++++++++++++++++++++--------
> --
> > > >>>>>    devtools/test-meson-builds.sh |  9 ++++---
> > > >>>>>    meson_options.txt             |  8 ++++--
> > > >>>>>    4 files changed, 47 insertions(+), 21 deletions(-)
> > > >>>>>
> > > >>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > >>>>> index
> > > >>>>> 42b4e43c7..6b09a74a7 100644
> > > >>>>> --- a/config/arm/meson.build
> > > >>>>> +++ b/config/arm/meson.build
> > > >>>>> @@ -3,10 +3,10 @@
> > > >>>>>    # Copyright(c) 2017 Cavium, Inc
> > > >>>>>
> > > >>>>>    # for checking defines we need to use the correct compiler
> > > >>>>> flags -march_opt = '-
> > > >>>>> march=@0@'.format(machine)
> > > >>>>> +march_opt = '-march=@0@'.format(cpu_instruction_set)
> > > >>>>>
> > > >>>>>    arm_force_native_march = false -arm_force_default_march =
> > > >>>>> (machine == 'default')
> > > >>>>> +arm_force_default_march = (platform == 'generic')
> > > >>>>>
> > > >>>>>    flags_common_default = [
> > > >>>>>    	# Accelarate rte_memcpy. Be sure to run unit test
> > > >>>>> (memcpy_perf_autotest) diff --git a/config/meson.build
> > > >>>>> b/config/meson.build index a3154e29c..647116513 100644
> > > >>>>> --- a/config/meson.build
> > > >>>>> +++ b/config/meson.build
> > > >>>>> @@ -63,42 +63,63 @@ if not is_windows
> > > >>>>>    			pmd_subdir_opt)
> > > >>>>>    endif
> > > >>>>>
> > > >>>>> -# set the machine type and cflags for it
> > > >>>>> +platform = get_option('platform')
> > > >>>>> +
> > > >>>>> +# set the cpu_instruction_set type and cflags for it
> > > >>>>>    if meson.is_cross_build()
> > > >>>>> -	machine = host_machine.cpu()
> > > >>>>> +	cpu_instruction_set = host_machine.cpu()
> > > >>>>>    else
> > > >>>>> -	machine = get_option('machine')
> > > >>>>> +	cpu_instruction_set = get_option('cpu_instruction_set')
> > > >>>>> +	if get_option('machine') != 'auto'
> > > >>>>> +		warning('The "machine" option is deprecated. ' +
> > > >>>>> +		        'Please use "cpu_instruction_set" instead.')
> > > >>>>> +		if cpu_instruction_set != 'auto'
> > > >>>>> +			error('Setting both "machine" and ' +
> > > >>>>> +			      '"cpu_instruction_set" is unsupported.')
> > > >>>>> +		endif
> > > >>>>> +		cpu_instruction_set = get_option('machine')
> > > >>>>> +	endif
> > > >>>>> +endif
> > > >>>>> +
> > > >>>>> +if platform == 'native'
> > > >>>>> +	if cpu_instruction_set == 'auto'
> > > >>>>> +		cpu_instruction_set = 'native'
> > > >>>>> +	endif
> > > >>>>> +elif platform == 'generic'
> > > >>>>> +	if cpu_instruction_set == 'auto'
> > > >>>>> +		cpu_instruction_set = 'default'
> > > >>>>> +	endif
> > > >>>>>    endif
> > > >>>>>
> > > >>>>> -# machine type 'default' is special, it defaults to the per
> > > >>>>> arch agreed common
> > > >>>>> +if cpu_instruction_set == 'default'
> > > >>>>> +# cpu_instruction_set type 'default' is special, it defaults
> > > >>>>> +to the per arch agreed common
> > > >>>>>    # minimal baseline needed for DPDK.
> > > >>>>>    # 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 host_machine.cpu_family().startswith('x86')
> > > >>>>>    		# matches the old pre-meson build systems default
> > > >>>>> -		machine = 'corei7'
> > > >>>>> +		cpu_instruction_set = 'corei7'
> > > >>>>>    	elif host_machine.cpu_family().startswith('arm')
> > > >>>>> -		machine = 'armv7-a'
> > > >>>>> +		cpu_instruction_set = 'armv7-a'
> > > >>>>>    	elif host_machine.cpu_family().startswith('aarch')
> > > >>>>>    		# arm64 manages defaults in config/arm/meson.build
> > > >>>>> -		machine = 'default'
> > > >>>>> +		cpu_instruction_set = 'default'
> > > >>>>>    	elif host_machine.cpu_family().startswith('ppc')
> > > >>>>> -		machine = 'power8'
> > > >>>>> +		cpu_instruction_set = 'power8'
> > > >>>>>    	endif
> > > >>>>>    endif
> > > >>>
> > > >>> This change forces the build on a P9 system to use the P8 instruction set.
> > > >>> Prior to this change the "native" machine type was used which
> > > >>> resulted in P9 instructions when built on a P9 system.  How do I
> > > >>> force the build to use the
> > > >>> power9 instruction set in this case?
> > > >>>
> > > >>> Dave
> > > >>
> > > >>  From looking at the patch, setting the "platform" to "native",
> > > >> or the instruction_set to "native" should do this.
> > > >> While I consider generic builds a good thing, I wonder if there
> > > >> is an expectation that "native" is always the default build type for DPDK
> builds?
> > > >>
> > > >> /Bruce
> > > >
> > > > I left this patch alone so that people could chime in, but noone
> > > > did, so let's try
> > > to find some agreeable solution.
> > > >
> > > > My current thoughts are as such:
> > > > The platform parameter specifies a set of DPDK options that will
> > > > be used. This
> > > is what arm uses for its builds, x86 and ppc don't use this.
> > > > The cpu_instruction_set sets just one configuration among the "platform"
> > > configuration set.
> > > > We want the build to work on most machines of the machine architecture.
> > > That implies platform=generic (as in use config options that will
> > > work on everything of that architecture) and
> > > cpu_instruction_set=generic (as in use ISA that will work on all cpus of the
> build machine architecture).
> > > > Setting cpu_instruction_set=generic changes the build without
> > > > cmdline options
> > > for ppc. Thus, the expectation may be that cpu_instruction_set
> > > should be native by default.
> > > >
> > > > For arm, cpu_instruction_set is ignored (and thus the value doen't
> > > > matter),
> > > since we can't use that without other config options (e.g. DPDK
> > > config for an SoC (such as RTE_ARM_FEATURE_ATOMICS) used with an
> > > invalid cpu_instuction_set). That means the only relevant parameter
> > > for Arm is platform and if we want to have a build usable on most
> > > machines of the build type, we have to use platform=generic.
> > > >
> > > > For x86 and ppc, there's no difference between native and generic
> > > > platform (as
> > > it's a new argument, the purpose of which is to differentiate DPDK
> > > config across platforms, which doesn't exist for x86 and ppc - DPDK
> > > config is the same (correct me if I'm wrong)).
> > > >
> > > > So it basically boils down to what should be the default value of
> > > cpu_instruction_set when platform=generic (for platform=native, it's
> > > obviously
> > > native):
> > > > 1. cpu_instruction_set=native, this would preserve the current
> > > > behavior, but
> > > we won't use the 'auto' default. I think we can fall back to this if
> > > we don't agree on anything better.
> > > > 2. cpu_instruction_set=auto, the same as
> > > > cpu_instruction_set=generic, 3. cpu_instruction_set=generic, this
> > > > changes behavior for ppc builds, but we
> > > may be able to remedy this:
> > > > Similarly to arm (arm is using platform for this, but the idea is
> > > > the same), if
> > > cpu_instruction_set is generic, we can do some discovery for pcc and
> > > set the ISA accordingly (either power8 or power9). If I understand
> > > it correctly, power8 is a different architecture from power9 (I
> > > could be wrong on this), so this is desirable. There's some logic in
> > > config/ppc/meson.build, but it doesn't seem sufficient as a discovery
> mechanism between power8/power9.
> > >
> > > POWER8 code (Power ISA v2.07) runs on POWER9 CPUs (Power ISA v3.0),
> > > so setting cpu_instruction_set to GENERIC is a reasonable option.
> > > POWER10 CPUs (Power ISA v3.1) are around the corner so I want to
> > > make sure developers can tune the application for the platform as
> > > easily as possible.  I'm fine with supporting GENERIC, just need
> > > clear instructions on how to build for a particular ISA.
> > >
> >
> > That's good to hear. Setting cpu_instruction_set will set the ISA.
> >
> > We'll need to document the behavior properly, but I'm not sure where - Bruce?
> Seems like meson_options.txt doesn't have enough room for that.
> >
> 
> Yes, agreed. I suggest putting in the option help text a link to the documentation
> where we explain the options more fully.
> 
> /Bruce

You mentioned in the kni cross compile patch that we should do a separate patch where we explain the option in docs and I believe this would go into that patch. Before we do that, let's figure out the best description without the link. I currently have:
option('cpu_instruction_set', type: 'string', value: 'auto',
	description: 'Set the target machine ISA (instruction set architecture). Will be set according to the platform option by default.')
option('platform', type: 'string', value: 'generic',
	description: 'Platform to build, either "native" or "generic".')

What do you think? Should we expand the platform description a bit?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-02-25 12:51                 ` Juraj Linkeš
@ 2021-02-25 12:54                   ` Bruce Richardson
  2021-02-25 12:57                     ` Juraj Linkeš
  0 siblings, 1 reply; 16+ messages in thread
From: Bruce Richardson @ 2021-02-25 12:54 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: David Christensen, thomas, Honnappa.Nagarahalli, dev

On Thu, Feb 25, 2021 at 12:51:57PM +0000, Juraj Linkeš wrote:
> 
> 
> > -----Original Message-----
> > From: Bruce Richardson <bruce.richardson@intel.com>
> > Sent: Tuesday, February 23, 2021 10:43 AM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Cc: David Christensen <drc@linux.vnet.ibm.com>; thomas@monjalon.net;
> > Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> > Subject: Re: [RFC PATCH v2] build: add platform meson option
> > 
> > On Tue, Feb 23, 2021 at 08:45:09AM +0000, Juraj Linkeš wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: David Christensen <drc@linux.vnet.ibm.com>
> > > > Sent: Monday, February 22, 2021 10:25 PM
> > > > To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
> > > > <bruce.richardson@intel.com>
> > > > Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com;
> > dev@dpdk.org
> > > > Subject: Re: [RFC PATCH v2] build: add platform meson option
> > > >
> > > >
> > > >
> > > > On 2/19/21 1:11 AM, Juraj Linkeš wrote:
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: Bruce Richardson <bruce.richardson@intel.com>
> > > > >> Sent: Wednesday, January 6, 2021 3:43 PM
> > > > >> To: David Christensen <drc@linux.vnet.ibm.com>
> > > > >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>;
> > > > >> thomas@monjalon.net; Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> > > > >> Subject: Re: [RFC PATCH v2] build: add platform meson option
> > > > >>
> > > > >> On Tue, Jan 05, 2021 at 02:17:44PM -0800, David Christensen wrote:
> > > > >>>>> The current meson option 'machine' should only specify the
> > > > >>>>> ISA, which is not sufficient for Arm, where setting ISA
> > > > >>>>> implies other setting as
> > > > >> well.
> > > > >>>>> Add a new meson option, 'platform', which differentiates the
> > > > >>>>> type of the build
> > > > >>>>> (native/generic) and sets machine accordingly, unless the user
> > > > >>>>> chooses to override it.
> > > > >>>>> The 'machine' option also doesn't describe very well what it
> > > > >>>>> sets, so introduce a new option 'cpu_instruction_set', but
> > > > >>>>> keep 'machine' for backward compatibility.
> > > > >>>>> These two new variables, taken together, achieve what 'machine'
> > > > >>>>> was setting per architecture - setting the ISA in x86 build
> > > > >>>>> and setting native/default 'build type' in aarch64 build - is
> > > > >>>>> now properly being set for all architectures in a uniform manner.
> > > > >>>>>
> > > > >>>>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > >>>>> ---
> > > > >>>>>    config/arm/meson.build        |  4 +--
> > > > >>>>>    config/meson.build            | 47 +++++++++++++++++++++++++--------
> > --
> > > > >>>>>    devtools/test-meson-builds.sh |  9 ++++---
> > > > >>>>>    meson_options.txt             |  8 ++++--
> > > > >>>>>    4 files changed, 47 insertions(+), 21 deletions(-)
> > > > >>>>>
> > > > >>>>> diff --git a/config/arm/meson.build b/config/arm/meson.build
> > > > >>>>> index
> > > > >>>>> 42b4e43c7..6b09a74a7 100644
> > > > >>>>> --- a/config/arm/meson.build
> > > > >>>>> +++ b/config/arm/meson.build
> > > > >>>>> @@ -3,10 +3,10 @@
> > > > >>>>>    # Copyright(c) 2017 Cavium, Inc
> > > > >>>>>
> > > > >>>>>    # for checking defines we need to use the correct compiler
> > > > >>>>> flags -march_opt = '-
> > > > >>>>> march=@0@'.format(machine)
> > > > >>>>> +march_opt = '-march=@0@'.format(cpu_instruction_set)
> > > > >>>>>
> > > > >>>>>    arm_force_native_march = false -arm_force_default_march =
> > > > >>>>> (machine == 'default')
> > > > >>>>> +arm_force_default_march = (platform == 'generic')
> > > > >>>>>
> > > > >>>>>    flags_common_default = [
> > > > >>>>>    	# Accelarate rte_memcpy. Be sure to run unit test
> > > > >>>>> (memcpy_perf_autotest) diff --git a/config/meson.build
> > > > >>>>> b/config/meson.build index a3154e29c..647116513 100644
> > > > >>>>> --- a/config/meson.build
> > > > >>>>> +++ b/config/meson.build
> > > > >>>>> @@ -63,42 +63,63 @@ if not is_windows
> > > > >>>>>    			pmd_subdir_opt)
> > > > >>>>>    endif
> > > > >>>>>
> > > > >>>>> -# set the machine type and cflags for it
> > > > >>>>> +platform = get_option('platform')
> > > > >>>>> +
> > > > >>>>> +# set the cpu_instruction_set type and cflags for it
> > > > >>>>>    if meson.is_cross_build()
> > > > >>>>> -	machine = host_machine.cpu()
> > > > >>>>> +	cpu_instruction_set = host_machine.cpu()
> > > > >>>>>    else
> > > > >>>>> -	machine = get_option('machine')
> > > > >>>>> +	cpu_instruction_set = get_option('cpu_instruction_set')
> > > > >>>>> +	if get_option('machine') != 'auto'
> > > > >>>>> +		warning('The "machine" option is deprecated. ' +
> > > > >>>>> +		        'Please use "cpu_instruction_set" instead.')
> > > > >>>>> +		if cpu_instruction_set != 'auto'
> > > > >>>>> +			error('Setting both "machine" and ' +
> > > > >>>>> +			      '"cpu_instruction_set" is unsupported.')
> > > > >>>>> +		endif
> > > > >>>>> +		cpu_instruction_set = get_option('machine')
> > > > >>>>> +	endif
> > > > >>>>> +endif
> > > > >>>>> +
> > > > >>>>> +if platform == 'native'
> > > > >>>>> +	if cpu_instruction_set == 'auto'
> > > > >>>>> +		cpu_instruction_set = 'native'
> > > > >>>>> +	endif
> > > > >>>>> +elif platform == 'generic'
> > > > >>>>> +	if cpu_instruction_set == 'auto'
> > > > >>>>> +		cpu_instruction_set = 'default'
> > > > >>>>> +	endif
> > > > >>>>>    endif
> > > > >>>>>
> > > > >>>>> -# machine type 'default' is special, it defaults to the per
> > > > >>>>> arch agreed common
> > > > >>>>> +if cpu_instruction_set == 'default'
> > > > >>>>> +# cpu_instruction_set type 'default' is special, it defaults
> > > > >>>>> +to the per arch agreed common
> > > > >>>>>    # minimal baseline needed for DPDK.
> > > > >>>>>    # 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 host_machine.cpu_family().startswith('x86')
> > > > >>>>>    		# matches the old pre-meson build systems default
> > > > >>>>> -		machine = 'corei7'
> > > > >>>>> +		cpu_instruction_set = 'corei7'
> > > > >>>>>    	elif host_machine.cpu_family().startswith('arm')
> > > > >>>>> -		machine = 'armv7-a'
> > > > >>>>> +		cpu_instruction_set = 'armv7-a'
> > > > >>>>>    	elif host_machine.cpu_family().startswith('aarch')
> > > > >>>>>    		# arm64 manages defaults in config/arm/meson.build
> > > > >>>>> -		machine = 'default'
> > > > >>>>> +		cpu_instruction_set = 'default'
> > > > >>>>>    	elif host_machine.cpu_family().startswith('ppc')
> > > > >>>>> -		machine = 'power8'
> > > > >>>>> +		cpu_instruction_set = 'power8'
> > > > >>>>>    	endif
> > > > >>>>>    endif
> > > > >>>
> > > > >>> This change forces the build on a P9 system to use the P8 instruction set.
> > > > >>> Prior to this change the "native" machine type was used which
> > > > >>> resulted in P9 instructions when built on a P9 system.  How do I
> > > > >>> force the build to use the
> > > > >>> power9 instruction set in this case?
> > > > >>>
> > > > >>> Dave
> > > > >>
> > > > >>  From looking at the patch, setting the "platform" to "native",
> > > > >> or the instruction_set to "native" should do this.
> > > > >> While I consider generic builds a good thing, I wonder if there
> > > > >> is an expectation that "native" is always the default build type for DPDK
> > builds?
> > > > >>
> > > > >> /Bruce
> > > > >
> > > > > I left this patch alone so that people could chime in, but noone
> > > > > did, so let's try
> > > > to find some agreeable solution.
> > > > >
> > > > > My current thoughts are as such:
> > > > > The platform parameter specifies a set of DPDK options that will
> > > > > be used. This
> > > > is what arm uses for its builds, x86 and ppc don't use this.
> > > > > The cpu_instruction_set sets just one configuration among the "platform"
> > > > configuration set.
> > > > > We want the build to work on most machines of the machine architecture.
> > > > That implies platform=generic (as in use config options that will
> > > > work on everything of that architecture) and
> > > > cpu_instruction_set=generic (as in use ISA that will work on all cpus of the
> > build machine architecture).
> > > > > Setting cpu_instruction_set=generic changes the build without
> > > > > cmdline options
> > > > for ppc. Thus, the expectation may be that cpu_instruction_set
> > > > should be native by default.
> > > > >
> > > > > For arm, cpu_instruction_set is ignored (and thus the value doen't
> > > > > matter),
> > > > since we can't use that without other config options (e.g. DPDK
> > > > config for an SoC (such as RTE_ARM_FEATURE_ATOMICS) used with an
> > > > invalid cpu_instuction_set). That means the only relevant parameter
> > > > for Arm is platform and if we want to have a build usable on most
> > > > machines of the build type, we have to use platform=generic.
> > > > >
> > > > > For x86 and ppc, there's no difference between native and generic
> > > > > platform (as
> > > > it's a new argument, the purpose of which is to differentiate DPDK
> > > > config across platforms, which doesn't exist for x86 and ppc - DPDK
> > > > config is the same (correct me if I'm wrong)).
> > > > >
> > > > > So it basically boils down to what should be the default value of
> > > > cpu_instruction_set when platform=generic (for platform=native, it's
> > > > obviously
> > > > native):
> > > > > 1. cpu_instruction_set=native, this would preserve the current
> > > > > behavior, but
> > > > we won't use the 'auto' default. I think we can fall back to this if
> > > > we don't agree on anything better.
> > > > > 2. cpu_instruction_set=auto, the same as
> > > > > cpu_instruction_set=generic, 3. cpu_instruction_set=generic, this
> > > > > changes behavior for ppc builds, but we
> > > > may be able to remedy this:
> > > > > Similarly to arm (arm is using platform for this, but the idea is
> > > > > the same), if
> > > > cpu_instruction_set is generic, we can do some discovery for pcc and
> > > > set the ISA accordingly (either power8 or power9). If I understand
> > > > it correctly, power8 is a different architecture from power9 (I
> > > > could be wrong on this), so this is desirable. There's some logic in
> > > > config/ppc/meson.build, but it doesn't seem sufficient as a discovery
> > mechanism between power8/power9.
> > > >
> > > > POWER8 code (Power ISA v2.07) runs on POWER9 CPUs (Power ISA v3.0),
> > > > so setting cpu_instruction_set to GENERIC is a reasonable option.
> > > > POWER10 CPUs (Power ISA v3.1) are around the corner so I want to
> > > > make sure developers can tune the application for the platform as
> > > > easily as possible.  I'm fine with supporting GENERIC, just need
> > > > clear instructions on how to build for a particular ISA.
> > > >
> > >
> > > That's good to hear. Setting cpu_instruction_set will set the ISA.
> > >
> > > We'll need to document the behavior properly, but I'm not sure where - Bruce?
> > Seems like meson_options.txt doesn't have enough room for that.
> > >
> > 
> > Yes, agreed. I suggest putting in the option help text a link to the documentation
> > where we explain the options more fully.
> > 
> > /Bruce
> 
> You mentioned in the kni cross compile patch that we should do a separate patch where we explain the option in docs and I believe this would go into that patch. Before we do that, let's figure out the best description without the link. I currently have:
> option('cpu_instruction_set', type: 'string', value: 'auto',
> 	description: 'Set the target machine ISA (instruction set architecture). Will be set according to the platform option by default.')
> option('platform', type: 'string', value: 'generic',
> 	description: 'Platform to build, either "native" or "generic".')
> 
> What do you think? Should we expand the platform description a bit?

Are there not more platform options than just those two?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option
  2021-02-25 12:54                   ` Bruce Richardson
@ 2021-02-25 12:57                     ` Juraj Linkeš
  0 siblings, 0 replies; 16+ messages in thread
From: Juraj Linkeš @ 2021-02-25 12:57 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: David Christensen, thomas, Honnappa.Nagarahalli, dev



> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Thursday, February 25, 2021 1:54 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: David Christensen <drc@linux.vnet.ibm.com>; thomas@monjalon.net;
> Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> Subject: Re: [RFC PATCH v2] build: add platform meson option
> 
> On Thu, Feb 25, 2021 at 12:51:57PM +0000, Juraj Linkeš wrote:
> >
> >
> > > -----Original Message-----
> > > From: Bruce Richardson <bruce.richardson@intel.com>
> > > Sent: Tuesday, February 23, 2021 10:43 AM
> > > To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > Cc: David Christensen <drc@linux.vnet.ibm.com>; thomas@monjalon.net;
> > > Honnappa.Nagarahalli@arm.com; dev@dpdk.org
> > > Subject: Re: [RFC PATCH v2] build: add platform meson option
> > >
> > > On Tue, Feb 23, 2021 at 08:45:09AM +0000, Juraj Linkeš wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: David Christensen <drc@linux.vnet.ibm.com>
> > > > > Sent: Monday, February 22, 2021 10:25 PM
> > > > > To: Juraj Linkeš <juraj.linkes@pantheon.tech>; Bruce Richardson
> > > > > <bruce.richardson@intel.com>
> > > > > Cc: thomas@monjalon.net; Honnappa.Nagarahalli@arm.com;
> > > dev@dpdk.org
> > > > > Subject: Re: [RFC PATCH v2] build: add platform meson option
> > > > >
> > > > >
> > > > >
> > > > > On 2/19/21 1:11 AM, Juraj Linkeš wrote:
> > > > > >
> > > > > >
> > > > > >> -----Original Message-----
> > > > > >> From: Bruce Richardson <bruce.richardson@intel.com>
> > > > > >> Sent: Wednesday, January 6, 2021 3:43 PM
> > > > > >> To: David Christensen <drc@linux.vnet.ibm.com>
> > > > > >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>;
> > > > > >> thomas@monjalon.net; Honnappa.Nagarahalli@arm.com;
> > > > > >> dev@dpdk.org
> > > > > >> Subject: Re: [RFC PATCH v2] build: add platform meson option
> > > > > >>
> > > > > >> On Tue, Jan 05, 2021 at 02:17:44PM -0800, David Christensen wrote:
> > > > > >>>>> The current meson option 'machine' should only specify the
> > > > > >>>>> ISA, which is not sufficient for Arm, where setting ISA
> > > > > >>>>> implies other setting as
> > > > > >> well.
> > > > > >>>>> Add a new meson option, 'platform', which differentiates
> > > > > >>>>> the type of the build
> > > > > >>>>> (native/generic) and sets machine accordingly, unless the
> > > > > >>>>> user chooses to override it.
> > > > > >>>>> The 'machine' option also doesn't describe very well what
> > > > > >>>>> it sets, so introduce a new option 'cpu_instruction_set',
> > > > > >>>>> but keep 'machine' for backward compatibility.
> > > > > >>>>> These two new variables, taken together, achieve what 'machine'
> > > > > >>>>> was setting per architecture - setting the ISA in x86
> > > > > >>>>> build and setting native/default 'build type' in aarch64
> > > > > >>>>> build - is now properly being set for all architectures in a uniform
> manner.
> > > > > >>>>>
> > > > > >>>>> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > > > >>>>> ---
> > > > > >>>>>    config/arm/meson.build        |  4 +--
> > > > > >>>>>    config/meson.build            | 47 +++++++++++++++++++++++++----
> ----
> > > --
> > > > > >>>>>    devtools/test-meson-builds.sh |  9 ++++---
> > > > > >>>>>    meson_options.txt             |  8 ++++--
> > > > > >>>>>    4 files changed, 47 insertions(+), 21 deletions(-)
> > > > > >>>>>
> > > > > >>>>> diff --git a/config/arm/meson.build
> > > > > >>>>> b/config/arm/meson.build index
> > > > > >>>>> 42b4e43c7..6b09a74a7 100644
> > > > > >>>>> --- a/config/arm/meson.build
> > > > > >>>>> +++ b/config/arm/meson.build
> > > > > >>>>> @@ -3,10 +3,10 @@
> > > > > >>>>>    # Copyright(c) 2017 Cavium, Inc
> > > > > >>>>>
> > > > > >>>>>    # for checking defines we need to use the correct
> > > > > >>>>> compiler flags -march_opt = '-
> > > > > >>>>> march=@0@'.format(machine)
> > > > > >>>>> +march_opt = '-march=@0@'.format(cpu_instruction_set)
> > > > > >>>>>
> > > > > >>>>>    arm_force_native_march = false -arm_force_default_march
> > > > > >>>>> = (machine == 'default')
> > > > > >>>>> +arm_force_default_march = (platform == 'generic')
> > > > > >>>>>
> > > > > >>>>>    flags_common_default = [
> > > > > >>>>>    	# Accelarate rte_memcpy. Be sure to run unit test
> > > > > >>>>> (memcpy_perf_autotest) diff --git a/config/meson.build
> > > > > >>>>> b/config/meson.build index a3154e29c..647116513 100644
> > > > > >>>>> --- a/config/meson.build
> > > > > >>>>> +++ b/config/meson.build
> > > > > >>>>> @@ -63,42 +63,63 @@ if not is_windows
> > > > > >>>>>    			pmd_subdir_opt)
> > > > > >>>>>    endif
> > > > > >>>>>
> > > > > >>>>> -# set the machine type and cflags for it
> > > > > >>>>> +platform = get_option('platform')
> > > > > >>>>> +
> > > > > >>>>> +# set the cpu_instruction_set type and cflags for it
> > > > > >>>>>    if meson.is_cross_build()
> > > > > >>>>> -	machine = host_machine.cpu()
> > > > > >>>>> +	cpu_instruction_set = host_machine.cpu()
> > > > > >>>>>    else
> > > > > >>>>> -	machine = get_option('machine')
> > > > > >>>>> +	cpu_instruction_set = get_option('cpu_instruction_set')
> > > > > >>>>> +	if get_option('machine') != 'auto'
> > > > > >>>>> +		warning('The "machine" option is deprecated. ' +
> > > > > >>>>> +		        'Please use "cpu_instruction_set" instead.')
> > > > > >>>>> +		if cpu_instruction_set != 'auto'
> > > > > >>>>> +			error('Setting both "machine" and ' +
> > > > > >>>>> +			      '"cpu_instruction_set" is unsupported.')
> > > > > >>>>> +		endif
> > > > > >>>>> +		cpu_instruction_set = get_option('machine')
> > > > > >>>>> +	endif
> > > > > >>>>> +endif
> > > > > >>>>> +
> > > > > >>>>> +if platform == 'native'
> > > > > >>>>> +	if cpu_instruction_set == 'auto'
> > > > > >>>>> +		cpu_instruction_set = 'native'
> > > > > >>>>> +	endif
> > > > > >>>>> +elif platform == 'generic'
> > > > > >>>>> +	if cpu_instruction_set == 'auto'
> > > > > >>>>> +		cpu_instruction_set = 'default'
> > > > > >>>>> +	endif
> > > > > >>>>>    endif
> > > > > >>>>>
> > > > > >>>>> -# machine type 'default' is special, it defaults to the
> > > > > >>>>> per arch agreed common
> > > > > >>>>> +if cpu_instruction_set == 'default'
> > > > > >>>>> +# cpu_instruction_set type 'default' is special, it
> > > > > >>>>> +defaults to the per arch agreed common
> > > > > >>>>>    # minimal baseline needed for DPDK.
> > > > > >>>>>    # 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 host_machine.cpu_family().startswith('x86')
> > > > > >>>>>    		# matches the old pre-meson build systems default
> > > > > >>>>> -		machine = 'corei7'
> > > > > >>>>> +		cpu_instruction_set = 'corei7'
> > > > > >>>>>    	elif host_machine.cpu_family().startswith('arm')
> > > > > >>>>> -		machine = 'armv7-a'
> > > > > >>>>> +		cpu_instruction_set = 'armv7-a'
> > > > > >>>>>    	elif host_machine.cpu_family().startswith('aarch')
> > > > > >>>>>    		# arm64 manages defaults in config/arm/meson.build
> > > > > >>>>> -		machine = 'default'
> > > > > >>>>> +		cpu_instruction_set = 'default'
> > > > > >>>>>    	elif host_machine.cpu_family().startswith('ppc')
> > > > > >>>>> -		machine = 'power8'
> > > > > >>>>> +		cpu_instruction_set = 'power8'
> > > > > >>>>>    	endif
> > > > > >>>>>    endif
> > > > > >>>
> > > > > >>> This change forces the build on a P9 system to use the P8 instruction
> set.
> > > > > >>> Prior to this change the "native" machine type was used
> > > > > >>> which resulted in P9 instructions when built on a P9 system.
> > > > > >>> How do I force the build to use the
> > > > > >>> power9 instruction set in this case?
> > > > > >>>
> > > > > >>> Dave
> > > > > >>
> > > > > >>  From looking at the patch, setting the "platform" to
> > > > > >> "native", or the instruction_set to "native" should do this.
> > > > > >> While I consider generic builds a good thing, I wonder if
> > > > > >> there is an expectation that "native" is always the default
> > > > > >> build type for DPDK
> > > builds?
> > > > > >>
> > > > > >> /Bruce
> > > > > >
> > > > > > I left this patch alone so that people could chime in, but
> > > > > > noone did, so let's try
> > > > > to find some agreeable solution.
> > > > > >
> > > > > > My current thoughts are as such:
> > > > > > The platform parameter specifies a set of DPDK options that
> > > > > > will be used. This
> > > > > is what arm uses for its builds, x86 and ppc don't use this.
> > > > > > The cpu_instruction_set sets just one configuration among the
> "platform"
> > > > > configuration set.
> > > > > > We want the build to work on most machines of the machine
> architecture.
> > > > > That implies platform=generic (as in use config options that
> > > > > will work on everything of that architecture) and
> > > > > cpu_instruction_set=generic (as in use ISA that will work on all
> > > > > cpus of the
> > > build machine architecture).
> > > > > > Setting cpu_instruction_set=generic changes the build without
> > > > > > cmdline options
> > > > > for ppc. Thus, the expectation may be that cpu_instruction_set
> > > > > should be native by default.
> > > > > >
> > > > > > For arm, cpu_instruction_set is ignored (and thus the value
> > > > > > doen't matter),
> > > > > since we can't use that without other config options (e.g. DPDK
> > > > > config for an SoC (such as RTE_ARM_FEATURE_ATOMICS) used with an
> > > > > invalid cpu_instuction_set). That means the only relevant
> > > > > parameter for Arm is platform and if we want to have a build
> > > > > usable on most machines of the build type, we have to use
> platform=generic.
> > > > > >
> > > > > > For x86 and ppc, there's no difference between native and
> > > > > > generic platform (as
> > > > > it's a new argument, the purpose of which is to differentiate
> > > > > DPDK config across platforms, which doesn't exist for x86 and
> > > > > ppc - DPDK config is the same (correct me if I'm wrong)).
> > > > > >
> > > > > > So it basically boils down to what should be the default value
> > > > > > of
> > > > > cpu_instruction_set when platform=generic (for platform=native,
> > > > > it's obviously
> > > > > native):
> > > > > > 1. cpu_instruction_set=native, this would preserve the current
> > > > > > behavior, but
> > > > > we won't use the 'auto' default. I think we can fall back to
> > > > > this if we don't agree on anything better.
> > > > > > 2. cpu_instruction_set=auto, the same as
> > > > > > cpu_instruction_set=generic, 3. cpu_instruction_set=generic,
> > > > > > this changes behavior for ppc builds, but we
> > > > > may be able to remedy this:
> > > > > > Similarly to arm (arm is using platform for this, but the idea
> > > > > > is the same), if
> > > > > cpu_instruction_set is generic, we can do some discovery for pcc
> > > > > and set the ISA accordingly (either power8 or power9). If I
> > > > > understand it correctly, power8 is a different architecture from
> > > > > power9 (I could be wrong on this), so this is desirable. There's
> > > > > some logic in config/ppc/meson.build, but it doesn't seem
> > > > > sufficient as a discovery
> > > mechanism between power8/power9.
> > > > >
> > > > > POWER8 code (Power ISA v2.07) runs on POWER9 CPUs (Power ISA
> > > > > v3.0), so setting cpu_instruction_set to GENERIC is a reasonable option.
> > > > > POWER10 CPUs (Power ISA v3.1) are around the corner so I want to
> > > > > make sure developers can tune the application for the platform
> > > > > as easily as possible.  I'm fine with supporting GENERIC, just
> > > > > need clear instructions on how to build for a particular ISA.
> > > > >
> > > >
> > > > That's good to hear. Setting cpu_instruction_set will set the ISA.
> > > >
> > > > We'll need to document the behavior properly, but I'm not sure where -
> Bruce?
> > > Seems like meson_options.txt doesn't have enough room for that.
> > > >
> > >
> > > Yes, agreed. I suggest putting in the option help text a link to the
> > > documentation where we explain the options more fully.
> > >
> > > /Bruce
> >
> > You mentioned in the kni cross compile patch that we should do a separate
> patch where we explain the option in docs and I believe this would go into that
> patch. Before we do that, let's figure out the best description without the link. I
> currently have:
> > option('cpu_instruction_set', type: 'string', value: 'auto',
> > 	description: 'Set the target machine ISA (instruction set
> > architecture). Will be set according to the platform option by default.')
> option('platform', type: 'string', value: 'generic',
> > 	description: 'Platform to build, either "native" or "generic".')
> >
> > What do you think? Should we expand the platform description a bit?
> 
> Are there not more platform options than just those two?

We're adding more in http://patches.dpdk.org/project/dpdk/patch/1612361037-12746-3-git-send-email-juraj.linkes@pantheon.tech/. I don't think there are more than two without that.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-02-25 12:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-26 15:47 [dpdk-dev] [RFC PATCH v1] build: add platform meson option Juraj Linkeš
2020-11-26 16:02 ` Bruce Richardson
2020-11-27  8:31   ` Juraj Linkeš
2020-11-27 14:07     ` Bruce Richardson
2020-12-23 11:23       ` Juraj Linkeš
2021-01-04 11:52 ` [dpdk-dev] [RFC PATCH v2] " Juraj Linkeš
2021-01-04 11:59   ` Juraj Linkeš
2021-01-05 22:17     ` David Christensen
2021-01-06 14:42       ` Bruce Richardson
2021-02-19  9:11         ` Juraj Linkeš
2021-02-22 21:25           ` David Christensen
2021-02-23  8:45             ` Juraj Linkeš
2021-02-23  9:43               ` Bruce Richardson
2021-02-25 12:51                 ` Juraj Linkeš
2021-02-25 12:54                   ` Bruce Richardson
2021-02-25 12:57                     ` Juraj Linkeš

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git