Skip to content

Silence gcc on the use of the GNU typeof extension. #9894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

bosilca
Copy link
Member

@bosilca bosilca commented Jan 20, 2022

The compilation on the M1 laptops was extremely verbose, with hundreds of warnings about using non-standard extensions. This patch disable this warning for the LL/SC atomic load/store.

The same can be done for different compiler (clang supports the same mechanism as gcc).

Signed-off-by: George Bosilca [email protected]

@bosilca bosilca added this to the v5.0.0 milestone Jan 20, 2022
@bosilca bosilca requested a review from hjelmn January 20, 2022 07:30
@jsquyres jsquyres requested a review from bwbarrett January 20, 2022 15:45
@@ -228,7 +229,10 @@ static inline opal_list_item_t *opal_fifo_pop_atomic(opal_fifo_t *fifo)
attempt = 0;
}

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wlanguage-extension-token"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather get rid of the typeof() usage in the ll/sc code. I have a bunch of patches to clean up the atomics code; let me go see how bad that change would make things and then we can come back to this. But we really don't want to pragma every usage of the opal_atomic_ll code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only have to protect the use of opal_atomic_ll because they are defined as macros (and the diagnostic do not apply to macros when they are read, but only when they are applied. If we convert he macro into an inlined function then we can have the pragma in a single location.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nathan originally used inline functions, but even with always_inline primitives, couldn't keep all the compilers from sometimes not inlining the function. And once not inlined, there was a very high probability of the link failing due to some write in setting up the function call, causing the conditional store to fail, infinitely.

But I think I found a better solution to this problem. Unfortunately, I checked in my branch full of atomic changes. I'll post it later today.

The same can be done for different compiler (clang supports the same
mechanism as gcc).

Signed-off-by: George Bosilca <[email protected]>
@bosilca bosilca force-pushed the topic/silence_ll_sc branch from 39e1b64 to 15c3a4b Compare January 20, 2022 17:26
@bwbarrett
Copy link
Member

I think #9900 might be a more elegant solution. Thoughts?

@devreal
Copy link
Contributor

devreal commented Jan 20, 2022

This one might be related (and probably should be closed eventually): #8458

@bwbarrett
Copy link
Member

Closing, as #9900 has been committed.

@bwbarrett bwbarrett closed this Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants