[dpdk-dev] [PATCH v4 2/6] build: refactor Arm build

Honnappa Nagarahalli Honnappa.Nagarahalli at arm.com
Thu Oct 29 21:54:07 CET 2020


> > <snip>
> >
> > > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > > 491842cad..6c31ab167 100644
> > > --- a/config/arm/meson.build
> > > +++ b/config/arm/meson.build
> > > @@ -3,12 +3,12 @@
> > >  # Copyright(c) 2017 Cavium, Inc
> > >  # Copyright(c) 2020 PANTHEON.tech s.r.o.
> > >
> > > -# for checking defines we need to use the correct compiler flags
> > > -march_opt = '-march=@0@'.format(machine)
> > > -
> > > +# set arm_force_native_march if you want to use machine args below
> > > +# instead of discovered values; only works when doing an actual
> > > +native build
> > >  arm_force_native_march = false
> > > -arm_force_generic_march = (machine == 'generic')
> > > +native_machine_args = ['-march=native', '-mtune=native']
> > >
> >
> > [...]
> >
> > > -
> > > -machine_args_default = [
> > > -	['default', ['-march=armv8-a+crc', '-moutline-atomics']],
> > > -	['native', ['-march=native']],
> > > -	['0xd03', ['-mcpu=cortex-a53']],
> > > -	['0xd04', ['-mcpu=cortex-a35']],
> > > -	['0xd07', ['-mcpu=cortex-a57']],
> > > -	['0xd08', ['-mcpu=cortex-a72']],
> > > -	['0xd09', ['-mcpu=cortex-a73']],
> > > -	['0xd0a', ['-mcpu=cortex-a75']],
> > > -	['0xd0b', ['-mcpu=cortex-a76']],
> > > -	['0xd0c', ['-march=armv8.2-a+crc+crypto', '-mcpu=neoverse-n1'],
> > > flags_n1sdp_extra]]
> > > -
> > > -machine_args_cavium = [
> > > -	['default', ['-march=armv8-a+crc+crypto','-mcpu=thunderx']],
> > > -	['native', ['-march=native']],
> > > -	['0xa1', ['-mcpu=thunderxt88'], flags_thunderx_extra],
> > > -	['0xa2', ['-mcpu=thunderxt81'], flags_thunderx_extra],
> > > -	['0xa3', ['-mcpu=thunderxt83'], flags_thunderx_extra],
> > > -	['0xaf', ['-march=armv8.1-a+crc+crypto','-mcpu=thunderx2t99'],
> > > flags_thunderx2_extra],
> > > -	['0xb2', ['-march=armv8.2-a+crc+crypto+lse','-mcpu=octeontx2'],
> > > flags_octeontx2_extra]]
> > > -
> > > -machine_args_emag = [
> > > -	['default', ['-march=armv8-a+crc+crypto', '-mtune=emag']],
> > > -	['native', ['-march=native']]]
> > > +	['RTE_USE_C11_MEM_MODEL', true]
> > > +]
> > > +# arm config (implementer 0x41) is the default config
> > > +pn_config_default
> > What does it mean by 'default' here? I am somewhat confused between
> 'default'
> > and 'generic'. We should look to remove 'default' as much as possible
> > and stick with 'generic'.
> >
> 
> This default means what default means, no special meaning, that is if there isn't
> a more specific configuration, default to this one. It's possible that generic is
> better, but now that I think about it, using something else than default or
> generic might be the best to avoid confusion. Possibly just part_number_arm,
> which will make it in line with the other var names.
Agree, better to call it 'part_number_arm'.

