Re: [dpdk-dev] [PATCH v8 2/4] meson: add infra to support machine specific flags

[Yongseok Koh <yskoh@mellanox.com>]

> On Apr 11, 2019, at 4:37 PM, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> 10/04/2019 18:13, jerinjacobk@gmail.com:
>> From: Pavan Nikhilesh <pbhagavatula@marvell.com>
>> 
>> Currently, RTE_* flags are set based on the implementer ID but there might
>> be some micro arch specific differences from the same vendor
>> eg. CACHE_LINESIZE. Add support to set micro arch specific flags.
> 
> I don't like how flags are set in config/arm/meson.build.
> It is a real mess to find which flag applies to which machine.
> Adding the flags_*_extra in the machine_args_* is adding more mess.
> 
> [...]
>> 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
>> @@ -52,12 +33,10 @@ flags_generic = [
>> 	['RTE_USE_C11_MEM_MODEL', true],
>> 	['RTE_CACHE_LINE_SIZE', 128]]
>> flags_cavium = [
>> -	['RTE_MACHINE', '"thunderx"'],
>> 	['RTE_CACHE_LINE_SIZE', 128],
>> 	['RTE_MAX_NUMA_NODES', 2],
>> 	['RTE_MAX_LCORE', 96],
>> -	['RTE_MAX_VFIO_GROUPS', 128],
>> -	['RTE_USE_C11_MEM_MODEL', false]]
>> +	['RTE_MAX_VFIO_GROUPS', 128]]
>> flags_dpaa = [
>> 	['RTE_MACHINE', '"dpaa"'],
>> 	['RTE_USE_C11_MEM_MODEL', true],
>> @@ -71,6 +50,27 @@ flags_dpaa2 = [
>> 	['RTE_MAX_NUMA_NODES', 1],
>> 	['RTE_MAX_LCORE', 16],
>> 	['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', false]]
>> +flags_default_extra = []
>> +flags_thunderx_extra = [
>> +	['RTE_MACHINE', '"thunderx"'],
>> +	['RTE_USE_C11_MEM_MODEL', false]]
>> +
>> +machine_args_generic = [
>> +	['default', ['-march=armv8-a+crc+crypto']],
>> +	['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']]]
>> +
>> +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]]
> 
> I think we should have a simpler model.
> We need only to know the machine name and get all the related machine config.
> In native compilation, machine name is guessed from implementor id and pn

It can be guessed unless machine name is specified as an option.
We can add machine_name option?
One observation. We do have machine option but it isn't used much for arm.
And in config/arm/meson.build, march_opt isn't used either.

> (from config/arm/armv8_machine.py). We can directly output the machine name
> from this script and leave the naming logic in this script.
> In the cross-compilation config files (config/arm/*),
> we can just specify the machine name.
> Then every machine config (machine_args and dpdk_conf) would be specified
> in some arrays based on the machine name.
> Of course, we can keep some common default values.
> 
> Thoughts?

Sounds like we can cleanly reorganize it with the suggestion.

Thanks,
Yongseok