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: Dmitri Gribenko (gribozavr_at_[hidden])
Date: 2012-10-31 14:19:56


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]>*/