> -----Original Message-----
> From: Ilya Maximets <i.maxim...@ovn.org>
> Sent: Wednesday 2 February 2022 13:57
> To: Stokes, Ian <ian.sto...@intel.com>; Ferriter, Cian
> <cian.ferri...@intel.com>; William Tu
> <u9012...@gmail.com>; d...@openvswitch.org
> Cc: i.maxim...@ovn.org
> Subject: Re: [ovs-dev] [PATCH] acinclude: Detect avx512 vpopcntdq compiler
> support.
>
> On 2/2/22 13:57, Stokes, Ian wrote:
> >>> -----Original Message-----
> >>> From: Ferriter, Cian
> >>> Sent: Friday 28 January 2022 16:32
> >>> To: William Tu <u9012...@gmail.com>; d...@openvswitch.org
> >>> Subject: RE: [ovs-dev] [PATCH] acinclude: Detect avx512 vpopcntdq compiler
> >> support.
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: dev <ovs-dev-boun...@openvswitch.org> On Behalf Of William Tu
> >>>> Sent: Thursday 27 January 2022 03:45
> >>>> To: d...@openvswitch.org
> >>>> Subject: [ovs-dev] [PATCH] acinclude: Detect avx512 vpopcntdq compiler
> >> support.
> >>>>
> >>>> Ubuntu Xenial 16.04 is using GCC 5.4 and it does not support
> >>>> target "-mavx512vpopcntdq" and cuases error
> >>>> lib/dpif-netdev-lookup-avx512-gather.c:356:47:
> >>>> error: attribute(target("avx512vpopcntdq")) is unknown
> >>>> GCC 7+ supports vpopcntdq:
> >>>> https://gcc.gnu.org/gcc-7/changes.html
> >>>> The patch detects vpopcntdq and disables AVX512 when not found.
> >>>>
> >>>> Reported-by: Greg Rose <gvrose8...@gmail.com>
> >>>> Signed-off-by: William Tu <u9012...@gmail.com>
> >>>> ---
> >>>> acinclude.m4 | 2 +-
> >>>> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/acinclude.m4 b/acinclude.m4
> >>>> index 5c971e98ce91..0c360fd1ef73 100644
> >>>> --- a/acinclude.m4
> >>>> +++ b/acinclude.m4
> >>>> @@ -77,7 +77,7 @@ dnl Checks if compiler and binutils supports AVX512.
> >>>> AC_DEFUN([OVS_CHECK_AVX512], [
> >>>> OVS_CHECK_BINUTILS_AVX512
> >>>> OVS_CHECK_CC_OPTION(
> >>>> - [-mavx512f], [ovs_have_cc_mavx512f=yes],
> >> [ovs_have_cc_mavx512f=no])
> >>>> + [-mavx512f -mavx512vpopcntdq], [ovs_have_cc_mavx512f=yes],
> >> [ovs_have_cc_mavx512f=no])
> >>>> AM_CONDITIONAL([HAVE_AVX512F], [test $ovs_have_cc_mavx512f =
> >> yes])
> >>>> if test "$ovs_have_cc_mavx512f" = yes; then
> >>>> AC_DEFINE([HAVE_AVX512F], [1],
> >>>> --
> >>>
> >>> Hi William,
> >>>
> >>> Thanks for this fix, I can verify that this fixes the below error for GCC
> >>> 5 (it will
> >> work for GCC 4.9
> >>> - GCC 6):
> >>> make[3]: Entering directory '/root/cian/ovs/datapath'
> >>> lib/dpif-netdev-lookup-avx512-gather.c:90:1: error:
> >> attribute(target("avx512vpopcntdq")) is unknown
> >>>
> >>> We would like to re-spin a new version of this patch that fixes the
> >>> compile
> >> error for the relevant GCC
> >>> versions (GCC 4.9 - GCC 6) but allows some AVX512 code to be build.
> >>> For setups with GCC 6 for example, we still have support for most of the
> >> AVX512 ISA used in OVS, so we
> >>> can still enable building of this ISA, while still correctly avoiding the
> >> avx512vpopcntdq error that
> >>> your patch does.
> >>>
> >>> Please allow us a few days to test and re-spin a new version of this
> >>> patch.
> >>> Thanks,
> >>> Cian
> >>
> >> Hi again William,
> >>
> >> I have been looking into a new version of this patch to add partial
> >> support to
> >> GCC versions that don't have full support for all the AVX512 ISA we use in
> >> OVS.
> >> This is still a work in progress.
> >>
> >> While I'm looking into this more, I think it doesn't make sense to hold
> >> back this
> >> patch from being merged. It fixes build issues.
> >>
> >> I think the patch should be applied as is. We can add further improvements
> >> later.
> >>
> >> Acked-by: Cian Ferriter <cian.ferri...@intel.com>
> >>
> >> More details about what I tested:
> >> I switched to GCC 5 on my system and can see the compile issue below before
> >> applying the patch.
> >> After applying the patch, the build is successful.
> >> Before patch
> >> Configure message:
> >> checking whether gcc accepts -mavx512f... yes
> >>
> >> Build error:
> >> make[3]: Entering directory '/root/cian/ovs/datapath'
> >> lib/dpif-netdev-lookup-avx512-gather.c:90:1: error:
> >> attribute(target("avx512vpopcntdq")) is unknown
> >>
> >> After patch
> >> Configure message:
> >> checking whether gcc accepts -mavx512f -mavx512vpopcntdq... no
> >>
> >> On GCC 4.8 before the patch is applied, the build is actually successful,
> >> since the
> >> "avx512f" only check fails, so the AVX512 build is disabled.
> >> On GCC 4.8 after the patch is applied, the build still works as expected.
> >> Before patch
> >> Configure message:
> >> checking whether gcc -std=gnu99 accepts -mavx512f... no
> >>
> >> After patch
> >> Configure message:
> >> checking whether gcc -std=gnu99 accepts -mavx512f -mavx512vpopcntdq... no
> >>
> >> I also checked that all was working as expected with the compile time
> >> AVX512
> >> flags:
> >> --enable-dpif-default-avx512 --enable-autovalidator --enable-mfex-default-
> >> autovalidator
> >>
> >> I can confirm that these compile time flags don't cause issues.
> >>
> >> Finally, I tested and confirmed that OVS was still building and running
> >> with
> >> AVX512 support when built with GCC 9.
> >
> > Thanks for the patch William and thanks for testing Cian.
> >
> > Just looking and it seems this should be backported as far as 2.16 if I'm
> > correct? At least from
> code inspection I see reference to vpopcntdq mainly around dpif (If memory
> serves me correctly it was
> 2.16 when AVX512 was introduced to dpif).
> >
> > Thoughts?
>
> Yes, the backport is needed. I believe we had this issue reported
> for 2.16 on IRC in September, but we didn't have enough details,
> and no real actions followed:
> https://mail.openvswitch.org/pipermail/ovs-dev/2021-September/387675.html
>
Agreed, it is enough to backport this as far as 2.16. The commit that
introduced the use of this ISA is below. The first release this commit made was
2.16.
commit 1e314891340d9b964f2da975936974b09912c225
Author: Harry van Haaren <harry.van.haa...@intel.com>
AuthorDate: Fri Jul 9 15:58:24 2021 +0000
Commit: Ian Stokes <ian.sto...@intel.com>
CommitDate: Fri Jul 9 17:15:08 2021 +0100
dpcls-avx512: Enable avx512 vector popcount instruction.
Thanks,
Cian
> >
> > Thanks
> > Ian
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
