> > 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 Ilya & Cian, added a fixes tag to reference that commit and pushed to
master, 2.17 and 2.16.
Regards
Ian
>
> > >
> > > Thanks
> > > Ian
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