> 
> > > += {
> > > +	'generic': [['-march=armv8-a+crc', '-moutline-atomics']],
> > I like that we have taken out 'native' from this list. Would it be
> > possible to take out 'generic' from this and others below. This is
> > because the binary built with 'generic' build should run on any Arm
> > platform. There is no dependency on any underlying platform.
> >
> 
> This actually means generic part for the implementer, not generic for
> everything. I understand this is here to produce binaries that would run on
> everything from that impelemeter (in line of what you mention below, this
> would be implementer-generic configuration, a fourth category). In my
> patchset, it's also a fallback when building for an unknown part number from
> the implementer.
We do not need implementer-generic binaries/build. However, we will have some parameters that are common across all the part numbers from that implementer (probably we should not call it as 'implementer-generic' to avoid confusion. May be 'implementer-common-flags' or 'implementer-flags-extra'). Those parameters can be used for every part number.

If we know the implementer, we will have the part number as well (this is part of the Arm architecture definition). Unknown part number should result in an error message.

> 
> Since this is not generic for everything, only for the implementer, we're lacking
> the true common default machine args for everything.
> 
> > > +	'0xd03': [['-mcpu=cortex-a53']],
> > > +	'0xd04': [['-mcpu=cortex-a35']],
> > > +	'0xd07': [['-mcpu=cortex-a57']],
> > > +	'0xd08': [['-mcpu=cortex-a72']],
> > > +	'0xd09': [['-mcpu=cortex-a73']],
> > > +	'0xd0a': [['-mcpu=cortex-a75']],
> > > +	'0xd0b': [['-mcpu=cortex-a76']],
> > > +	'0xd0c': [['-march=armv8.2-a+crc+crypto', '-mcpu=neoverse-n1'],
> > > +flags_n1sdp_extra]
> > 'flags_n1sdp_extra' does not fit here. For the part number '0xd0c'
> > there could be multiple SoCs (N1SDP is one of them). So, if the SoC is
> > not known, but if we know that the CPU is N1, then we should build a N1-
> Generic build.
> 
> This should be core-generic configuration, I can rename it to
> flags_n1generic_extra.
Agree.

> 
> > So, from my perspective, there are 3 kinds of binaries:
> > 1) generic - best portability -  (possibly) least optimized for a
> > given platform
> > 2) Core-Generic (for ex: N1-generic) - Portable on all N1 based SoCs
> > only - Optimized for N1 cores
> > 3) SoC specific - (possibly) Not portable - Most optimized for the SoC
> >
> 
> The fourth category I mentioned above, implementer-generic, is used in how
We should not have this category.

> the arm configuration is currently processed:
> 1) default configuration
> Added to/overwritten by
> 2) implementer configuration (one of which is the configuration for generic
> build) Added to/overwritten by
This should be just parameters that are common across all the part numbers of that implementer (for ex: RTE_CACHE_LINE_SIZE = 128), not a build.

> 3) part number (or core-generic) configuration Added to/overwritten by
> 4) SoC configuration (what we're adding now)
> 
> It's not organized as cleanly - the machine args contain both 2) and 3).
> Depending on what we want to do with the 'generic' part number I'll adjust the
> config organization.
> 
> >  } pn_config_cavium = {
> > > +	'generic': [['-march=armv8-a+crc+crypto', '-mcpu=thunderx']],
> > 'generic' does not make sense here.
> >
> > > +	'0xa1': [['-mcpu=thunderxt88'], flags_thunderx_extra],
> > > +	'0xa2': [['-mcpu=thunderxt81'], flags_thunderx_extra],
> > > +	'0xa3': [['-mcpu=thunderxt83'], flags_thunderx_extra],
> > > +	'0xaf': [['-march=armv8.1-a+crc+crypto','-mcpu=thunderx2t99'],
> > > flags_thunderx2_extra],
> > > +	'0xb2': [['-march=armv8.2-a+crc+crypto+lse','-mcpu=octeontx2'],
> > > +flags_octeontx2_extra], } pn_config_emag = {
> > > +	'generic': [['-march=armv8-a+crc+crypto', '-mtune=emag']] }
> > Same here.
> > I understand that this is coming from the existing code. But, I think
> > we should try to set it right.
> >
> 
> The generic config for these makes sense if a fourth category, implementer-
> generic makes sense.
> For example, for part_number_config_emag this means: for implementer
> _0x50 (Ampere Computing) use the generic machine args if there's nothing
> more specific (which there isn't - no other part number).
There should be a part number for this. Not sure why it is not present here. I will find out the info.

> 
> > >
> > >  ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page
> > > G7-5321)
> > Nit, Would be good to remove the reference to the doc
> >
> 
> Is that because the docs mentioned here may not be the most recent? It was
> useful to me in understanding where the implementer/part number values
> come from.
Yes, mainly because the spec gets updated. Instead you could say "Refer to MIDR in Arm Architecture Reference Manual")

