<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.lin...@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
