Hi Maxime,
Thanks for reviewing, some comments below.
>-----Original Message-----
>From: Maxime Coquelin <maxime.coque...@redhat.com>
>Sent: Tuesday 6 October 2020 12:50
>To: Power, Ciara <ciara.po...@intel.com>; dev@dpdk.org
>Cc: Ray Kinsella <m...@ashroe.eu>; Neil Horman <nhor...@tuxdriver.com>
>Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>
>
>
>On 9/30/20 3:03 PM, Ciara Power wrote:
>> This patch adds a max SIMD bitwidth EAL configuration. The API allows
>> for an app to set this value. It can also be set using EAL argument
>> --force-max-simd-bitwidth, which will lock the value and override any
>> modifications made by the app.
>>
>> Signed-off-by: Ciara Power <ciara.po...@intel.com>
>>
>> ---
>> v3:
>> - Added enum value to essentially disable using max SIMD to choose
>> paths, intended for use by ARM SVE.
>> - Fixed parsing bitwidth argument to return an error for values
>> greater than uint16_t.
>> v2: Added to Doxygen comment for API.
>> ---
<snip>
>> @@ -1903,6 +1939,33 @@ eal_check_common_options(struct
>internal_config *internal_cfg)
>> return 0;
>> }
>>
>> +uint16_t
>
>shouldn't it return an enum rte_max_simd_t?
I kept the return value as uint16_t to allow for arbitrary values,
and will allow it be more flexible for future additions as new enums won't need
to be added.
The enums are really used for readability when checking the bitwidth limit in
drivers/libs, so
essentially #defines in enum form.
>
>> +rte_get_max_simd_bitwidth(void)
>> +{
>> + const struct internal_config *internal_conf =
>> + eal_get_internal_configuration();
>> + return internal_conf->max_simd_bitwidth.bitwidth;
>
>What is the default value if not set?
>
The default value for max_simd_bitwidth is set depending on the architecture,
256 for x86/ppc,
and UINT16_MAX for ARM. So for example the default on x86 allows for AVX2 and
under.
The defaults can be seen in patch 2: https://patchwork.dpdk.org/patch/79339/
<snip>
Thanks,
Ciara