> 
> > > -impl_generic = ['Generic armv8', flags_generic,
> > > machine_args_default]
> > > -impl_0x41 = ['Arm', flags_arm, machine_args_default]
> > > -impl_0x42 = ['Broadcom', flags_generic, machine_args_default]
> > > -impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
> > > -impl_0x44 = ['DEC', flags_generic, machine_args_default]
> > > -impl_0x49 = ['Infineon', flags_generic, machine_args_default]
> > > -impl_0x4d = ['Motorola', flags_generic, machine_args_default]
> > > -impl_0x4e = ['NVIDIA', flags_generic, machine_args_default]
> > > -impl_0x50 = ['Ampere Computing', flags_emag, machine_args_emag]
> > > -impl_0x51 = ['Qualcomm', flags_generic, machine_args_default]
> > > -impl_0x53 = ['Samsung', flags_generic, machine_args_default]
> > > -impl_0x56 = ['Marvell ARMADA', flags_armada, machine_args_default]
> > > -impl_0x69 = ['Intel', flags_generic, machine_args_default]
> > > -impl_dpaa = ['NXP DPAA', flags_dpaa, machine_args_default]
> > > +impl_generic = ['Generic armv8', flags_generic, pn_config_default]
> > > +impl_0x41 = ['Arm', flags_arm, pn_config_default]
> > > +impl_0x42 = ['Broadcom', flags_generic, pn_config_default]
> > > +impl_0x43 = ['Cavium', flags_cavium, pn_config_cavium]
> > > +impl_0x44 = ['DEC', flags_generic, pn_config_default]
> > > +impl_0x49 = ['Infineon', flags_generic, pn_config_default]
> > > +impl_0x4d = ['Motorola', flags_generic, pn_config_default]
> > > +impl_0x4e = ['NVIDIA', flags_generic, pn_config_default]
> > > +impl_0x50 = ['Ampere Computing', flags_emag, pn_config_emag]
> > > +impl_0x51 = ['Qualcomm', flags_generic, pn_config_default]
> > > +impl_0x53 = ['Samsung', flags_generic, pn_config_default]
> > > +impl_0x56 = ['Marvell ARMADA', flags_armada, pn_config_default]
> > > +impl_0x69 = ['Intel', flags_generic, pn_config_default] impl_dpaa =
> > > +['NXP DPAA', flags_dpaa, pn_config_default]
> > >
> > >  dpdk_conf.set('RTE_ARCH_ARM', 1)
> > >  dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
> > >
> > >  if dpdk_conf.get('RTE_ARCH_32')
> > > +	# armv7 build
> > >  	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > >  	dpdk_conf.set('RTE_ARCH_ARMv7', 1)
> > >  	# the minimum architecture supported, armv7-a, needs the following,
> > >  	# mk/machine/armv7a/rte.vars.mk sets it too
> > >  	machine_args += '-mfpu=neon'
> > >  else
> > > -	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> > > -	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> > > +	# aarch64 build
> > > +	if not meson.is_cross_build()
> > > +		if machine == 'generic'
> > > +			# default build
> >                                               ^^^^^^^^^^^ Generic build?
> >
> 
> Already addressed in the next version.
> 
> > > +			impl_config = impl_generic
> > > +			part_number = 'generic'
> > > +		else
> > > +			# native build
> > > +			# The script returns ['Implementer', 'Variant',
> > > 'Architecture',
> > > +			# 'Primary Part number', 'Revision']
> > > +			detect_vendor = find_program(join_paths(
> > > +					meson.current_source_dir(),
> > > 'armv8_machine.py'))
> > > +			cmd = run_command(detect_vendor.path())
> > > +			if cmd.returncode() == 0
> > > +				cmd_output =
> > > cmd.stdout().to_lower().strip().split(' ')
> > > +			endif
> > > +			if arm_force_native_march == true
> > > +				part_number = 'native'
> > > +			else
> > > +				part_number = cmd_output[3]
> > > +			endif
> > > +			# Set to generic implementer if implementer is not
> > > found
> > > +			impl_config = get_variable('impl_' + cmd_output[0],
> > > 'impl_generic')
> > > +		endif
> > > +	else
> > > +		# cross build
> > > +		impl_id = meson.get_cross_property('implementer_id',
> > > 'generic')
> > > +		part_number = meson.get_cross_property('part_number',
> > > 'generic')
> > > +		impl_config = get_variable('impl_' + impl_id)
> > > +	endif
> > >
> > > -	machine = []
> > > -	cmd_generic = ['generic', '', '', 'default', '']
> > > -	cmd_output = cmd_generic # Set generic by default
> > > -	machine_args = [] # Clear previous machine args
> > > -	if arm_force_generic_march and not meson.is_cross_build()
> > > -		machine = impl_generic
> > > -		impl_pn = 'default'
> > > +	message('Arm implementer: ' + impl_config[0])
> > > +	message('Arm part number: ' + part_number)
> > > +
> > > +	implementer_flags = impl_config[1]
> > > +	part_number_config = impl_config[2]
> > > +
> > > +	if part_number_config.has_key(part_number)
> > > +		# use the specified part_number machine args if found
> > > +		part_number_config = part_number_config[part_number]
> > > +	elif part_number == 'native'
> > > +		# use native machine args
> > > +		part_number_config = [[native_machine_args]]
> > >  	elif not meson.is_cross_build()
> > > -		# The script returns ['Implementer', 'Variant', 'Architecture',
> > > -		# 'Primary Part number', 'Revision']
> > > -		detect_vendor = find_program(join_paths(
> > > -				meson.current_source_dir(),
> > > 'armv8_machine.py'))
> > > -		cmd = run_command(detect_vendor.path())
> > > -		if cmd.returncode() == 0
> > > -			cmd_output = cmd.stdout().to_lower().strip().split('
> > > ')
> > > -		endif
> > > -		# Set to generic if variable is not found
> > > -		machine = get_variable('impl_' + cmd_output[0], ['generic'])
> > > -		if machine[0] == 'generic'
> > > -			machine = impl_generic
> > > -			cmd_output = cmd_generic
> > > -		endif
> > > -		impl_pn = cmd_output[3]
> > > -		if arm_force_native_march == true
> > > -			impl_pn = 'native'
> > > -		endif
> > > +		# default to generic machine args if part_number is not found
> > > +		# and not forcing native machine args
> > > +		# but don't default in cross-builds; if part_number is specified
> > > +		# incorrectly in a cross-file, it needs to be fixed there
> > > +		part_number_config = part_number_config['generic']
> > >  	else
> > > -		impl_id = meson.get_cross_property('implementor_id',
> > > 'generic')
> > > -		impl_pn = meson.get_cross_property('implementor_pn',
> > > 'default')
> > > -		machine = get_variable('impl_' + impl_id)
> > > +		# cross build and part number is not in part_number_config
> > > +		error('Cross build part number 0 at 0 not
> > > found.'.format(part_number))
> > >  	endif
> > >
> > > -	# Apply Common Defaults. These settings may be overwritten by
> > > machine
> > > -	# settings later.
> > > -	foreach flag: flags_common_default
> > > -		if flag.length() > 0
> > > -			dpdk_conf.set(flag[0], flag[1])
> > > +	dpdk_flags = flags_common_default + implementer_flags
> > > +
> > > +	if part_number_config.length() > 1
> > > +		dpdk_flags += part_number_config[1]
> > > +	endif
> > > +
> > > +	machine_args = [] # Clear previous machine args
> > > +	foreach flag: part_number_config[0]
> > > +		if cc.has_argument(flag)
> > > +			machine_args += flag
> > >  		endif
> > >  	endforeach
> > >
> > > -	message('Implementer : ' + machine[0])
> > > -	foreach flag: machine[1]
> > > +	foreach flag: dpdk_flags
> > >  		if flag.length() > 0
> > >  			dpdk_conf.set(flag[0], flag[1])
> > >  		endif
> > >  	endforeach
> > > -
> > > -	foreach marg: machine[2]
> > > -		if marg[0] == impl_pn
> > > -			foreach flag: marg[1]
> > > -				if cc.has_argument(flag)
> > > -					machine_args += flag
> > > -				endif
> > > -			endforeach
> > > -			# Apply any extra machine specific flags.
> > > -			foreach flag: marg.get(2, flags_default_extra)
> > > -				if flag.length() > 0
> > > -					dpdk_conf.set(flag[0], flag[1])
> > > -				endif
> > > -			endforeach
> > > -		endif
> > > -	endforeach
> > >  endif
> > > -message(machine_args)
> > > +
> > > +message('Using machine args: @0@'.format(machine_args))
> > >
> > >  if (cc.get_define('__ARM_NEON', args: machine_args) != '' or
> > >      cc.get_define('__aarch64__', args: machine_args) != '')
> > > --
> > > 2.20.1



More information about the dev mailing list