| Message ID | 20220127034526.11692-1-u9012063@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev] acinclude: Detect avx512 vpopcntdq compiler support. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 1/26/2022 7:45 PM, William Tu wrote: > 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 <gvrose8192@gmail.com> > Signed-off-by: William Tu <u9012063@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], Tested-by: Greg Rose <gvrose8192@gmail.com> Reviewed-by: Greg Rose <gvrose8192@gmail.com> Thanks William!
> -----Original Message----- > From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of William Tu > Sent: Thursday 27 January 2022 03:45 > To: dev@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 <gvrose8192@gmail.com> > Signed-off-by: William Tu <u9012063@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
> -----Original Message----- > From: Ferriter, Cian > Sent: Friday 28 January 2022 16:32 > To: William Tu <u9012063@gmail.com>; dev@openvswitch.org > Subject: RE: [ovs-dev] [PATCH] acinclude: Detect avx512 vpopcntdq compiler support. > > > > -----Original Message----- > > From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of William Tu > > Sent: Thursday 27 January 2022 03:45 > > To: dev@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 <gvrose8192@gmail.com> > > Signed-off-by: William Tu <u9012063@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.ferriter@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.
> > -----Original Message----- > > From: Ferriter, Cian > > Sent: Friday 28 January 2022 16:32 > > To: William Tu <u9012063@gmail.com>; dev@openvswitch.org > > Subject: RE: [ovs-dev] [PATCH] acinclude: Detect avx512 vpopcntdq compiler > support. > > > > > > > -----Original Message----- > > > From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of William Tu > > > Sent: Thursday 27 January 2022 03:45 > > > To: dev@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 <gvrose8192@gmail.com> > > > Signed-off-by: William Tu <u9012063@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.ferriter@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? Thanks Ian > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On 2/2/22 13:57, Stokes, Ian wrote: >>> -----Original Message----- >>> From: Ferriter, Cian >>> Sent: Friday 28 January 2022 16:32 >>> To: William Tu <u9012063@gmail.com>; dev@openvswitch.org >>> Subject: RE: [ovs-dev] [PATCH] acinclude: Detect avx512 vpopcntdq compiler >> support. >>> >>> >>>> -----Original Message----- >>>> From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of William Tu >>>> Sent: Thursday 27 January 2022 03:45 >>>> To: dev@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 <gvrose8192@gmail.com> >>>> Signed-off-by: William Tu <u9012063@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.ferriter@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 > > Thanks > Ian
> -----Original Message----- > From: Ilya Maximets <i.maximets@ovn.org> > Sent: Wednesday 2 February 2022 13:57 > To: Stokes, Ian <ian.stokes@intel.com>; Ferriter, Cian <cian.ferriter@intel.com>; William Tu > <u9012063@gmail.com>; dev@openvswitch.org > Cc: i.maximets@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 <u9012063@gmail.com>; dev@openvswitch.org > >>> Subject: RE: [ovs-dev] [PATCH] acinclude: Detect avx512 vpopcntdq compiler > >> support. > >>> > >>> > >>>> -----Original Message----- > >>>> From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of William Tu > >>>> Sent: Thursday 27 January 2022 03:45 > >>>> To: dev@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 <gvrose8192@gmail.com> > >>>> Signed-off-by: William Tu <u9012063@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.ferriter@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.haaren@intel.com> AuthorDate: Fri Jul 9 15:58:24 2021 +0000 Commit: Ian Stokes <ian.stokes@intel.com> CommitDate: Fri Jul 9 17:15:08 2021 +0100 dpcls-avx512: Enable avx512 vector popcount instruction. Thanks, Cian > > > > Thanks > > Ian
> > On 2/2/22 13:57, Stokes, Ian wrote: > > >>> -----Original Message----- > > >>> From: Ferriter, Cian > > >>> Sent: Friday 28 January 2022 16:32 > > >>> To: William Tu <u9012063@gmail.com>; dev@openvswitch.org > > >>> Subject: RE: [ovs-dev] [PATCH] acinclude: Detect avx512 vpopcntdq > compiler > > >> support. > > >>> > > >>> > > >>>> -----Original Message----- > > >>>> From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of William > Tu > > >>>> Sent: Thursday 27 January 2022 03:45 > > >>>> To: dev@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 <gvrose8192@gmail.com> > > >>>> Signed-off-by: William Tu <u9012063@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.ferriter@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.haaren@intel.com> > AuthorDate: Fri Jul 9 15:58:24 2021 +0000 > Commit: Ian Stokes <ian.stokes@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 >
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],
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 <gvrose8192@gmail.com> Signed-off-by: William Tu <u9012063@gmail.com> --- acinclude.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)