Re: [dpdk-dev] [PATCH v8 02/14] build: rename Arm build variables

[Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>]

<snip>

> >
> > Rename Arm build variables and values so that they better conform to
> > Arm specifications. Also rename generically sounding variable to names
> > that better capture what the variables hold.
> >
> > Rename machine_args_generic to part_number_config_arm since the
> > variable contains more than just the generic machine args and is used
> > mainly as the fallback arm configuration.
> > Rename the default machine args to generic machine args to reflect that.
> > The rest of the variables are self-explanatory.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >  config/arm/arm64_armada_linux_gcc    |   2 +-
> >  config/arm/arm64_armv8_linux_gcc     |   8 +-
> >  config/arm/arm64_bluefield_linux_gcc |   4 +-
> >  config/arm/arm64_dpaa_linux_gcc      |   2 +-
> >  config/arm/arm64_emag_linux_gcc      |   2 +-
> >  config/arm/arm64_n1sdp_linux_gcc     |   4 +-
> >  config/arm/arm64_octeontx2_linux_gcc |   4 +-
> >  config/arm/arm64_stingray_linux_gcc  |   4 +-
> >  config/arm/arm64_thunderx2_linux_gcc |   4 +-
> >  config/arm/arm64_thunderx_linux_gcc  |   2 +-
> >  config/arm/meson.build               | 110 +++++++++++++--------------
> >  11 files changed, 73 insertions(+), 73 deletions(-)
> >
> > diff --git a/config/arm/arm64_armada_linux_gcc
> > b/config/arm/arm64_armada_linux_gcc
> > index fa40c0398..52c5f4476 100644
> > --- a/config/arm/arm64_armada_linux_gcc
> > +++ b/config/arm/arm64_armada_linux_gcc
> > @@ -14,4 +14,4 @@ cpu = 'armv8-a'
> >  endian = 'little'
> >
> >  [properties]
> > -implementor_id = '0x56'
> > +implementer_id = '0x56'
> > diff --git a/config/arm/arm64_armv8_linux_gcc
> > b/config/arm/arm64_armv8_linux_gcc
> > index 88f0ff9da..13ee8b223 100644
> > --- a/config/arm/arm64_armv8_linux_gcc
> > +++ b/config/arm/arm64_armv8_linux_gcc
> > @@ -13,10 +13,10 @@ cpu = 'armv8-a'
> >  endian = 'little'
> >
> >  [properties]
> > -implementor_id = 'generic'
> > +implementer_id = 'generic'
> >
> > -# Valid options for Arm's implementor_pn:
> > -# 'default': valid for all armv8-a architectures (default value)
> > +# Valid options for Arm's part_number:
> > +# 'generic': valid for all armv8-a architectures (default value)
> >  # '0xd03':   cortex-a53
> >  # '0xd04':   cortex-a35
> >  # '0xd05':   cortex-a55
> > @@ -25,4 +25,4 @@ implementor_id = 'generic'
> >  # '0xd09':   cortex-a73
> >  # '0xd0a':   cortex-a75
> >  # '0xd0b':   cortex-a76
> > -implementor_pn = 'default'
> > +part_number = 'generic'
> 
> <snip>
> 
> > diff --git a/config/arm/meson.build b/config/arm/meson.build index
> > d4066ade8..7fe0f3f3c 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -29,58 +29,58 @@ flags_common_default = [
>                                                                             ^^^^^^ this causes confusion. I
> think can be just 'flags_common'.
> 
> >  	['RTE_ARM_USE_WFE', false],
> >  ]
> >
> > -flags_generic = [
> > +flags_implementer_generic = [
> >  	['RTE_MACHINE', '"armv8a"'],
> >  	['RTE_MAX_LCORE', 256],
> >  	['RTE_USE_C11_MEM_MODEL', true],
> >  	['RTE_CACHE_LINE_SIZE', 128]]
> 
> <snip>
> 
> >
> > -flags_default_extra = []
> > -flags_thunderx_extra = [
> > +flags_part_number_default = []
> Should this be "flags_part_number_generic"?
> 
> > +flags_part_number_thunderx = [
> >  	['RTE_MACHINE', '"thunderx"'],
> >  	['RTE_USE_C11_MEM_MODEL', false]]
> 
> <snip>
> 
> >
> >  ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page
> > G7-5321) -impl_generic = ['Generic armv8', flags_generic,
> > machine_args_generic]
> > -impl_0x41 = ['Arm', flags_arm, machine_args_generic]
> > -impl_0x42 = ['Broadcom', flags_generic, machine_args_generic]
> > -impl_0x43 = ['Cavium', flags_cavium, machine_args_cavium]
> > -impl_0x44 = ['DEC', flags_generic, machine_args_generic]
> > -impl_0x49 = ['Infineon', flags_generic, machine_args_generic]
> > -impl_0x4d = ['Motorola', flags_generic, machine_args_generic]
> > -impl_0x4e = ['NVIDIA', flags_generic, machine_args_generic]
> > -impl_0x50 = ['Ampere Computing', flags_emag, machine_args_emag]
> > -impl_0x51 = ['Qualcomm', flags_generic, machine_args_generic]
> > -impl_0x53 = ['Samsung', flags_generic, machine_args_generic]
> > -impl_0x56 = ['Marvell ARMADA', flags_armada, machine_args_generic]
> > -impl_0x69 = ['Intel', flags_generic, machine_args_generic] -impl_dpaa
> > = ['NXP DPAA', flags_dpaa, machine_args_generic]
> > +implementer_generic = ['Generic armv8', flags_implementer_generic,
> > +part_number_config_arm]
> > +implementer_0x41 = ['Arm', flags_implementer_arm,
> > +part_number_config_arm]
> > +implementer_0x42 = ['Broadcom', flags_implementer_generic,
> > +part_number_config_arm]
> > +implementer_0x43 = ['Cavium', flags_implementer_cavium,
> > +part_number_config_cavium]
> > +implementer_0x44 = ['DEC', flags_implementer_generic,
> > +part_number_config_arm]
> > +implementer_0x49 = ['Infineon', flags_implementer_generic,
> > +part_number_config_arm] implementer_0x4d = ['Motorola',
> > +flags_implementer_generic, part_number_config_arm] implementer_0x4e
> =
> > +['NVIDIA', flags_implementer_generic, part_number_config_arm]
> > +implementer_0x50 = ['Ampere Computing', flags_implementer_emag,
> > +part_number_config_emag]
> > +implementer_0x51 = ['Qualcomm', flags_implementer_generic,
> > +part_number_config_arm]
> > +implementer_0x53 = ['Samsung', flags_implementer_generic,
> > +part_number_config_arm]
> > +implementer_0x56 = ['Marvell ARMADA', flags_implementer_armada,
> > +part_number_config_arm]
> > +implementer_0x69 = ['Intel', flags_implementer_generic,
> > +part_number_config_arm] implementer_dpaa = ['NXP DPAA',
> > +flags_implementer_dpaa, part_number_config_arm]
> >
> >  dpdk_conf.set('RTE_ARCH_ARM', 1)
> >  dpdk_conf.set('RTE_FORCE_INTRINSICS', 1) @@ -145,13 +145,13 @@ else
> >  	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> >  	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> >
> > -	machine = []
> > -	cmd_generic = ['generic', '', '', 'default', '']
> > +	implementer_config = []
> > +	cmd_generic = ['generic', '', '', 'generic', '']
> >  	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'
> > +		implementer_config = implementer_generic
> > +		part_number = 'generic'
> >  	elif not meson.is_cross_build()
> >  		# The script returns ['Implementer', 'Variant', 'Architecture',
> >  		# 'Primary Part number', 'Revision'] @@ -162,19 +162,19 @@
> else
> >  			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
> > +		implementer_config = get_variable('implementer_' +
> > cmd_output[0], ['generic'])
> > +		if implementer_config[0] == 'generic'
> > +			implementer_config = implementer_generic
> >  			cmd_output = cmd_generic
> >  		endif
> > -		impl_pn = cmd_output[3]
> > +		part_number = cmd_output[3]
> >  		if arm_force_native_march == true
> > -			impl_pn = 'native'
> > +			part_number = 'native'
> >  		endif
> >  	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)
> > +		implementer_id =
> > meson.get_cross_property('implementer_id', 'generic')
> > +		part_number = meson.get_cross_property('part_number',
> > 'generic')
> > +		implementer_config = get_variable('implementer_' +
> > implementer_id)
> >  	endif
> >
> >  	# Apply Common Defaults. These settings may be overwritten by
> > machine @@ -185,22 +185,22 @@ else
> >  		endif
> >  	endforeach
> >
> > -	message('Implementer : ' + machine[0])
> > -	foreach flag: machine[1]
> > +	message('Implementer : ' + implementer_config[0])
> > +	foreach flag: implementer_config[1]
> >  		if flag.length() > 0
> >  			dpdk_conf.set(flag[0], flag[1])
> >  		endif
> >  	endforeach
> >
> > -	foreach marg: machine[2]
> > -		if marg[0] == impl_pn
> > +	foreach marg: implementer_config[2]
> > +		if marg[0] == part_number
> >  			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)
> > +			foreach flag: marg.get(2, flags_part_number_default)
>                                                                                                                              ^^^^^^
> generic?
> What does 'flags_part_number_default' (or 'flags_default_extra' earlier)
> mean? Why does it need to be applied to each part number? (this question
> does not belong to the changes in this patch)
Please ignore the question, I see it is fixed in 3/14

> 
> >  				if flag.length() > 0
> >  					dpdk_conf.set(flag[0], flag[1])
> >  				endif
> > --
> > 2.20.1
> Otherwise, it looks good.
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>