[dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth

[Power, Ciara]

Hi Bruce,

>-----Original Message-----
>From: Bruce Richardson <bruce.richardson at intel.com>
>Sent: Wednesday 7 October 2020 11:52
>To: Power, Ciara <ciara.power at intel.com>
>Cc: Olivier Matz <olivier.matz at 6wind.com>; dev at dpdk.org; Ray Kinsella
><mdr at ashroe.eu>; Neil Horman <nhorman at tuxdriver.com>
>Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>
>On Wed, Oct 07, 2020 at 11:47:34AM +0100, Power, Ciara wrote:
>> Hi Olivier,
>>
>> Thanks for reviewing, some comments below.
>>
>>
>> >-----Original Message-----
>> >From: Olivier Matz <olivier.matz at 6wind.com>
>> >Sent: Tuesday 6 October 2020 10:32
>> >To: Power, Ciara <ciara.power at intel.com>
>> >Cc: dev at dpdk.org; Ray Kinsella <mdr at ashroe.eu>; Neil Horman
>> ><nhorman at tuxdriver.com>
>> >Subject: Re: [dpdk-dev] [PATCH v3 01/18] eal: add max SIMD bitwidth
>> >
>> >Hi Ciara,
>> >
>> >Please find some comments below.
>> >
>> >On Wed, Sep 30, 2020 at 02:03:57PM +0100, 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.power at 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>
>>
>> >>
>> >> +uint16_t
>> >> +rte_get_max_simd_bitwidth(void)
>> >> +{
>> >> +const struct internal_config *internal_conf =
>> >> +eal_get_internal_configuration();
>> >> +return internal_conf->max_simd_bitwidth.bitwidth;
>> >> +}
>> >
>> >Should the return value be enum rte_max_simd_t?
>> >If not, do we really need the enum definition?
>> >
>>
>> I kept the return value and param value below 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.
>> For the set function below, this is used when a user passes the EAL
>> command line flag, which passes an integer value rather than an enum one.
>> The enums are useful when checking the max_simd_bitwidth in
>> drivers/libs, for example using "RTE_MAX_256_SIMD" instead of "256" in
>the condition checks.
>>
>So basically these enum values are #defines for readability, just in enum
>form, right?

Yes, that's exactly right.

Thanks,
Ciara