Thread (33 messages) 33 messages, 5 authors, 2021-07-09

Re: [dpdk-dev] [RFC PATCH v2] build: add platform meson option

From: Juraj Linkeš <hidden>
Date: 2021-02-23 08:45:14

-----Original Message-----
From: David Christensen <redacted>
Sent: Monday, February 22, 2021 10:25 PM
To: Juraj Linkeš <redacted>; Bruce Richardson
[off-list ref]
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:
quoted
quoted
-----Original Message-----
From: Bruce Richardson <redacted>
Sent: Wednesday, January 6, 2021 3:43 PM
To: David Christensen <redacted>
Cc: Juraj Linkeš <redacted>; 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:
quoted
quoted
quoted
The current meson option 'machine' should only specify the ISA,
which is not sufficient for Arm, where setting ISA implies other
setting as
well.
quoted
quoted
quoted
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š <redacted>
---
   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.
quoted
quoted
quoted
   # 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.
quoted
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.
quoted
The cpu_instruction_set sets just one configuration among the "platform"
configuration set.
quoted
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).
quoted
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.
quoted
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.
quoted
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)).
quoted
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):
quoted
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.
quoted
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:
quoted
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
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help