Silence numerous warnings from Studio compilers

[PHHargrove]

This commit adds selective use of a compiler-specific pragma to
silence the numerous warnings the Sun/Oracle/Studio compilers emit for
the GNU-style inline asm used in atomic.h.

Now that there are a non-trivial number of real warnings, the improvement
this change make to the signal-to-noise ratio is valuable.

I would like follow-up with a PR for v2.x as well (with 2.0.2 milestone) if this is PR is accepted.

[ibm-ompi]

Build Failed with GNU compiler! Please review the log, and get in touch if you have questions.

Gist: https://gist.github.com/d15cd6e0a115b846bf0d75ede57b5742

[PHHargrove]

The "IBM-CI (GNU Compiler)" failure appears spurious:

Assembler messages:
Error: can't open /tmp/ccH92ZHh.s for reading: No such file or directory
make[2]: *** [base/pml_base_ft.lo] Error 1
make[2]: *** Waiting for unfinished jobs....
Assembler messages:
Error: can't open /tmp/ccdUQ8vh.s for reading: No such file or directory
make[2]: *** [base/pml_base_request.lo] Error 1
Assembler messages:
Error: can't open /tmp/ccTxw7Kh.s for reading: No such file or directory
make[2]: *** [base/pml_base_recvreq.lo] Error 1
Assembler messages:
Error: can't open /tmp/cc3yOFHh.s for reading: No such file or directory
make[2]: *** [base/pml_base_sendreq.lo] Error 1
Assembler messages:
Error: can't open /tmp/ccs1gJph.s for reading: No such file or directory
make[2]: *** [base/pml_base_frame.lo] Error 1
Assembler messages:
Error: can't open /tmp/ccFRfyLh.s for reading: No such file or directory
make[2]: *** [base/pml_base_select.lo] Error 1
Assembler messages:
Error: can't open /tmp/ccEm8pKh.s for reading: No such file or directory
make[2]: *** [base/pml_base_bsend.lo] Error 1

bot:ibm:retest

[ibm-ompi]

Build Failed with XL compiler! Please review the log, and get in touch if you have questions.

Gist: https://gist.github.com/23b036abbb6b69c3a803c77c5db2b3fd

[PHHargrove]

On the second pass IBM passed w/ GNU and failed w/ XLC.
Again the failure is /tmp related:

Failed to open file "/tmp/_ipa_128322_ANNEAc"
1586-346 (U) An error occurred during code generation.  The code generation return code was 1.
make[2]: *** [mca_base_pvar.lo] Error 1

[ibm-ompi]

Build Failed with GNU compiler! Please review the log, and get in touch if you have questions.

Gist: https://gist.github.com/201509b7852f50f5b8530b53c1e88d1e

[ggouaillardet]

@PHHargrove that looks a bit odd to me.

Iirc, I silenced all these warnings in 5dae7a4
If I understand your patch correctly, this is a workaround that silence the error message, without really fixing the (so called) declared but unused parameters.

I will review this tomorrow.

Btw, are you using the latest (and non beta) 12.4 version ?

[jjhursey]

IBM CI test ran into nightly MTT testing and bad things happened. Let's try again:
bot:ibm:retest

[ibm-ompi]

Build Failed with GNU compiler! Please review the log, and get in touch if you have questions.

Gist: https://gist.github.com/f9a416c01fc5795d67aa7155e8c64432

[jjhursey]

bot:ibm:retest

[PHHargrove]

@ggouaillardet

I am using the latest 12.5 compilers (no longer beta).

I don't have build logs on hand for master, and the pmix3x/flock problem prevents me from building master on Solaris right now. However, I recall that there is still one warning in the amd64/atomic.h code and numerous ones in the ia32 code. Since Solaris on amd64 h/w has a default ABI of ILP32, it is the ia32 code that is in greater need of a fix.

Since you have at least a good start on fixing the warning for real (not suppressing them as I had tried), I wil withdraw this PR in favor of getting the asm fixed correctly.

[ggouaillardet]

yes, oracle compilers do issue tons of warnings that looks like

../../../../src/ompi-master/opal/include/opal/sys/gcc_builtin/atomic.h", line 73: warning: argument #2 is incompatible with prototype:
    prototype: pointer to void : "../../../../src/ompi-master/opal/datatype/opal_datatype_unpack.c", line 0
    argument : pointer to volatile int

let's try this sample program

#include <stdbool.h>

