[v4,2/6] build: refactor Arm build
Checks
Commit Message
* Rename variables to have names that better describe what the variables
store
* Remove unused or superfluous variables
* Change a list to dictionary where key lookup is needed
* Add informatory comments in the code
* Minor code restructure and reformatting
Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
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 | 247 +++++++++++++++------------
11 files changed, 153 insertions(+), 130 deletions(-)
Comments
<snip>
I think we need to split this further. Few suggestions below.
>
> * Rename variables to have names that better describe what the variables
> store
This should be a separate commit.
> * Remove unused or superfluous variables
Same here
> * Change a list to dictionary where key lookup is needed
Same here
> * Add informatory comments in the code
> * Minor code restructure and reformatting
Same for this
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
> 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 | 247 +++++++++++++++------------
> 11 files changed, 153 insertions(+), 130 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'
Implementor and implementer mean the same. Looked at Arm specs, they use 'implementer'. So, I am fine.
> 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'
Same here, Arm specs refer to this as 'PartNumber'. So, this should be fine.
I like 'generic' for part_number here.
> diff --git a/config/arm/arm64_bluefield_linux_gcc
> b/config/arm/arm64_bluefield_linux_gcc
> index 86797d23c..b79389d85 100644
> --- a/config/arm/arm64_bluefield_linux_gcc
> +++ b/config/arm/arm64_bluefield_linux_gcc
> @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> endian = 'little'
>
> [properties]
> -implementor_id = '0x41'
> -implementor_pn = '0xd08'
> +implementer_id = '0x41'
> +part_number = '0xd08'
> diff --git a/config/arm/arm64_dpaa_linux_gcc
> b/config/arm/arm64_dpaa_linux_gcc index 1a4682154..573ae7e42 100644
> --- a/config/arm/arm64_dpaa_linux_gcc
> +++ b/config/arm/arm64_dpaa_linux_gcc
> @@ -14,4 +14,4 @@ cpu = 'armv8-a'
> endian = 'little'
>
> [properties]
> -implementor_id = 'dpaa'
> +implementer_id = 'dpaa'
> diff --git a/config/arm/arm64_emag_linux_gcc
> b/config/arm/arm64_emag_linux_gcc index 8edcd3e97..24f3d533e 100644
> --- a/config/arm/arm64_emag_linux_gcc
> +++ b/config/arm/arm64_emag_linux_gcc
> @@ -13,4 +13,4 @@ cpu = 'armv8-a'
> endian = 'little'
>
> [properties]
> -implementor_id = '0x50'
> +implementer_id = '0x50'
> diff --git a/config/arm/arm64_n1sdp_linux_gcc
> b/config/arm/arm64_n1sdp_linux_gcc
> index 022e06303..6fb3f02ea 100644
> --- a/config/arm/arm64_n1sdp_linux_gcc
> +++ b/config/arm/arm64_n1sdp_linux_gcc
> @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> endian = 'little'
>
> [properties]
> -implementor_id = '0x41'
> -implementor_pn = '0xd0c'
> +implementer_id = '0x41'
> +part_number = '0xd0c'
> diff --git a/config/arm/arm64_octeontx2_linux_gcc
> b/config/arm/arm64_octeontx2_linux_gcc
> index 365bd7cbd..ac1042806 100644
> --- a/config/arm/arm64_octeontx2_linux_gcc
> +++ b/config/arm/arm64_octeontx2_linux_gcc
> @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> endian = 'little'
>
> [properties]
> -implementor_id = '0x43'
> -implementor_pn = '0xb2'
> +implementer_id = '0x43'
> +part_number = '0xb2'
> diff --git a/config/arm/arm64_stingray_linux_gcc
> b/config/arm/arm64_stingray_linux_gcc
> index 86797d23c..b79389d85 100644
> --- a/config/arm/arm64_stingray_linux_gcc
> +++ b/config/arm/arm64_stingray_linux_gcc
> @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> endian = 'little'
>
> [properties]
> -implementor_id = '0x41'
> -implementor_pn = '0xd08'
> +implementer_id = '0x41'
> +part_number = '0xd08'
> diff --git a/config/arm/arm64_thunderx2_linux_gcc
> b/config/arm/arm64_thunderx2_linux_gcc
> index 2b41acc61..dd257745e 100644
> --- a/config/arm/arm64_thunderx2_linux_gcc
> +++ b/config/arm/arm64_thunderx2_linux_gcc
> @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> endian = 'little'
>
> [properties]
> -implementor_id = '0x43'
> -implementor_pn = '0xaf'
> +implementer_id = '0x43'
> +part_number = '0xaf'
> diff --git a/config/arm/arm64_thunderx_linux_gcc
> b/config/arm/arm64_thunderx_linux_gcc
> index 6572ab615..670764437 100644
> --- a/config/arm/arm64_thunderx_linux_gcc
> +++ b/config/arm/arm64_thunderx_linux_gcc
> @@ -13,4 +13,4 @@ cpu = 'armv8-a'
> endian = 'little'
>
> [properties]
> -implementor_id = '0x43'
> +implementer_id = '0x43'
> 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']
>
> +# common flags to all aarch64 builds, with lowest priority
> flags_common_default = [
> # Accelarate rte_memcpy. Be sure to run unit test
> (memcpy_perf_autotest)
> # to determine the best threshold in code. Refer to notes in source
> file @@ -16,8 +16,9 @@ flags_common_default = [
> ['RTE_ARCH_ARM64_MEMCPY', false],
> # ['RTE_ARM64_MEMCPY_ALIGNED_THRESHOLD', 2048],
> # ['RTE_ARM64_MEMCPY_UNALIGNED_THRESHOLD', 512],
> - # Leave below RTE_ARM64_MEMCPY_xxx options commented out,
> unless there're
> - # strong reasons.
> +
> + # Leave below RTE_ARM64_MEMCPY_xxx options commented out,
> + # unless there are strong reasons.
> # ['RTE_ARM64_MEMCPY_SKIP_GCC_VER_CHECK', false],
> # ['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
> # ['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
> @@ -28,184 +29,206 @@ flags_common_default = [
>
> ['RTE_SCHED_VECTOR', false],
> ['RTE_ARM_USE_WFE', false],
> + ['RTE_CACHE_LINE_SIZE', 128],
> + ['RTE_ARCH_ARM64', true]
> ]
>
> +# implementer specific aarch64 flags, with middle priority # (will
> +overwrite common flags)
> flags_generic = [
> ['RTE_MACHINE', '"armv8a"'],
> ['RTE_MAX_LCORE', 256],
> ['RTE_USE_C11_MEM_MODEL', true],
> - ['RTE_CACHE_LINE_SIZE', 128]]
> + ['RTE_CACHE_LINE_SIZE', 128]
> +]
Any particular reason for this change? (and similar changes below)
> flags_arm = [
> ['RTE_MACHINE', '"armv8a"'],
> ['RTE_MAX_LCORE', 16],
> ['RTE_USE_C11_MEM_MODEL', true],
> - ['RTE_CACHE_LINE_SIZE', 64]]
> + ['RTE_CACHE_LINE_SIZE', 64]
> +]
> flags_cavium = [
> ['RTE_CACHE_LINE_SIZE', 128],
> ['RTE_MAX_NUMA_NODES', 2],
> ['RTE_MAX_LCORE', 96],
> - ['RTE_MAX_VFIO_GROUPS', 128]]
> + ['RTE_MAX_VFIO_GROUPS', 128]
> +]
> flags_dpaa = [
> ['RTE_MACHINE', '"dpaa"'],
> ['RTE_USE_C11_MEM_MODEL', true],
> ['RTE_CACHE_LINE_SIZE', 64],
> ['RTE_MAX_NUMA_NODES', 1],
> ['RTE_MAX_LCORE', 16],
> - ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
> + ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false] ]
This is not needed
> flags_emag = [
> ['RTE_MACHINE', '"emag"'],
> - ['RTE_CACHE_LINE_SIZE', 64],
> ['RTE_MAX_NUMA_NODES', 1],
> - ['RTE_MAX_LCORE', 32]]
> + ['RTE_MAX_LCORE', 32],
> + ['RTE_CACHE_LINE_SIZE', 64]
> +]
> flags_armada = [
> ['RTE_MACHINE', '"armv8a"'],
> - ['RTE_CACHE_LINE_SIZE', 64],
> ['RTE_MAX_NUMA_NODES', 1],
> - ['RTE_MAX_LCORE', 16]]
> + ['RTE_MAX_LCORE', 16],
> + ['RTE_CACHE_LINE_SIZE', 64]
> +]
Any reason for this change?
>
> -flags_default_extra = []
> +# part number specific aarch64 flags, with highest priority # (will
> +overwrite both common and implementer specific flags)
> flags_n1sdp_extra = [
> ['RTE_MACHINE', '"n1sdp"'],
> ['RTE_MAX_NUMA_NODES', 1],
> ['RTE_MAX_LCORE', 4],
> ['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
> - ['RTE_LIBRTE_VHOST_NUMA', false]]
> + ['RTE_LIBRTE_VHOST_NUMA', false]
> +]
> flags_thunderx_extra = [
> ['RTE_MACHINE', '"thunderx"'],
> - ['RTE_USE_C11_MEM_MODEL', false]]
> + ['RTE_USE_C11_MEM_MODEL', false]
> +]
> flags_thunderx2_extra = [
> ['RTE_MACHINE', '"thunderx2"'],
> ['RTE_CACHE_LINE_SIZE', 64],
> ['RTE_MAX_NUMA_NODES', 2],
> ['RTE_MAX_LCORE', 256],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> - ['RTE_USE_C11_MEM_MODEL', true]]
> + ['RTE_USE_C11_MEM_MODEL', true]
> +]
> flags_octeontx2_extra = [
> ['RTE_MACHINE', '"octeontx2"'],
> ['RTE_MAX_NUMA_NODES', 1],
> ['RTE_MAX_LCORE', 36],
> ['RTE_ARM_FEATURE_ATOMICS', true],
> ['RTE_EAL_IGB_UIO', false],
> - ['RTE_USE_C11_MEM_MODEL', true]]
> -
> -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
'pn' here for 'part_number' is not consistent.
> += {
> + 'generic': [['-march=armv8-a+crc', '-moutline-atomics']],
> + '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] } pn_config_cavium = {
> + 'generic': [['-march=armv8-a+crc+crypto', '-mcpu=thunderx']],
> + '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']] }
>
> ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321)
> -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
> + 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@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
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Tuesday, October 27, 2020 5:56 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; bruce.richardson@intel.com;
> Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com
> Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v4 2/6] build: refactor Arm build
>
> <snip>
> I think we need to split this further. Few suggestions below.
>
> >
> > * Rename variables to have names that better describe what the
> > variables store
> This should be a separate commit.
>
> > * Remove unused or superfluous variables
> Same here
>
> > * Change a list to dictionary where key lookup is needed
> Same here
>
> > * Add informatory comments in the code
>
> > * Minor code restructure and reformatting
> Same for this
>
Ok, hopefully I'll be able to separate these cleanly.
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > ---
> > 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 | 247 +++++++++++++++------------
> > 11 files changed, 153 insertions(+), 130 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'
> Implementor and implementer mean the same. Looked at Arm specs, they use
> 'implementer'. So, I am fine.
>
That's where I got this and some of the other variable name changes from.
> > 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'
> Same here, Arm specs refer to this as 'PartNumber'. So, this should be fine.
> I like 'generic' for part_number here.
>
> > diff --git a/config/arm/arm64_bluefield_linux_gcc
> > b/config/arm/arm64_bluefield_linux_gcc
> > index 86797d23c..b79389d85 100644
> > --- a/config/arm/arm64_bluefield_linux_gcc
> > +++ b/config/arm/arm64_bluefield_linux_gcc
> > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > endian = 'little'
> >
> > [properties]
> > -implementor_id = '0x41'
> > -implementor_pn = '0xd08'
> > +implementer_id = '0x41'
> > +part_number = '0xd08'
> > diff --git a/config/arm/arm64_dpaa_linux_gcc
> > b/config/arm/arm64_dpaa_linux_gcc index 1a4682154..573ae7e42 100644
> > --- a/config/arm/arm64_dpaa_linux_gcc
> > +++ b/config/arm/arm64_dpaa_linux_gcc
> > @@ -14,4 +14,4 @@ cpu = 'armv8-a'
> > endian = 'little'
> >
> > [properties]
> > -implementor_id = 'dpaa'
> > +implementer_id = 'dpaa'
> > diff --git a/config/arm/arm64_emag_linux_gcc
> > b/config/arm/arm64_emag_linux_gcc index 8edcd3e97..24f3d533e 100644
> > --- a/config/arm/arm64_emag_linux_gcc
> > +++ b/config/arm/arm64_emag_linux_gcc
> > @@ -13,4 +13,4 @@ cpu = 'armv8-a'
> > endian = 'little'
> >
> > [properties]
> > -implementor_id = '0x50'
> > +implementer_id = '0x50'
> > diff --git a/config/arm/arm64_n1sdp_linux_gcc
> > b/config/arm/arm64_n1sdp_linux_gcc
> > index 022e06303..6fb3f02ea 100644
> > --- a/config/arm/arm64_n1sdp_linux_gcc
> > +++ b/config/arm/arm64_n1sdp_linux_gcc
> > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > endian = 'little'
> >
> > [properties]
> > -implementor_id = '0x41'
> > -implementor_pn = '0xd0c'
> > +implementer_id = '0x41'
> > +part_number = '0xd0c'
> > diff --git a/config/arm/arm64_octeontx2_linux_gcc
> > b/config/arm/arm64_octeontx2_linux_gcc
> > index 365bd7cbd..ac1042806 100644
> > --- a/config/arm/arm64_octeontx2_linux_gcc
> > +++ b/config/arm/arm64_octeontx2_linux_gcc
> > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > endian = 'little'
> >
> > [properties]
> > -implementor_id = '0x43'
> > -implementor_pn = '0xb2'
> > +implementer_id = '0x43'
> > +part_number = '0xb2'
> > diff --git a/config/arm/arm64_stingray_linux_gcc
> > b/config/arm/arm64_stingray_linux_gcc
> > index 86797d23c..b79389d85 100644
> > --- a/config/arm/arm64_stingray_linux_gcc
> > +++ b/config/arm/arm64_stingray_linux_gcc
> > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > endian = 'little'
> >
> > [properties]
> > -implementor_id = '0x41'
> > -implementor_pn = '0xd08'
> > +implementer_id = '0x41'
> > +part_number = '0xd08'
> > diff --git a/config/arm/arm64_thunderx2_linux_gcc
> > b/config/arm/arm64_thunderx2_linux_gcc
> > index 2b41acc61..dd257745e 100644
> > --- a/config/arm/arm64_thunderx2_linux_gcc
> > +++ b/config/arm/arm64_thunderx2_linux_gcc
> > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > endian = 'little'
> >
> > [properties]
> > -implementor_id = '0x43'
> > -implementor_pn = '0xaf'
> > +implementer_id = '0x43'
> > +part_number = '0xaf'
> > diff --git a/config/arm/arm64_thunderx_linux_gcc
> > b/config/arm/arm64_thunderx_linux_gcc
> > index 6572ab615..670764437 100644
> > --- a/config/arm/arm64_thunderx_linux_gcc
> > +++ b/config/arm/arm64_thunderx_linux_gcc
> > @@ -13,4 +13,4 @@ cpu = 'armv8-a'
> > endian = 'little'
> >
> > [properties]
> > -implementor_id = '0x43'
> > +implementer_id = '0x43'
> > 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']
> >
> > +# common flags to all aarch64 builds, with lowest priority
> > flags_common_default = [
> > # Accelarate rte_memcpy. Be sure to run unit test
> > (memcpy_perf_autotest)
> > # to determine the best threshold in code. Refer to notes in source
> > file @@ -16,8 +16,9 @@ flags_common_default = [
> > ['RTE_ARCH_ARM64_MEMCPY', false],
> > # ['RTE_ARM64_MEMCPY_ALIGNED_THRESHOLD', 2048],
> > # ['RTE_ARM64_MEMCPY_UNALIGNED_THRESHOLD', 512],
> > - # Leave below RTE_ARM64_MEMCPY_xxx options commented out,
> > unless there're
> > - # strong reasons.
> > +
> > + # Leave below RTE_ARM64_MEMCPY_xxx options commented out,
> > + # unless there are strong reasons.
> > # ['RTE_ARM64_MEMCPY_SKIP_GCC_VER_CHECK', false],
> > # ['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
> > # ['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
> > @@ -28,184 +29,206 @@ flags_common_default = [
> >
> > ['RTE_SCHED_VECTOR', false],
> > ['RTE_ARM_USE_WFE', false],
> > + ['RTE_CACHE_LINE_SIZE', 128],
> > + ['RTE_ARCH_ARM64', true]
> > ]
> >
> > +# implementer specific aarch64 flags, with middle priority # (will
> > +overwrite common flags)
> > flags_generic = [
> > ['RTE_MACHINE', '"armv8a"'],
> > ['RTE_MAX_LCORE', 256],
> > ['RTE_USE_C11_MEM_MODEL', true],
> > - ['RTE_CACHE_LINE_SIZE', 128]]
> > + ['RTE_CACHE_LINE_SIZE', 128]
> > +]
> Any particular reason for this change? (and similar changes below)
>
The first bracket is split from the second bracket so I did the same for the last two brackets. It makes it more apparent which brackets are paired, it's more consistent and also in line with how flags_common_default is formatted.
> > flags_arm = [
> > ['RTE_MACHINE', '"armv8a"'],
> > ['RTE_MAX_LCORE', 16],
> > ['RTE_USE_C11_MEM_MODEL', true],
> > - ['RTE_CACHE_LINE_SIZE', 64]]
> > + ['RTE_CACHE_LINE_SIZE', 64]
> > +]
> > flags_cavium = [
> > ['RTE_CACHE_LINE_SIZE', 128],
> > ['RTE_MAX_NUMA_NODES', 2],
> > ['RTE_MAX_LCORE', 96],
> > - ['RTE_MAX_VFIO_GROUPS', 128]]
> > + ['RTE_MAX_VFIO_GROUPS', 128]
> > +]
> > flags_dpaa = [
> > ['RTE_MACHINE', '"dpaa"'],
> > ['RTE_USE_C11_MEM_MODEL', true],
> > ['RTE_CACHE_LINE_SIZE', 64],
> > ['RTE_MAX_NUMA_NODES', 1],
> > ['RTE_MAX_LCORE', 16],
> > - ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
> > + ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false] ]
> This is not needed
>
Do you mean the space? It should be a line break. I'll check the exact characters, but I see this as adding a space in my local patch. Or do you mean the config option? It's set to true by default in config/meson.build and according to [1] it should be disabled.
[1] http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-dpaa-linuxapp-gcc?h=v20.08
> > flags_emag = [
> > ['RTE_MACHINE', '"emag"'],
> > - ['RTE_CACHE_LINE_SIZE', 64],
> > ['RTE_MAX_NUMA_NODES', 1],
> > - ['RTE_MAX_LCORE', 32]]
> > + ['RTE_MAX_LCORE', 32],
> > + ['RTE_CACHE_LINE_SIZE', 64]
> > +]
> > flags_armada = [
> > ['RTE_MACHINE', '"armv8a"'],
> > - ['RTE_CACHE_LINE_SIZE', 64],
> > ['RTE_MAX_NUMA_NODES', 1],
> > - ['RTE_MAX_LCORE', 16]]
> > + ['RTE_MAX_LCORE', 16],
> > + ['RTE_CACHE_LINE_SIZE', 64]
> > +]
> Any reason for this change?
>
The default (from flags_common_default) is 128 and I found here [0] that it should be set to 64 so I added it here. Should this also be in a separate patch (apart from those 4 already mention above)?
[0] http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-armada-linuxapp-gcc?h=v20.08
> >
> > -flags_default_extra = []
> > +# part number specific aarch64 flags, with highest priority # (will
> > +overwrite both common and implementer specific flags)
> > flags_n1sdp_extra = [
> > ['RTE_MACHINE', '"n1sdp"'],
> > ['RTE_MAX_NUMA_NODES', 1],
> > ['RTE_MAX_LCORE', 4],
> > ['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
> > - ['RTE_LIBRTE_VHOST_NUMA', false]]
> > + ['RTE_LIBRTE_VHOST_NUMA', false]
> > +]
> > flags_thunderx_extra = [
> > ['RTE_MACHINE', '"thunderx"'],
> > - ['RTE_USE_C11_MEM_MODEL', false]]
> > + ['RTE_USE_C11_MEM_MODEL', false]
> > +]
> > flags_thunderx2_extra = [
> > ['RTE_MACHINE', '"thunderx2"'],
> > ['RTE_CACHE_LINE_SIZE', 64],
> > ['RTE_MAX_NUMA_NODES', 2],
> > ['RTE_MAX_LCORE', 256],
> > ['RTE_ARM_FEATURE_ATOMICS', true],
> > - ['RTE_USE_C11_MEM_MODEL', true]]
> > + ['RTE_USE_C11_MEM_MODEL', true]
> > +]
> > flags_octeontx2_extra = [
> > ['RTE_MACHINE', '"octeontx2"'],
> > ['RTE_MAX_NUMA_NODES', 1],
> > ['RTE_MAX_LCORE', 36],
> > ['RTE_ARM_FEATURE_ATOMICS', true],
> > ['RTE_EAL_IGB_UIO', false],
> > - ['RTE_USE_C11_MEM_MODEL', true]]
> > -
> > -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
> 'pn' here for 'part_number' is not consistent.
>
Ok, I can rename it to part_number_config_default. Same for the other two pn variables.
> > += {
> > + 'generic': [['-march=armv8-a+crc', '-moutline-atomics']],
> > + '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] } pn_config_cavium = {
> > + 'generic': [['-march=armv8-a+crc+crypto', '-mcpu=thunderx']],
> > + '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']] }
> >
> > ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page
> > G7-5321) -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
> > + 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@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
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Juraj Linkeš
> Sent: Tuesday, October 27, 2020 9:11 AM
> To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>;
> bruce.richardson@intel.com; Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil
> Yang <Phil.Yang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> <Dharmik.Thakkar@arm.com>; jerinjacobk@gmail.com;
> hemant.agrawal@nxp.com
> Cc: dev@dpdk.org; nd <nd@arm.com>; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v4 2/6] build: refactor Arm build
>
>
>
> > -----Original Message-----
> > From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> > Sent: Tuesday, October 27, 2020 5:56 AM
> > To: Juraj Linkeš <juraj.linkes@pantheon.tech>;
> > bruce.richardson@intel.com; Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil
> > Yang <Phil.Yang@arm.com>; vcchunga@amazon.com; Dharmik Thakkar
> > <Dharmik.Thakkar@arm.com>; jerinjacobk@gmail.com;
> > hemant.agrawal@nxp.com
> > Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> > Subject: RE: [PATCH v4 2/6] build: refactor Arm build
> >
> > <snip>
> > I think we need to split this further. Few suggestions below.
> >
> > >
> > > * Rename variables to have names that better describe what the
> > > variables store
> > This should be a separate commit.
> >
> > > * Remove unused or superfluous variables
> > Same here
> >
> > > * Change a list to dictionary where key lookup is needed
> > Same here
> >
> > > * Add informatory comments in the code
> >
> > > * Minor code restructure and reformatting
> > Same for this
> >
>
> Ok, hopefully I'll be able to separate these cleanly.
>
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > ---
> > > 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 | 247 +++++++++++++++------------
> > > 11 files changed, 153 insertions(+), 130 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'
> > Implementor and implementer mean the same. Looked at Arm specs, they
> > use 'implementer'. So, I am fine.
> >
>
> That's where I got this and some of the other variable name changes from.
>
> > > 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'
> > Same here, Arm specs refer to this as 'PartNumber'. So, this should be fine.
> > I like 'generic' for part_number here.
> >
> > > diff --git a/config/arm/arm64_bluefield_linux_gcc
> > > b/config/arm/arm64_bluefield_linux_gcc
> > > index 86797d23c..b79389d85 100644
> > > --- a/config/arm/arm64_bluefield_linux_gcc
> > > +++ b/config/arm/arm64_bluefield_linux_gcc
> > > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
> > > [properties]
> > > -implementor_id = '0x41'
> > > -implementor_pn = '0xd08'
> > > +implementer_id = '0x41'
> > > +part_number = '0xd08'
> > > diff --git a/config/arm/arm64_dpaa_linux_gcc
> > > b/config/arm/arm64_dpaa_linux_gcc index 1a4682154..573ae7e42 100644
> > > --- a/config/arm/arm64_dpaa_linux_gcc
> > > +++ b/config/arm/arm64_dpaa_linux_gcc
> > > @@ -14,4 +14,4 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
> > > [properties]
> > > -implementor_id = 'dpaa'
> > > +implementer_id = 'dpaa'
> > > diff --git a/config/arm/arm64_emag_linux_gcc
> > > b/config/arm/arm64_emag_linux_gcc index 8edcd3e97..24f3d533e 100644
> > > --- a/config/arm/arm64_emag_linux_gcc
> > > +++ b/config/arm/arm64_emag_linux_gcc
> > > @@ -13,4 +13,4 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
> > > [properties]
> > > -implementor_id = '0x50'
> > > +implementer_id = '0x50'
> > > diff --git a/config/arm/arm64_n1sdp_linux_gcc
> > > b/config/arm/arm64_n1sdp_linux_gcc
> > > index 022e06303..6fb3f02ea 100644
> > > --- a/config/arm/arm64_n1sdp_linux_gcc
> > > +++ b/config/arm/arm64_n1sdp_linux_gcc
> > > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
> > > [properties]
> > > -implementor_id = '0x41'
> > > -implementor_pn = '0xd0c'
> > > +implementer_id = '0x41'
> > > +part_number = '0xd0c'
> > > diff --git a/config/arm/arm64_octeontx2_linux_gcc
> > > b/config/arm/arm64_octeontx2_linux_gcc
> > > index 365bd7cbd..ac1042806 100644
> > > --- a/config/arm/arm64_octeontx2_linux_gcc
> > > +++ b/config/arm/arm64_octeontx2_linux_gcc
> > > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
> > > [properties]
> > > -implementor_id = '0x43'
> > > -implementor_pn = '0xb2'
> > > +implementer_id = '0x43'
> > > +part_number = '0xb2'
> > > diff --git a/config/arm/arm64_stingray_linux_gcc
> > > b/config/arm/arm64_stingray_linux_gcc
> > > index 86797d23c..b79389d85 100644
> > > --- a/config/arm/arm64_stingray_linux_gcc
> > > +++ b/config/arm/arm64_stingray_linux_gcc
> > > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
> > > [properties]
> > > -implementor_id = '0x41'
> > > -implementor_pn = '0xd08'
> > > +implementer_id = '0x41'
> > > +part_number = '0xd08'
> > > diff --git a/config/arm/arm64_thunderx2_linux_gcc
> > > b/config/arm/arm64_thunderx2_linux_gcc
> > > index 2b41acc61..dd257745e 100644
> > > --- a/config/arm/arm64_thunderx2_linux_gcc
> > > +++ b/config/arm/arm64_thunderx2_linux_gcc
> > > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
> > > [properties]
> > > -implementor_id = '0x43'
> > > -implementor_pn = '0xaf'
> > > +implementer_id = '0x43'
> > > +part_number = '0xaf'
> > > diff --git a/config/arm/arm64_thunderx_linux_gcc
> > > b/config/arm/arm64_thunderx_linux_gcc
> > > index 6572ab615..670764437 100644
> > > --- a/config/arm/arm64_thunderx_linux_gcc
> > > +++ b/config/arm/arm64_thunderx_linux_gcc
> > > @@ -13,4 +13,4 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
> > > [properties]
> > > -implementor_id = '0x43'
> > > +implementer_id = '0x43'
> > > 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']
> > >
> > > +# common flags to all aarch64 builds, with lowest priority
> > > flags_common_default = [
> > > # Accelarate rte_memcpy. Be sure to run unit test
> > > (memcpy_perf_autotest)
> > > # to determine the best threshold in code. Refer to notes in
> > > source file @@ -16,8 +16,9 @@ flags_common_default = [
> > > ['RTE_ARCH_ARM64_MEMCPY', false],
> > > # ['RTE_ARM64_MEMCPY_ALIGNED_THRESHOLD', 2048],
> > > # ['RTE_ARM64_MEMCPY_UNALIGNED_THRESHOLD', 512],
> > > - # Leave below RTE_ARM64_MEMCPY_xxx options commented out,
> > > unless there're
> > > - # strong reasons.
> > > +
> > > + # Leave below RTE_ARM64_MEMCPY_xxx options commented out,
> > > + # unless there are strong reasons.
> > > # ['RTE_ARM64_MEMCPY_SKIP_GCC_VER_CHECK', false],
> > > # ['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
> > > # ['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
> > > @@ -28,184 +29,206 @@ flags_common_default = [
> > >
> > > ['RTE_SCHED_VECTOR', false],
> > > ['RTE_ARM_USE_WFE', false],
> > > + ['RTE_CACHE_LINE_SIZE', 128],
> > > + ['RTE_ARCH_ARM64', true]
> > > ]
> > >
> > > +# implementer specific aarch64 flags, with middle priority # (will
> > > +overwrite common flags)
> > > flags_generic = [
> > > ['RTE_MACHINE', '"armv8a"'],
> > > ['RTE_MAX_LCORE', 256],
> > > ['RTE_USE_C11_MEM_MODEL', true],
> > > - ['RTE_CACHE_LINE_SIZE', 128]]
> > > + ['RTE_CACHE_LINE_SIZE', 128]
> > > +]
> > Any particular reason for this change? (and similar changes below)
> >
>
> The first bracket is split from the second bracket so I did the same for the last
> two brackets. It makes it more apparent which brackets are paired, it's more
> consistent and also in line with how flags_common_default is formatted.
>
> > > flags_arm = [
> > > ['RTE_MACHINE', '"armv8a"'],
> > > ['RTE_MAX_LCORE', 16],
> > > ['RTE_USE_C11_MEM_MODEL', true],
> > > - ['RTE_CACHE_LINE_SIZE', 64]]
> > > + ['RTE_CACHE_LINE_SIZE', 64]
> > > +]
> > > flags_cavium = [
> > > ['RTE_CACHE_LINE_SIZE', 128],
> > > ['RTE_MAX_NUMA_NODES', 2],
> > > ['RTE_MAX_LCORE', 96],
> > > - ['RTE_MAX_VFIO_GROUPS', 128]]
> > > + ['RTE_MAX_VFIO_GROUPS', 128]
> > > +]
> > > flags_dpaa = [
> > > ['RTE_MACHINE', '"dpaa"'],
> > > ['RTE_USE_C11_MEM_MODEL', true],
> > > ['RTE_CACHE_LINE_SIZE', 64],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > ['RTE_MAX_LCORE', 16],
> > > - ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
> > > + ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false] ]
> > This is not needed
> >
>
> Do you mean the space? It should be a line break. I'll check the exact characters,
> but I see this as adding a space in my local patch. Or do you mean the config
> option? It's set to true by default in config/meson.build and according to [1] it
> should be disabled.
>
> [1] http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-dpaa-linuxapp-
> gcc?h=v20.08
>
> > > flags_emag = [
> > > ['RTE_MACHINE', '"emag"'],
> > > - ['RTE_CACHE_LINE_SIZE', 64],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > - ['RTE_MAX_LCORE', 32]]
> > > + ['RTE_MAX_LCORE', 32],
> > > + ['RTE_CACHE_LINE_SIZE', 64]
> > > +]
> > > flags_armada = [
> > > ['RTE_MACHINE', '"armv8a"'],
> > > - ['RTE_CACHE_LINE_SIZE', 64],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > - ['RTE_MAX_LCORE', 16]]
> > > + ['RTE_MAX_LCORE', 16],
> > > + ['RTE_CACHE_LINE_SIZE', 64]
> > > +]
> > Any reason for this change?
> >
>
> The default (from flags_common_default) is 128 and I found here [0] that it
> should be set to 64 so I added it here. Should this also be in a separate patch
> (apart from those 4 already mention above)?
>
> [0] http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-armada-linuxapp-
> gcc?h=v20.08
>
Sorry, I overlooked that I just moved the config around. I don't remember why. I could see that I wanted to make the order consistent across the different flag group, but that is not the case even with these changes. I'll make it consistent when I split the patch.
> > >
> > > -flags_default_extra = []
> > > +# part number specific aarch64 flags, with highest priority # (will
> > > +overwrite both common and implementer specific flags)
> > > flags_n1sdp_extra = [
> > > ['RTE_MACHINE', '"n1sdp"'],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > ['RTE_MAX_LCORE', 4],
> > > ['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
> > > - ['RTE_LIBRTE_VHOST_NUMA', false]]
> > > + ['RTE_LIBRTE_VHOST_NUMA', false]
> > > +]
> > > flags_thunderx_extra = [
> > > ['RTE_MACHINE', '"thunderx"'],
> > > - ['RTE_USE_C11_MEM_MODEL', false]]
> > > + ['RTE_USE_C11_MEM_MODEL', false]
> > > +]
> > > flags_thunderx2_extra = [
> > > ['RTE_MACHINE', '"thunderx2"'],
> > > ['RTE_CACHE_LINE_SIZE', 64],
> > > ['RTE_MAX_NUMA_NODES', 2],
> > > ['RTE_MAX_LCORE', 256],
> > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > - ['RTE_USE_C11_MEM_MODEL', true]]
> > > + ['RTE_USE_C11_MEM_MODEL', true]
> > > +]
> > > flags_octeontx2_extra = [
> > > ['RTE_MACHINE', '"octeontx2"'],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > ['RTE_MAX_LCORE', 36],
> > > ['RTE_ARM_FEATURE_ATOMICS', true],
> > > ['RTE_EAL_IGB_UIO', false],
> > > - ['RTE_USE_C11_MEM_MODEL', true]]
> > > -
> > > -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
> > 'pn' here for 'part_number' is not consistent.
> >
>
> Ok, I can rename it to part_number_config_default. Same for the other two pn
> variables.
>
> > > += {
> > > + 'generic': [['-march=armv8-a+crc', '-moutline-atomics']],
> > > + '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] } pn_config_cavium = {
> > > + 'generic': [['-march=armv8-a+crc+crypto', '-mcpu=thunderx']],
> > > + '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']] }
> > >
> > > ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page
> > > G7-5321) -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
> > > + 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@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
<snip>
> > I think we need to split this further. Few suggestions below.
> >
> > >
> > > * Rename variables to have names that better describe what the
> > > variables store
> > This should be a separate commit.
> >
> > > * Remove unused or superfluous variables
> > Same here
> >
> > > * Change a list to dictionary where key lookup is needed
> > Same here
> >
> > > * Add informatory comments in the code
> >
> > > * Minor code restructure and reformatting
> > Same for this
> >
>
> Ok, hopefully I'll be able to separate these cleanly.
>
> > >
> > > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > > ---
> > > 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 | 247 +++++++++++++++------------
> > > 11 files changed, 153 insertions(+), 130 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'
> > Implementor and implementer mean the same. Looked at Arm specs, they
> > use 'implementer'. So, I am fine.
> >
>
> That's where I got this and some of the other variable name changes from.
>
> > > 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'
> > Same here, Arm specs refer to this as 'PartNumber'. So, this should be fine.
> > I like 'generic' for part_number here.
> >
> > > diff --git a/config/arm/arm64_bluefield_linux_gcc
> > > b/config/arm/arm64_bluefield_linux_gcc
> > > index 86797d23c..b79389d85 100644
> > > --- a/config/arm/arm64_bluefield_linux_gcc
> > > +++ b/config/arm/arm64_bluefield_linux_gcc
> > > @@ -13,5 +13,5 @@ cpu = 'armv8-a'
> > > endian = 'little'
> > >
[...]
> > > 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)
> > > -
[...]
> > >
> > > +# implementer specific aarch64 flags, with middle priority # (will
> > > +overwrite common flags)
> > > flags_generic = [
> > > ['RTE_MACHINE', '"armv8a"'],
> > > ['RTE_MAX_LCORE', 256],
> > > ['RTE_USE_C11_MEM_MODEL', true],
> > > - ['RTE_CACHE_LINE_SIZE', 128]]
> > > + ['RTE_CACHE_LINE_SIZE', 128]
> > > +]
> > Any particular reason for this change? (and similar changes below)
> >
>
> The first bracket is split from the second bracket so I did the same for the last
> two brackets. It makes it more apparent which brackets are paired, it's more
> consistent and also in line with how flags_common_default is formatted.
Ok
>
[...]
> > > flags_dpaa = [
> > > ['RTE_MACHINE', '"dpaa"'],
> > > ['RTE_USE_C11_MEM_MODEL', true],
> > > ['RTE_CACHE_LINE_SIZE', 64],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > ['RTE_MAX_LCORE', 16],
> > > - ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
> > > + ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false] ]
> > This is not needed
> >
>
> Do you mean the space? It should be a line break. I'll check the exact
I meant the space. I guess it needs to be on the next line.
> characters, but I see this as adding a space in my local patch. Or do you mean
> the config option? It's set to true by default in config/meson.build and
> according to [1] it should be disabled.
>
> [1] http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-dpaa-linuxapp-
> gcc?h=v20.08
>
> > > flags_emag = [
> > > ['RTE_MACHINE', '"emag"'],
> > > - ['RTE_CACHE_LINE_SIZE', 64],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > - ['RTE_MAX_LCORE', 32]]
> > > + ['RTE_MAX_LCORE', 32],
> > > + ['RTE_CACHE_LINE_SIZE', 64]
> > > +]
> > > flags_armada = [
> > > ['RTE_MACHINE', '"armv8a"'],
> > > - ['RTE_CACHE_LINE_SIZE', 64],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > - ['RTE_MAX_LCORE', 16]]
> > > + ['RTE_MAX_LCORE', 16],
> > > + ['RTE_CACHE_LINE_SIZE', 64]
> > > +]
> > Any reason for this change?
> >
>
> The default (from flags_common_default) is 128 and I found here [0] that it
> should be set to 64 so I added it here. Should this also be in a separate patch
> (apart from those 4 already mention above)?
The RTE_CACHE_LINE_SIZE is already set to 64 for 'flags_armada'. It looks like you have moved the line down.
>
> [0] http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-armada-
> linuxapp-gcc?h=v20.08
>
> > >
> > > -flags_default_extra = []
> > > +# part number specific aarch64 flags, with highest priority # (will
> > > +overwrite both common and implementer specific flags)
> > > flags_n1sdp_extra = [
> > > ['RTE_MACHINE', '"n1sdp"'],
> > > ['RTE_MAX_NUMA_NODES', 1],
> > > ['RTE_MAX_LCORE', 4],
> > > ['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
> > > - ['RTE_LIBRTE_VHOST_NUMA', false]]
> > > + ['RTE_LIBRTE_VHOST_NUMA', false]
> > > +]
<snip>
> > > -
> > > -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
> > 'pn' here for 'part_number' is not consistent.
> >
>
> Ok, I can rename it to part_number_config_default. Same for the other two
> pn variables.
Ok
>
<snip>
> > > --
> > > 2.20.1
<snip>
> >
> > > > flags_emag = [
> > > > ['RTE_MACHINE', '"emag"'],
> > > > - ['RTE_CACHE_LINE_SIZE', 64],
> > > > ['RTE_MAX_NUMA_NODES', 1],
> > > > - ['RTE_MAX_LCORE', 32]]
> > > > + ['RTE_MAX_LCORE', 32],
> > > > + ['RTE_CACHE_LINE_SIZE', 64]
> > > > +]
> > > > flags_armada = [
> > > > ['RTE_MACHINE', '"armv8a"'],
> > > > - ['RTE_CACHE_LINE_SIZE', 64],
> > > > ['RTE_MAX_NUMA_NODES', 1],
> > > > - ['RTE_MAX_LCORE', 16]]
> > > > + ['RTE_MAX_LCORE', 16],
> > > > + ['RTE_CACHE_LINE_SIZE', 64]
> > > > +]
> > > Any reason for this change?
> > >
> >
> > The default (from flags_common_default) is 128 and I found here [0]
> > that it should be set to 64 so I added it here. Should this also be in
> > a separate patch (apart from those 4 already mention above)?
> >
> > [0]
> > http://git.dpdk.org/dpdk/tree/config/defconfig_arm64-armada-linuxapp-
> > gcc?h=v20.08
> >
>
> Sorry, I overlooked that I just moved the config around. I don't remember
> why. I could see that I wanted to make the order consistent across the
> different flag group, but that is not the case even with these changes. I'll
> make it consistent when I split the patch.
Ok
<snip>
<snip>
>
> * Rename variables to have names that better describe what the variables
> store
> * Remove unused or superfluous variables
> * Change a list to dictionary where key lookup is needed
> * Add informatory comments in the code
> * Minor code restructure and reformatting
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> ---
> 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 | 247 +++++++++++++++------------
> 11 files changed, 153 insertions(+), 130 deletions(-)
>
<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'.
> += {
> + '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.
> + '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.
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
} 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.
>
> ## 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
> -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?
> + 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@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
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Wednesday, October 28, 2020 5:59 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; bruce.richardson@intel.com;
> Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com
> Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v4 2/6] build: refactor Arm build
>
> <snip>
>
> >
> > * Rename variables to have names that better describe what the
> > variables store
> > * Remove unused or superfluous variables
> > * Change a list to dictionary where key lookup is needed
> > * Add informatory comments in the code
> > * Minor code restructure and reformatting
> >
> > Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
> > ---
> > 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 | 247 +++++++++++++++------------
> > 11 files changed, 153 insertions(+), 130 deletions(-)
> >
>
> <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.
> > += {
> > + '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.
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.
> 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 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
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).
> >
> > ## 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.
> > -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@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
> > <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@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
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Thursday, October 29, 2020 9:54 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; bruce.richardson@intel.com;
> Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com
> Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v4 2/6] build: refactor Arm build
>
> > > <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.
These are currently named flags_<implementer> such as flags_arm and flags_cavium. We could rename them to implementer_flags_<implementer>.
>
> 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.
>
Yes, we'll always have both. But there are still a couple of corner cases with native builds, which should always work:
1. We don't support the implementer (i.e. there's no variable with name implementer_<implementer_id>). In this case, what do default to? The generic build, which is the current behavior?
2. We support the implementer, but we don't support the part number. In this case, we can just use native machine args (-march=native) and use only implementer specific flags.
For generic, soc-specifc and cross builds it makes sense to error with uknown implementer/part number, since we wouldn't know what to build otherwise.
> >
> > 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.
>
It's not a build, just how the configation is organized and processed, as in we apply the common default flags, then implementer flags, then part number flags and then soc flags, overwriting flags in all of the steps.
> > 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")
>
Ok, I'll make this change.
> >
> > > > -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@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
<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.
>
> These are currently named flags_<implementer> such as flags_arm and
> flags_cavium. We could rename them to
> implementer_flags_<implementer>.
Ok
>
> >
> > 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.
> >
>
> Yes, we'll always have both. But there are still a couple of corner cases with
> native builds, which should always work:
I do not think these are corner cases. These are basically cases where the support is not added yet in the build system. We do not need to handle these specifically.
I also do not think the notion that the 'native' build should always work is correct (at least on Arm platforms). The reason I say that is because 'native', IMO, not only defines the ISA, but other parameters as well in DPDK.
> 1. We don't support the implementer (i.e. there's no variable with name
> implementer_<implementer_id>). In this case, what do default to? The
> generic build, which is the current behavior?
This should result in an error. The implementer should add support in meson.build
> 2. We support the implementer, but we don't support the part number. In
> this case, we can just use native machine args (-march=native) and use only
> implementer specific flags.
Same thing, should result in error. The implementer should add support in meson.build.
>
> For generic, soc-specifc and cross builds it makes sense to error with uknown
> implementer/part number, since we wouldn't know what to build otherwise.
>
> > >
> > > 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.
> >
>
> It's not a build, just how the configation is organized and processed, as in we
> apply the common default flags, then implementer flags, then part number
> flags and then soc flags, overwriting flags in all of the steps.
Ok
>
> > > 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")
> >
>
> Ok, I'll make this change.
>
> > >
> > > > > -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@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
> -----Original Message-----
> From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Sent: Monday, November 2, 2020 8:32 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>; bruce.richardson@intel.com;
> Ruifeng Wang <Ruifeng.Wang@arm.com>; Phil Yang <Phil.Yang@arm.com>;
> vcchunga@amazon.com; Dharmik Thakkar <Dharmik.Thakkar@arm.com>;
> jerinjacobk@gmail.com; hemant.agrawal@nxp.com
> Cc: dev@dpdk.org; nd <nd@arm.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; nd <nd@arm.com>
> Subject: RE: [PATCH v4 2/6] build: refactor Arm build
>
> <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.
> >
> > These are currently named flags_<implementer> such as flags_arm and
> > flags_cavium. We could rename them to implementer_flags_<implementer>.
> Ok
>
> >
> > >
> > > 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.
> > >
> >
> > Yes, we'll always have both. But there are still a couple of corner cases with
> > native builds, which should always work:
> I do not think these are corner cases. These are basically cases where the
> support is not added yet in the build system. We do not need to handle these
> specifically.
> I also do not think the notion that the 'native' build should always work is correct
> (at least on Arm platforms). The reason I say that is because 'native', IMO, not
> only defines the ISA, but other parameters as well in DPDK.
>
So our stance would basically be the native build doesn't work for unsupported implementers/part numbers and then direct the user to generic build. That seems reasonable.
> > 1. We don't support the implementer (i.e. there's no variable with name
> > implementer_<implementer_id>). In this case, what do default to? The
> > generic build, which is the current behavior?
> This should result in an error. The implementer should add support in
> meson.build
>
Ok, then we need to change the current configuration. The implementers were added in bulk with the same configuration, so that can't really be considered supporting them in this way. I think we should keep implementers which have specific implementer flags or machine configuration defined for them and remove the rest. That would leave us with:
implementer_generic = ['Generic armv8', flags_implementer_generic, part_number_config_arm]
implementer_0x41 = ['Arm', flags_implementer_arm, part_number_config_arm]
implementer_0x43 = ['Cavium', flags_implementer_cavium, part_number_config_cavium]
implementer_0x50 = ['Ampere Computing', flags_implementer_emag, part_number_config_emag]
implementer_0x56 = ['Marvell ARMADA', flags_implementer_armada, part_number_config_arm]
implementer_dpaa = ['NXP DPAA', flags_implementer_dpaa, part_number_config_arm]
And we would remove these (everything with the same flags/part number config as the generic build):
implementer_0x42 = ['Broadcom', flags_implementer_generic, part_number_config_arm]
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_0x51 = ['Qualcomm', flags_implementer_generic, part_number_config_arm]
implementer_0x53 = ['Samsung', flags_implementer_generic, part_number_config_arm]
implementer_0x69 = ['Intel', flags_implementer_generic, part_number_config_arm]
> > 2. We support the implementer, but we don't support the part number. In
> > this case, we can just use native machine args (-march=native) and use only
> > implementer specific flags.
> Same thing, should result in error. The implementer should add support in
> meson.build.
>
There are two different scenarios, native and soc-specific/cross builds.
For native builds, we could just use the implementer flags with native machine args. This could be still viewed as implementer intending to support it for native build - by omitting it, they'd basically be saying there aren't any extra flags for the part number. There are hidden assumptions here and it results in data that's not clear, so It's probably best to enforce even for native builds, as that we remove any ambiguity.
For soc-specific/cross builds, we don't know which machine args to use, so that's an error.
I'll make the changes so that an unknown implementer/part number results in an error for all builds.
> >
> > For generic, soc-specifc and cross builds it makes sense to error with uknown
> > implementer/part number, since we wouldn't know what to build otherwise.
> >
> > > >
> > > > 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.
> > >
> >
> > It's not a build, just how the configation is organized and processed, as in we
> > apply the common default flags, then implementer flags, then part number
> > flags and then soc flags, overwriting flags in all of the steps.
> Ok
>
> >
> > > > 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")
> > >
> >
> > Ok, I'll make this change.
> >
> > > >
> > > > > > -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@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
@@ -14,4 +14,4 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = '0x56'
+implementer_id = '0x56'
@@ -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'
@@ -13,5 +13,5 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = '0x41'
-implementor_pn = '0xd08'
+implementer_id = '0x41'
+part_number = '0xd08'
@@ -14,4 +14,4 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = 'dpaa'
+implementer_id = 'dpaa'
@@ -13,4 +13,4 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = '0x50'
+implementer_id = '0x50'
@@ -13,5 +13,5 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = '0x41'
-implementor_pn = '0xd0c'
+implementer_id = '0x41'
+part_number = '0xd0c'
@@ -13,5 +13,5 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = '0x43'
-implementor_pn = '0xb2'
+implementer_id = '0x43'
+part_number = '0xb2'
@@ -13,5 +13,5 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = '0x41'
-implementor_pn = '0xd08'
+implementer_id = '0x41'
+part_number = '0xd08'
@@ -13,5 +13,5 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = '0x43'
-implementor_pn = '0xaf'
+implementer_id = '0x43'
+part_number = '0xaf'
@@ -13,4 +13,4 @@ cpu = 'armv8-a'
endian = 'little'
[properties]
-implementor_id = '0x43'
+implementer_id = '0x43'
@@ -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']
+# common flags to all aarch64 builds, with lowest priority
flags_common_default = [
# Accelarate rte_memcpy. Be sure to run unit test (memcpy_perf_autotest)
# to determine the best threshold in code. Refer to notes in source file
@@ -16,8 +16,9 @@ flags_common_default = [
['RTE_ARCH_ARM64_MEMCPY', false],
# ['RTE_ARM64_MEMCPY_ALIGNED_THRESHOLD', 2048],
# ['RTE_ARM64_MEMCPY_UNALIGNED_THRESHOLD', 512],
- # Leave below RTE_ARM64_MEMCPY_xxx options commented out, unless there're
- # strong reasons.
+
+ # Leave below RTE_ARM64_MEMCPY_xxx options commented out,
+ # unless there are strong reasons.
# ['RTE_ARM64_MEMCPY_SKIP_GCC_VER_CHECK', false],
# ['RTE_ARM64_MEMCPY_ALIGN_MASK', 0xF],
# ['RTE_ARM64_MEMCPY_STRICT_ALIGN', false],
@@ -28,184 +29,206 @@ flags_common_default = [
['RTE_SCHED_VECTOR', false],
['RTE_ARM_USE_WFE', false],
+ ['RTE_CACHE_LINE_SIZE', 128],
+ ['RTE_ARCH_ARM64', true]
]
+# implementer specific aarch64 flags, with middle priority
+# (will overwrite common flags)
flags_generic = [
['RTE_MACHINE', '"armv8a"'],
['RTE_MAX_LCORE', 256],
['RTE_USE_C11_MEM_MODEL', true],
- ['RTE_CACHE_LINE_SIZE', 128]]
+ ['RTE_CACHE_LINE_SIZE', 128]
+]
flags_arm = [
['RTE_MACHINE', '"armv8a"'],
['RTE_MAX_LCORE', 16],
['RTE_USE_C11_MEM_MODEL', true],
- ['RTE_CACHE_LINE_SIZE', 64]]
+ ['RTE_CACHE_LINE_SIZE', 64]
+]
flags_cavium = [
['RTE_CACHE_LINE_SIZE', 128],
['RTE_MAX_NUMA_NODES', 2],
['RTE_MAX_LCORE', 96],
- ['RTE_MAX_VFIO_GROUPS', 128]]
+ ['RTE_MAX_VFIO_GROUPS', 128]
+]
flags_dpaa = [
['RTE_MACHINE', '"dpaa"'],
['RTE_USE_C11_MEM_MODEL', true],
['RTE_CACHE_LINE_SIZE', 64],
['RTE_MAX_NUMA_NODES', 1],
['RTE_MAX_LCORE', 16],
- ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
+ ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]
+]
flags_emag = [
['RTE_MACHINE', '"emag"'],
- ['RTE_CACHE_LINE_SIZE', 64],
['RTE_MAX_NUMA_NODES', 1],
- ['RTE_MAX_LCORE', 32]]
+ ['RTE_MAX_LCORE', 32],
+ ['RTE_CACHE_LINE_SIZE', 64]
+]
flags_armada = [
['RTE_MACHINE', '"armv8a"'],
- ['RTE_CACHE_LINE_SIZE', 64],
['RTE_MAX_NUMA_NODES', 1],
- ['RTE_MAX_LCORE', 16]]
+ ['RTE_MAX_LCORE', 16],
+ ['RTE_CACHE_LINE_SIZE', 64]
+]
-flags_default_extra = []
+# part number specific aarch64 flags, with highest priority
+# (will overwrite both common and implementer specific flags)
flags_n1sdp_extra = [
['RTE_MACHINE', '"n1sdp"'],
['RTE_MAX_NUMA_NODES', 1],
['RTE_MAX_LCORE', 4],
['RTE_EAL_NUMA_AWARE_HUGEPAGES', false],
- ['RTE_LIBRTE_VHOST_NUMA', false]]
+ ['RTE_LIBRTE_VHOST_NUMA', false]
+]
flags_thunderx_extra = [
['RTE_MACHINE', '"thunderx"'],
- ['RTE_USE_C11_MEM_MODEL', false]]
+ ['RTE_USE_C11_MEM_MODEL', false]
+]
flags_thunderx2_extra = [
['RTE_MACHINE', '"thunderx2"'],
['RTE_CACHE_LINE_SIZE', 64],
['RTE_MAX_NUMA_NODES', 2],
['RTE_MAX_LCORE', 256],
['RTE_ARM_FEATURE_ATOMICS', true],
- ['RTE_USE_C11_MEM_MODEL', true]]
+ ['RTE_USE_C11_MEM_MODEL', true]
+]
flags_octeontx2_extra = [
['RTE_MACHINE', '"octeontx2"'],
['RTE_MAX_NUMA_NODES', 1],
['RTE_MAX_LCORE', 36],
['RTE_ARM_FEATURE_ATOMICS', true],
['RTE_EAL_IGB_UIO', false],
- ['RTE_USE_C11_MEM_MODEL', true]]
-
-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 = {
+ 'generic': [['-march=armv8-a+crc', '-moutline-atomics']],
+ '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]
+}
+pn_config_cavium = {
+ 'generic': [['-march=armv8-a+crc+crypto', '-mcpu=thunderx']],
+ '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']]
+}
## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321)
-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
+ 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@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) != '')