diff mbox series

[ovs-dev,1/6] m4: Don't add -mno-avx512f if compiler doesn't support it.

Message ID 20251009092228.382349-2-i.maximets@ovn.org
State Accepted
Commit 7fb51c4fdd99c51f2e7287f0e9391a85db1c312f
Headers show
Series Build fixes for OVS on old distributions. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Ilya Maximets Oct. 9, 2025, 9:21 a.m. UTC
When building using an older compiler that doesn't support avx512,
we are adding '-mno-avx512f' into the command line, which results
with a build failure:

  gcc: error: unrecognized command line option '-mno-avx512f'

This is a case, for example, while trying to build OVS with GCC 4.8.

Fix that by avoiding binutils check when compiler doesn't understand
-mavx512f.

Later in the call chain there is also an explicit check for -mavx512f
support, but it will just use the cached result, so it's not a problem.

Fixes: 930f135f5ddc ("configure: explicitly disable avx512 if bintuils check fails")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 m4/openvswitch.m4 | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Kevin Traynor Oct. 10, 2025, 11:22 a.m. UTC | #1
On 09/10/2025 10:21, Ilya Maximets wrote:
> When building using an older compiler that doesn't support avx512,
> we are adding '-mno-avx512f' into the command line, which results
> with a build failure:
> 
>   gcc: error: unrecognized command line option '-mno-avx512f'
> 
> This is a case, for example, while trying to build OVS with GCC 4.8.
> 
> Fix that by avoiding binutils check when compiler doesn't understand
> -mavx512f.
> 
> Later in the call chain there is also an explicit check for -mavx512f
> support, but it will just use the cached result, so it's not a problem.
> 
> Fixes: 930f135f5ddc ("configure: explicitly disable avx512 if bintuils check fails")
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
>  m4/openvswitch.m4 | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Acked-by: Kevin Traynor <ktraynor@redhat.com>
Eelco Chaudron Oct. 10, 2025, 11:47 a.m. UTC | #2
On 9 Oct 2025, at 11:21, Ilya Maximets wrote:

> When building using an older compiler that doesn't support avx512,
> we are adding '-mno-avx512f' into the command line, which results
> with a build failure:
>
>   gcc: error: unrecognized command line option '-mno-avx512f'
>
> This is a case, for example, while trying to build OVS with GCC 4.8.
>
> Fix that by avoiding binutils check when compiler doesn't understand
> -mavx512f.
>
> Later in the call chain there is also an explicit check for -mavx512f
> support, but it will just use the cached result, so it's not a problem.
>
> Fixes: 930f135f5ddc ("configure: explicitly disable avx512 if bintuils check fails")
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

Thanks for digging into the root cause of these compiler failures! At first, I thought I’d have to comment on the indentation after your change, but then I noticed the rest of the file also has that one-character indent. So I guess it’s “consistent style” now ;)

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Ilya Maximets Oct. 10, 2025, 8:05 p.m. UTC | #3
On 10/10/25 1:47 PM, Eelco Chaudron wrote:
> 
> 
> On 9 Oct 2025, at 11:21, Ilya Maximets wrote:
> 
>> When building using an older compiler that doesn't support avx512,
>> we are adding '-mno-avx512f' into the command line, which results
>> with a build failure:
>>
>>   gcc: error: unrecognized command line option '-mno-avx512f'
>>
>> This is a case, for example, while trying to build OVS with GCC 4.8.
>>
>> Fix that by avoiding binutils check when compiler doesn't understand
>> -mavx512f.
>>
>> Later in the call chain there is also an explicit check for -mavx512f
>> support, but it will just use the cached result, so it's not a problem.
>>
>> Fixes: 930f135f5ddc ("configure: explicitly disable avx512 if bintuils check fails")
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> 
> Thanks for digging into the root cause of these compiler failures! At first,
> I thought I’d have to comment on the indentation after your change, but then
> I noticed the rest of the file also has that one-character indent. So I guess
> it’s “consistent style” now ;)

Ideally, we need 2-space alignment here, which is a custom in m4 and
autotests, but when I did it this way initially, it's so hard to see
what changed by looking at the diff.  So, I decided to just keep the
diff simple instead.  Hopefully, we can remove this entire section
at some point...

> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>
>
diff mbox series

Patch

diff --git a/m4/openvswitch.m4 b/m4/openvswitch.m4
index 6d41ffc44..060070475 100644
--- a/m4/openvswitch.m4
+++ b/m4/openvswitch.m4
@@ -475,7 +475,9 @@  dnl The checking of binutils funcationality instead of LD version is similar
 dnl to as how DPDK proposes to solve this issue:
 dnl   http://patches.dpdk.org/patch/71723/
 AC_DEFUN([OVS_CHECK_BINUTILS_AVX512],
-  [AC_CACHE_CHECK(
+  [OVS_CHECK_CC_OPTION(
+   [-mavx512f],
+   [AC_CACHE_CHECK(
     [binutils avx512 assembler checks passing],
     [ovs_cv_binutils_avx512_good],
     [dnl Assemble a short snippet to test for issue in "build-aux" dir:
@@ -496,7 +498,8 @@  AC_DEFUN([OVS_CHECK_BINUTILS_AVX512],
      else
        dnl non x86_64 architectures don't have avx512, so not affected
        ovs_cv_binutils_avx512_good=no
-     fi])
+     fi])],
+    [ovs_cv_binutils_avx512_good=no])
    if test "$ovs_cv_binutils_avx512_good" = yes; then
      AC_DEFINE([HAVE_LD_AVX512_GOOD], [1],
                [Define to 1 if binutils correctly supports AVX512.])