diff mbox series

[ovs-dev] ovs-atomic: Fix inclusion of Clang header by GCC 14.

Message ID 20240118145910.3371452-1-i.maximets@ovn.org
State Accepted
Commit 335a5deac3ff91448ca14651e92f39dfdd512fcf
Delegated to: Ilya Maximets
Headers show
Series [ovs-dev] ovs-atomic: Fix inclusion of Clang header by GCC 14. | expand

Checks

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

Commit Message

Ilya Maximets Jan. 18, 2024, 2:59 p.m. UTC
GCC 14 started to advertise c_atomic extension, older versions didn't
do that.  Add check for __clang__, so GCC doesn't include headers
designed for Clang.

Another option would be to prefer stdatomic implementation instead,
but some older versions of Clang are not able to use stdatomic.h
supplied by GCC as described in commit:
  07ece367fb5f ("ovs-atomic: Prefer Clang intrinsics over <stdatomic.h>.")

This change fixes OVS build with GCC on Fedora Rawhide (40).

Reported-by: Jakob Meng <code@jakobmeng.de>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 lib/ovs-atomic.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jakob Meng Jan. 18, 2024, 3:05 p.m. UTC | #1
On 18.01.24 15:59, Ilya Maximets wrote:
> GCC 14 started to advertise c_atomic extension, older versions didn't
> do that.  Add check for __clang__, so GCC doesn't include headers
> designed for Clang.
>
> Another option would be to prefer stdatomic implementation instead,
> but some older versions of Clang are not able to use stdatomic.h
> supplied by GCC as described in commit:
>   07ece367fb5f ("ovs-atomic: Prefer Clang intrinsics over <stdatomic.h>.")
>
> This change fixes OVS build with GCC on Fedora Rawhide (40).
>
> Reported-by: Jakob Meng <code@jakobmeng.de>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
>  lib/ovs-atomic.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
> index ab9ce6b2e..f140d25fe 100644
> --- a/lib/ovs-atomic.h
> +++ b/lib/ovs-atomic.h
> @@ -328,7 +328,7 @@
>      #if __CHECKER__
>          /* sparse doesn't understand some GCC extensions we use. */
>          #include "ovs-atomic-pthreads.h"
> -    #elif __has_extension(c_atomic)
> +    #elif __clang__ &&  __has_extension(c_atomic)
>          #include "ovs-atomic-clang.h"
>      #elif HAVE_ATOMIC && __cplusplus >= 201103L
>          #include "ovs-atomic-c++.h"

Tested with latest gcc 14 [0] from Fedora Rawhide and it fixed all my OVS compilation issues. Thank you!

[0] gcc (GCC) 14.0.1 20240113 (Red Hat 14.0.1-0)

Acked-by: Jakob Meng <jmeng@redhat.com>
Eelco Chaudron Jan. 18, 2024, 3:43 p.m. UTC | #2
On 18 Jan 2024, at 15:59, Ilya Maximets wrote:

> GCC 14 started to advertise c_atomic extension, older versions didn't
> do that.  Add check for __clang__, so GCC doesn't include headers
> designed for Clang.
>
> Another option would be to prefer stdatomic implementation instead,
> but some older versions of Clang are not able to use stdatomic.h
> supplied by GCC as described in commit:
>   07ece367fb5f ("ovs-atomic: Prefer Clang intrinsics over <stdatomic.h>.")
>
> This change fixes OVS build with GCC on Fedora Rawhide (40).
>
> Reported-by: Jakob Meng <code@jakobmeng.de>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

Seem to work fine on all my older build environments with both GCC and clang.

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Ilya Maximets Jan. 18, 2024, 4:23 p.m. UTC | #3
On 1/18/24 15:59, Ilya Maximets wrote:
> GCC 14 started to advertise c_atomic extension, older versions didn't
> do that.  Add check for __clang__, so GCC doesn't include headers
> designed for Clang.
> 
> Another option would be to prefer stdatomic implementation instead,
> but some older versions of Clang are not able to use stdatomic.h
> supplied by GCC as described in commit:
>   07ece367fb5f ("ovs-atomic: Prefer Clang intrinsics over <stdatomic.h>.")
> 
> This change fixes OVS build with GCC on Fedora Rawhide (40).
> 
> Reported-by: Jakob Meng <code@jakobmeng.de>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---
>  lib/ovs-atomic.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
> index ab9ce6b2e..f140d25fe 100644
> --- a/lib/ovs-atomic.h
> +++ b/lib/ovs-atomic.h
> @@ -328,7 +328,7 @@
>      #if __CHECKER__
>          /* sparse doesn't understand some GCC extensions we use. */
>          #include "ovs-atomic-pthreads.h"
> -    #elif __has_extension(c_atomic)
> +    #elif __clang__ &&  __has_extension(c_atomic)
>          #include "ovs-atomic-clang.h"
>      #elif HAVE_ATOMIC && __cplusplus >= 201103L
>          #include "ovs-atomic-c++.h"