static inline int atomic( volatile int *addr, int oldval, int newval)
{
    return __atomic_compare_exchange_n (addr, &oldval, newval, false,
                                        __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
}

gcc is perfectly happy with that, even with the ``-Wall` option

but cc is not

$ /opt/oracle/developerstudio12.5/bin/cc -c atomic.c 
"atomic.c", line 5: warning: argument #2 is incompatible with prototype:
    prototype: pointer to void : "atomic.c", line 0
    argument : pointer to volatile int

if i modify this program like this

$ cat atomic.c 
#include <stdbool.h>

static inline int atomic( volatile int *addr, int oldval, int newval)
{
    return __atomic_compare_exchange_n (addr, (void *)&oldval, newval, false,
                                        __ATOMIC_ACQUIRE, __ATOMIC_RELAXED);
}

gcc -Wall is still very happy, but thing got worst from cc point of view

$ /opt/oracle/developerstudio12.5/bin/cc -c atomic.c 
"atomic.c", line 6: argument #2 of "__atomic_compare_exchange_n" should be non-void pointer type
"atomic.c", line 5: warning: argument #2 is incompatible with prototype:
    prototype: pointer to void : "atomic.c", line 0
    argument : pointer to volatile int

if i read correctly

1. the second argument should not be a void *
2. the second argument is a volatile int * but a void * is expected !!!

i initially thought this was similar to what i fixed earlier at 5dae7a4, but this is a different issue.

to me, that looks like a compiler bug, so for the time being, silencing these warnings with directives is a good option

@hjelmn can you please comment on that PR ?

[PHHargrove]

@ggouaillardet wrote

i initially thought this was similar to what i fixed earlier at 5dae7a4, but this is a different issue.

No, you were correct that I was trying to fix that same issue which is why I closed this PR.

The "incompatible with prototype" message are separate from what this PR was intended to fix.

I believe the compiler doesn't care about passing int * where void * is expected, but does care about the fact that one carries the volatile qualifier and the other doesn't. I agree that is not a desirable compiler behavior.

The directives I added only suppressed the unused argument warnings. However, it should be possible to change the 2nd argument to the pragma to E_ARG_INCOMPATIBLE_WITH_ARG_L to suppress these "new" warnings instead.

[ggouaillardet]

@PHHargrove I tried several things, including using a temporary variable without the volatile qualifier, but this did not help.

I ll look for a way to report that to Oracle and see what they can do about it

[ggouaillardet]

i reported this to Oracle at https://community.oracle.com/thread/3968347

[ggouaillardet]

@PHHargrove so here is my revised and simpler version of this PR

commit 9cfbca802f2f82ca97f7c06e8fad88c5123559a5
Author: Paul H. Hargrove <PHHargrove@lbl.gov>
Date:   Mon Aug 29 18:13:18 2016 -0700

    Silence numerous warnings from Studio compilers

    This commit adds selective use of a compiler-specific pragma to
    silence the numerous warnings the Sun/Oracle/Studio compilers emit for
    the GNU-style inline asm used in atomic.h.

diff --git a/opal/include/opal/sys/gcc_builtin/atomic.h b/opal/include/opal/sys/gcc_builtin/atomic.h
index 82b75f4..319bb3b 100644
--- a/opal/include/opal/sys/gcc_builtin/atomic.h
+++ b/opal/include/opal/sys/gcc_builtin/atomic.h
@@ -67,6 +67,14 @@ static inline void opal_atomic_wmb(void)
  *
  *********************************************************************/

+/*
+ * Suppress numerous (spurious ?) warnings from Oracle Studio compilers
+ * see https://community.oracle.com/thread/3968347
+ */ 
+#if defined(__SUNPRO_C) || defined(__SUNPRO_CC)
+#pragma error_messages(off, E_ARG_INCOMPATIBLE_WITH_ARG_L)
+#endif
+
 static inline int opal_atomic_cmpset_acq_32( volatile int32_t *addr,
                                              int32_t oldval, int32_t newval)
 {
@@ -211,4 +219,8 @@ static inline void opal_atomic_unlock (opal_atomic_lock_t *lock)

 #endif

+#if defined(__SUNPRO_C) || defined(__SUNPRO_CC)
+#pragma error_messages(default, E_ARG_INCOMPATIBLE_WITH_ARG_L)
+#endif
+
 #endif /* ! OPAL_SYS_ARCH_ATOMIC_H */

do you mind updating this PR so you get properly credited from a git point of view ?

[EDIT] i updated the link to Oracle

[ggouaillardet]

@PHHargrove just to be clear, this patch replaces the existing PR
(e.g. only one file is modified instead of two)

[PHHargrove]

@ggouaillardet
Since I don't have a signed contributors agreement, lets not go to the trouble of updating the PR to give me the authorship credit.

[ggouaillardet]

ok, i pushed 894be78 and credited you in the commit message.