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-25 12:57:48

-----Original Message-----
From: Bruce Richardson <redacted>
Sent: Thursday, February 25, 2021 1:54 PM
To: Juraj Linkeš <redacted>
Cc: David Christensen <redacted>; 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:
quoted
quoted
-----Original Message-----
From: Bruce Richardson <redacted>
Sent: Tuesday, February 23, 2021 10:43 AM
To: Juraj Linkeš <redacted>
Cc: David Christensen <redacted>; 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:
quoted
quoted
-----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
quoted
quoted
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.
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
quoted
Signed-off-by: Juraj Linkeš <redacted>
---
   config/arm/meson.build        |  4 +--
   config/meson.build            | 47 +++++++++++++++++++++++++----
----
quoted
quoted
--
quoted
quoted
quoted
quoted
quoted
quoted
quoted
   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.
quoted
quoted
quoted
quoted
quoted
quoted
quoted
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?
quoted
quoted
quoted
quoted
/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"
quoted
quoted
quoted
quoted
configuration set.
quoted
We want the build to work on most machines of the machine
architecture.
quoted
quoted
quoted
quoted
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
quoted
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
quoted
quoted
quoted
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.
quoted
quoted
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?
quoted
quoted
Seems like meson_options.txt doesn't have enough room for that.
quoted
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:
quoted
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',
quoted
	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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help