Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] Compile-time MPI_Datatype checking
From: Paul Hargrove (phhargrove_at_[hidden])
Date: 2012-10-31 15:11:19


Ralph,

I work at a National Lab, and like many of my peers I develop/prototype
codes on my desktop and/or laptop. So, I think the default behavior of
mpicc on a Clang-based Mac is entirely relevant.

FWIW:
I agree w/ Jeff that these datatype checking warnings "feel" like
a candidate for "-Wall" (or "-Wextra"), rather than enabled by default.

-Paul

On Wed, Oct 31, 2012 at 12:04 PM, Ralph Castain <rhc_at_[hidden]> wrote:

> Understood, but also remember that the national labs don't have Mac
> clusters - and so they couldn't care less about Clang. The concerns over
> these changes were from the national labs, so my point was that this
> discussion may all be irrelevant.
>
>
> On Oct 31, 2012, at 11:47 AM, Paul Hargrove <phhargrove_at_[hidden]> wrote:
>
> Note that with Apple's latest versions of Xcode (4.2 and higher, IIRC)
> Clang is now the default C compiler. I am told that Clang is the ONLY
> bundled compiler for OSX 10.8 (Mountain Lion) unless you take extra steps
> to install gcc (which is actually llvm-gcc and cross-compiles for OSX 10.7).
>
> So, Clang *is* gaining some "market share", though not yet in major HPC
> systems.
>
> -Paul
>
>
> On Wed, Oct 31, 2012 at 11:40 AM, Ralph Castain <rhc_at_[hidden]> wrote:
>
>> If it's only on for Clang, I very much doubt anyone will care - I'm
>> unaware of any of our users that currently utilize that compiler, and
>> certainly not on the clusters in the national labs (gcc, Intel, etc. - but
>> I've never seen them use Clang).
>>
>> Not saying anything negative about Clang - just noting it isn't much used
>> in our current community that I've heard.
>>
>>
>> On Oct 31, 2012, at 11:19 AM, Dmitri Gribenko <gribozavr_at_[hidden]>
>> wrote:
>>
>> > On Wed, Oct 31, 2012 at 5:04 PM, Jeff Squyres <jsquyres_at_[hidden]>
>> wrote:
>> >> On Oct 31, 2012, at 9:38 AM, Dmitri Gribenko wrote:
>> >>
>> >>>> The rationale here is that correct MPI applications should not need
>> to add any extra compiler files to compile without warnings.
>> >>>
>> >>> I would disagree with this. Compiler warnings are most useful when
>> >>> they are on by default. Only a few developers will turn on a warning
>> >>> because warnings are hard to discover and enabling a warning requires
>> >>> an explicit action from the developer.
>> >>
>> >> Understood, but:
>> >>
>> >> a) MPI explicitly allows this kind of deliberate mismatch. It does
>> not make sense to warn for things that are correct in MPI.
>> >
>> > I don't think it is MPI. It is the C standard that allows one to
>> > store any pointer in void* and char*. But C standard also considers
>> > lots of other weird things to be valid, see below.
>> >
>> >> b) Warnings are significantly less useful if the user looks at them
>> and says, "the compiler is wrong; I know that MPI says that this deliberate
>> mismatch in my code is ok."
>> >
>> > Well, one can also argue that since the following is valid C, the
>> > warning in question should not be implemented at all:
>> >
>> > long *b = malloc(sizeof(int));
>> > MPI_Recv(b, 1, MPI_INT, ...);
>> >
>> > But this code is clearly badly written, so we are left with a question
>> > about where to draw the line.
>> >
>> >> c) as such, these warnings are really only useful for the application
>> where type/MPI_Datatype matching is expected/desired.
>> >
>> > Compilers already warn about valid C code. Silencing many warnings
>> > relies on conventions that are derived from best practices of being
>> > explicit about something unusual. For example:
>> >
>> > $ cat /tmp/aaa.c
>> > void foo(void *a) {
>> > for(int i = a; i < 10; i++)
>> > {
>> > if(i = 5)
>> > return;
>> > }
>> > }
>> > $ clang -fsyntax-only -std=c99 /tmp/aaa.c
>> > /tmp/aaa.c:2:11: warning: incompatible pointer to integer conversion
>> > initializing 'int' with an expression of type 'void *'
>> > [-Wint-conversion]
>> > for(int i = a; i < 10; i++)
>> > ^ ~
>> > /tmp/aaa.c:4:10: warning: using the result of an assignment as a
>> > condition without parentheses [-Wparentheses]
>> > if(i = 5)
>> > ~~^~~
>> > /tmp/aaa.c:4:10: note: place parentheses around the assignment to
>> > silence this warning
>> > if(i = 5)
>> > ^
>> > ( )
>> > /tmp/aaa.c:4:10: note: use '==' to turn this assignment into an
>> > equality comparison
>> > if(i = 5)
>> > ^
>> > ==
>> > 2 warnings generated.
>> >
>> > According to C standard this is valid C code, but clang emits two
>> > warnings on this.
>> >
>> >> Can these warnings be enabled as part of the warnings rollup -Wall
>> option? That would be an easy way to find/enable these warnings.
>> >
>> > IIRC, -Wall warning set is frozen in clang. -Wall is misleading in
>> > that it does not turn on all warnings implemented in the compiler.
>> > Clang has -Weverything to really turn on all warnings. But
>> > -Weverything is very noisy (by design, not to be fixed) unless one
>> > also turns off all warnings that are not interesting for the project
>> > with -Wno-foo.
>> >
>> > I don't think it is possible to disable this warning by default
>> > because off-by-default warnings are discouraged in Clang. There is no
>> > formal policy, but the rule of thumb is: either make the warning good
>> > enough for everyone or don't implement it; if some particular app does
>> > something strange, it can disable this warning.
>> >
>> >>> The pattern you described is an important one, but most MPI
>> >>> applications will have matching buffer types/type tags.
>> >>
>> >> I agree that most applications *probably* don't do this. But
>> significant developer in this community (i.e., Sandia) has at least
>> multiple applications that *do* do it. I can't ignore that. :-(
>> >
>> > Here are a few approaches to solving this in order of preference:
>> >
>> > 0. Is this really a concern for Sandia? (I.e., do they target Clang?)
>> >
>> > 1. Ask the developer to silence the warning with a cast to 'void *' or
>> > -Wno-type-safety. Rationale: compilers already do warn about valid
>> > but suspicious code.
>> >
>> > 2. Turn off checking for char* just like for void*. Rationale: C
>> > standard allows char* to alias a pointer of any type. Note that char*
>> > is special in this regard (strict aliasing rules).
>> >
>> > 3. Turn off annotations by default in mpi.h.
>> >
>> > Dmitri
>> >
>> > --
>> > main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
>> > (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr_at_[hidden]>*/
>> > _______________________________________________
>> > devel mailing list
>> > devel_at_[hidden]
>> > http://www.open-mpi.org/mailman/listinfo.cgi/devel
>>
>>
>> _______________________________________________
>> devel mailing list
>> devel_at_[hidden]
>> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>>
>
>
>
> --
> Paul H. Hargrove PHHargrove_at_[hidden]
> Future Technologies Group
> Computer and Data Sciences Department Tel: +1-510-495-2352
> Lawrence Berkeley National Laboratory Fax: +1-510-486-6900
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>
>
>
> _______________________________________________
> devel mailing list
> devel_at_[hidden]
> http://www.open-mpi.org/mailman/listinfo.cgi/devel
>

-- 
Paul H. Hargrove                          PHHargrove_at_[hidden]
Future Technologies Group
Computer and Data Sciences Department     Tel: +1-510-495-2352
Lawrence Berkeley National Laboratory     Fax: +1-510-486-6900