Skip to content

opal/asm: silence xlc warning about typeof usage #8458

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

Conversation

hjelmn
Copy link
Member

@hjelmn hjelmn commented Feb 6, 2021

Signed-off-by: Nathan Hjelm [email protected]

@hjelmn hjelmn requested a review from gpaulsen February 6, 2021 14:27
@ibm-ompi
Copy link

ibm-ompi commented Feb 6, 2021

The IBM CI (PGI) build failed! Please review the log, linked below.

Gist: https://gist.github.com/3f5da063ed91dc374a76f61c90cea2b6

@jjhursey
Copy link
Member

jjhursey commented Feb 8, 2021

IBM Ci had an issue over the weekend. Retrying. bot:ibm:retest

@hjelmn
Copy link
Member Author

hjelmn commented Feb 8, 2021

@jjhursey Thanks. Looks good now.

@hjelmn hjelmn requested a review from jjhursey February 8, 2021 16:50
Copy link
Member

@jjhursey jjhursey left a comment

Choose a reason for hiding this comment

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

Looking at the build log the warning is still present, and this section of code wasn't activated for some reason.

The -qkeyword is defined for the AIX version of XL, but not the Power version. We'll have to look at what we need ot do diferently.

*** C compiler and preprocessor
checking for gcc... (cached) xlc_r
checking whether we are using the GNU C compiler... (cached) yes
checking whether xlc_r accepts -g... (cached) yes
checking for xlc_r option to accept ISO C89... (cached) none needed
checking whether xlc_r understands -c and -o together... (cached) yes
checking if xlc_r requires a flag for C11... yes
configure: checking if xlc_r supports C11 with a flag
checking if xlc_r -std=gnu11 supports C11 _Thread_local... yes
checking if xlc_r -std=gnu11 supports C11 atomic variables... no
checking if xlc_r -std=gnu11 supports C11 _Atomic keyword... no
checking if xlc_r -std=gnu11 supports C11 _Generic keyword... yes
checking if xlc_r -std=gnu11 supports C11 _Static_assert... yes
checking if xlc_r -std=gnu11 supports C11 atomic_fetch_xor_explicit... no
checking if xlc_r -std=c11 supports C11 _Thread_local... yes
checking if xlc_r -std=c11 supports C11 atomic variables... no
checking if xlc_r -std=c11 supports C11 _Atomic keyword... no
checking if xlc_r -std=c11 supports C11 _Generic keyword... yes
checking if xlc_r -std=c11 supports C11 _Static_assert... yes
checking if xlc_r -std=c11 supports C11 atomic_fetch_xor_explicit... no
checking if xlc_r -c11 supports C11 _Thread_local... no
checking if xlc_r -c11 supports C11 atomic variables... no
checking if xlc_r -c11 supports C11 _Atomic keyword... no
checking if xlc_r -c11 supports C11 _Generic keyword... no
checking if xlc_r -c11 supports C11 _Static_assert... no
checking if xlc_r -c11 supports C11 atomic_fetch_xor_explicit... no
checking for xlc_r option to add a directory only to the search path for the quote form of include... -iquote
checking for xlc_r option to accept ISO C99... none needed
checking if xlc_r  supports C11 _Thread_local... yes
checking if xlc_r  supports C11 atomic variables... no
checking if xlc_r  supports C11 _Atomic keyword... no
checking if xlc_r  supports C11 _Generic keyword... yes
checking if xlc_r  supports C11 _Static_assert... yes
checking if xlc_r  supports C11 atomic_fetch_xor_explicit... no
checking if xlc_r  supports __thread... yes
checking if xlc_r  supports C11 _Thread_local... yes
checking for the C compiler vendor... gnu
checking for ANSI C header files... (cached) yes
checking if xlc_r supports -Wno-long-double... no
configure: WARNING:  -Wall -Wundef -Wno-long-long -Wsign-compare -Wmissing-prototypes -Wstrict-prototypes -Wcomment -pedantic -Werror-implicit-function-declaration  has been added to CFLAGS (--enable-picky)
checking if xlc_r supports -finline-functions... yes
checking if xlc_r supports -fno-strict-aliasing... yes
configure: WARNING:  -fno-strict-aliasing has been added to CFLAGS
checking if xlc_r supports __builtin_expect... yes
checking if xlc_r supports __builtin_prefetch... yes
checking if xlc_r supports __builtin_clz... yes
checking for C optimization flags... -O3 -DNDEBUG -Wall -Wundef -Wno-long-long -Wsign-compare -Wmissing-prototypes -Wstrict-prototypes -Wcomment -pedantic -Werror-implicit-function-declaration -finline-functions -fno-strict-aliasing
checking for Interix environment... no

@awlauria
Copy link
Contributor

awlauria commented Feb 8, 2021

@jjhursey @hjelmn it looks like it may be because the compiler got labeled as 'gnu', and not 'ibm'. I fixed that in this PR:
062a72b and 6d7a4d3

#8444

@hjelmn
Copy link
Member Author

hjelmn commented Feb 8, 2021

@awlauria Can you confirm the warning goes away with the PR?

@awlauria
Copy link
Contributor

awlauria commented Feb 8, 2021

I verified that this fix worked for the opal/powerpc atomics:
09dfb12

Since @jjhursey mentioned -qkeyword isn't available on Power machines, maybe we should go for that approach if xl is used. I can add that to #8444 or you can here, either works for me.

@hjelmn
Copy link
Member Author

hjelmn commented Feb 8, 2021

@awlauria Unfortunate that they removed that option yet kept the error. Maybe we should updating the --std flag selection to select c99 (or c11) with extensions? That should also fix it since this is complaining about extensions. I believe for gcc --std=gnu11 is default now so there is a precedent to use the extensions. I will leave it up to you on how we proceed.

@gpaulsen gpaulsen requested a review from awlauria February 8, 2021 20:21
@awlauria
Copy link
Contributor

awlauria commented Feb 8, 2021

@hjelmn this should do it:

2487ed9

@awlauria
Copy link
Contributor

awlauria commented Feb 8, 2021

If adding --std=c99 fixes it, I'd be for that as well. I can test it on our power nodes.

@hjelmn
Copy link
Member Author

hjelmn commented Feb 8, 2021

@awlauria Actually this: -qlanglvl=extc1x. And for non-C11 versions: -qlanglvl=extc99 should do it. Can you check if they do?

@awlauria
Copy link
Contributor

@hjelmn I can't seem to reproduce this warning on master anymore, so I can't verify either way.

Not sure what commit did it.

@gpaulsen
Copy link
Member

gpaulsen commented Mar 2, 2021

@hjelmn, once #8528 goes in, can we just close this?

@hjelmn
Copy link
Member Author

hjelmn commented Mar 2, 2021

@gpaulsen Ideally the LL/SC atomics will still be used even when using C11. I think @awlauria has already fixed this issue by using __typeof__ instead of typeof with xlc so this can be closed.

@gpaulsen
Copy link
Member

retest

@awlauria
Copy link
Contributor

I believe this has long been fixed, but if not this definitely did it:
7590b31

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

Successfully merging this pull request may close these issues.

5 participants