Recheck-request: github-robot
Simon Horman Jan. 18, 2024, 4:30 p.m. UTC | #4
On Thu, Jan 18, 2024 at 03:59:05PM +0100, Ilya Maximets wrote:
> GCC 14 started to advertise c_atomic extension, older versions didn't
> do that.  Add check for __clang__, so GCC doesn't include headers
> designed for Clang.
> 
> Another option would be to prefer stdatomic implementation instead,
> but some older versions of Clang are not able to use stdatomic.h
> supplied by GCC as described in commit:
>   07ece367fb5f ("ovs-atomic: Prefer Clang intrinsics over <stdatomic.h>.")
> 
> This change fixes OVS build with GCC on Fedora Rawhide (40).
> 
> Reported-by: Jakob Meng <code@jakobmeng.de>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

Recheck-request: github-robot
Simon Horman Jan. 19, 2024, 12:05 p.m. UTC | #5
On Thu, Jan 18, 2024 at 04:30:06PM +0000, Simon Horman wrote:
> On Thu, Jan 18, 2024 at 03:59:05PM +0100, Ilya Maximets wrote:
> > GCC 14 started to advertise c_atomic extension, older versions didn't
> > do that.  Add check for __clang__, so GCC doesn't include headers
> > designed for Clang.
> > 
> > Another option would be to prefer stdatomic implementation instead,
> > but some older versions of Clang are not able to use stdatomic.h
> > supplied by GCC as described in commit:
> >   07ece367fb5f ("ovs-atomic: Prefer Clang intrinsics over <stdatomic.h>.")
> > 
> > This change fixes OVS build with GCC on Fedora Rawhide (40).
> > 
> > Reported-by: Jakob Meng <code@jakobmeng.de>
> > Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> 
> Recheck-request: github-robot

That run did not work either [1],
but I was able to get a successful run in my own
GitHub repository [2].

[1] https://github.com/ovsrobot/ovs/actions/runs/7572217223/job/20625604785
[2] https://github.com/horms/ovs/actions/runs/7581260421

Acked-by: Simon Horman <horms@ovn.org>
Ilya Maximets Jan. 19, 2024, 12:58 p.m. UTC | #6
On 1/19/24 13:05, Simon Horman wrote:
> On Thu, Jan 18, 2024 at 04:30:06PM +0000, Simon Horman wrote:
>> On Thu, Jan 18, 2024 at 03:59:05PM +0100, Ilya Maximets wrote:
>>> GCC 14 started to advertise c_atomic extension, older versions didn't
>>> do that.  Add check for __clang__, so GCC doesn't include headers
>>> designed for Clang.
>>>
>>> Another option would be to prefer stdatomic implementation instead,
>>> but some older versions of Clang are not able to use stdatomic.h
>>> supplied by GCC as described in commit:
>>>   07ece367fb5f ("ovs-atomic: Prefer Clang intrinsics over <stdatomic.h>.")
>>>
>>> This change fixes OVS build with GCC on Fedora Rawhide (40).
>>>
>>> Reported-by: Jakob Meng <code@jakobmeng.de>
>>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>>
>> Recheck-request: github-robot
> 
> That run did not work either [1],
> but I was able to get a successful run in my own
> GitHub repository [2].
> 
> [1] https://github.com/ovsrobot/ovs/actions/runs/7572217223/job/20625604785
> [2] https://github.com/horms/ovs/actions/runs/7581260421
> 
> Acked-by: Simon Horman <horms@ovn.org>
> 


Thanks, Simon, Jakob and Eelco!

Applied and backported down to 2.17.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/lib/ovs-atomic.h b/lib/ovs-atomic.h
index ab9ce6b2e..f140d25fe 100644
--- a/lib/ovs-atomic.h
+++ b/lib/ovs-atomic.h
@@ -328,7 +328,7 @@ 
     #if __CHECKER__
         /* sparse doesn't understand some GCC extensions we use. */
         #include "ovs-atomic-pthreads.h"
-    #elif __has_extension(c_atomic)
+    #elif __clang__ &&  __has_extension(c_atomic)
         #include "ovs-atomic-clang.h"
     #elif HAVE_ATOMIC && __cplusplus >= 201103L
         #include "ovs-atomic-c++.h"