[ovs-dev] [PATCH] acinclude: Don't set AVX512-related configuration via CFLAGS.

[Ilya Maximets]

On 8/4/21 1:53 PM, Ferriter, Cian wrote:
> 
>> -----Original Message-----
>> From: Ilya Maximets <i.maximets at ovn.org>
>> Sent: Tuesday 3 August 2021 18:37
>> To: ovs-dev at openvswitch.org; Stokes, Ian <ian.stokes at intel.com>
>> Cc: Flavio Leitner <fbl at sysclose.org>; Amber, Kumar <kumar.amber at intel.com>; Ferriter, Cian
>> <cian.ferriter at intel.com>; Van Haaren, Harry <harry.van.haaren at intel.com>; Ilya Maximets
>> <i.maximets at ovn.org>
>> Subject: [PATCH] acinclude: Don't set AVX512-related configuration via CFLAGS.
>>
>> The correct way to pass configuration options is to define them
>> inside the config.h.  Additionally, few long lines wrapped and
>> fixed the unnecessary double check for -mavx512f.
>>
>> Fixes: abb807e27dd4 ("dpif-netdev: Add command to switch dpif implementation.")
>> Fixes: 5324b54e606a ("dpif-netdev: Add configure to enable autovalidator at build time.")
>> Fixes: e90e115a01af ("dpif-netdev: implement subtable lookup validation.")
>> Fixes: 352b6c7116cd ("dpif-lookup: add avx512 gather implementation.")
>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>> ---
>>
>> Only tested that it builds.  Didn't run any runtime checks
>> since I have no HW for that.
>>
>>  acinclude.m4      | 36 ++++++++++++++++++++++++++++++------
>>  configure.ac      |  4 +---
>>  m4/openvswitch.m4 |  5 ++++-
>>  3 files changed, 35 insertions(+), 10 deletions(-)
>>
> 
> <snip away changes>
> 
> Hi Ilya,
> 
> Thanks for the changes, much appreciated.
> 
> I've tested out this patch on a system with AVX512 and everything works as expected.
> 
> I can verify that:
> * I see an additional output line in the ./configure step like below:
>   checking whether gcc accepts -mavx512f... yes
> * When I add " --enable-autovalidator --enable-dpif-default-avx512 --enable-mfex-default-autovalidator" options to configure, these options still work as expected, enabling the respective features at compile time.
> * I can see the configuration options in config.h after the patch is applied:
> ~# diff config.h_before_patch config.h_after_patch
> 26a27,30
>> /* Autovalidator for the userspace datapath classifier is a default
>>    implementation. */
>> #define DPCLS_AUTOVALIDATOR_DEFAULT 1
>>
> 29a34,37
>> /* DPIF AVX512 is a default implementation of the userspace datapath
>>    interface. */
>> #define DPIF_AVX512_DEFAULT 1
>>
> 35a44,46
>> /* Define to 1 if compiler supports AVX512. */
>> #define HAVE_AVX512F 1
>>
> 90a102,104
>> /* Define to 1 if binutils correctly supports AVX512. */
>> #define HAVE_LD_AVX512_GOOD 1
>>
> 263a278,280
>>
>> /* Autovalidator for miniflow_extract is a default implementation. */
>> #define MFEX_AUTOVALIDATOR_DEFAULT 1
> 
> Acked-by: Cian Ferriter <cian.ferriter at intel.com>
> 

Thanks!  Applied to master and branch-2.16.

Best regards, Ilya Maximets.