Hi Jeff, On Thu, May 31, 2012 at 12:57 AM, Jeff Squyres <jsquy...@cisco.com> wrote: > I've reviewed the patch. Good stuff!
Thank you very much for the review. Answers to comments below.
Updated patch attached.
*** JMS What do the 3-argument forms of type_tag_for_datatype() do?
They aren't described in
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20120521/057992.html
My bad: that page has documentation for the previous version of clang
patch. Below is a relevant excerpt of current docs. Basically, the
third argument states additional instructions on how to compare types.
Please excuse me for a long prose copypaste.
+<h3 id="type_tag_for_datatype"><tt>type_tag_for_datatype(...)</tt></h3>
+
+<p>Clang supports annotating type tags of two forms.</p>
+
+<ul>
+<li><b>Type tag that is an expression containing a reference to some declared
+identifier.</b> Use <tt>__attribute__((type_tag_for_datatype(kind, type)))</tt>
+on a declaration with that identifier:
+
+<blockquote>
+<pre>
+extern struct mpi_datatype mpi_datatype_int
+ __attribute__(( type_tag_for_datatype(mpi,int) ));
+#define MPI_INT ((MPI_Datatype) &mpi_datatype_int)
+</pre>
+</blockquote></li>
+
+<li><b>Type tag that is an integral literal.</b> Introduce a <tt>static
+const</tt> variable with a corresponding initializer value and attach
+<tt>__attribute__((type_tag_for_datatype(kind, type)))</tt> on that
+declaration, for example:
+
+<blockquote>
+<pre>
+#define MPI_INT ((MPI_Datatype) 42)
+static const MPI_Datatype mpi_datatype_int
+ __attribute__(( type_tag_for_datatype(mpi,int) )) = 42
+</pre>
+</blockquote></li>
+</ul>
+
+<p>The attribute also accepts an optional third argument that determines how
+the expression is compared to the type tag. There are two supported flags:</p>
+
+<ul><li><tt>layout_compatible</tt> will cause types to be compared according to
+layout-compatibility rules (C++11 [class.mem] p 17, 18). This is
+implemented to support annotating types like <tt>MPI_DOUBLE_INT</tt>.
+
+<p>For example:</p>
+<blockquote>
+<pre>
+/* In mpi.h */
+struct internal_mpi_double_int { double d; int i; };
+extern struct mpi_datatype mpi_datatype_double_int
+ __attribute__(( type_tag_for_datatype(mpi, struct internal_mpi_double_int,
+ layout_compatible) ));
+
+#define MPI_DOUBLE_INT ((MPI_Datatype) &mpi_datatype_double_int)
+
+/* In user code */
+struct my_pair { double a; int b; };
+struct my_pair *buffer;
+MPI_Send(buffer, 1, MPI_DOUBLE_INT /*, ... */); // no warning
+
+struct my_int_pair { int a; int b; }
+struct my_int_pair *buffer2;
+MPI_Send(buffer2, 1, MPI_DOUBLE_INT /*, ... */); // warning: actual
buffer element
+ // type 'struct my_int_pair'
+ // doesn't match
specified MPI_Datatype
+</pre>
+</blockquote>
+</li>
+
+<li><tt>must_be_null</tt> specifies that the expression should be a null
+pointer constant, for example:
+
+<blockquote>
+<pre>
+/* In mpi.h */
+extern struct mpi_datatype mpi_datatype_null
+ __attribute__(( type_tag_for_datatype(mpi, void, must_be_null) ));
+
+#define MPI_DATATYPE_NULL ((MPI_Datatype) &mpi_datatype_null)
+
+/* In user code */
+MPI_Send(buffer, 1, MPI_DATATYPE_NULL /*, ... */); // warning:
MPI_DATATYPE_NULL
+ // was specified but buffer
+ // is not a null pointer
+</pre>
+</blockquote>
+</li>
+</ul>
*** JMS What happens if the argument type is (void*)? Does that match
the tag type? E.g.:
char recvbuf;
void *foo = &recvbuf;
MPI_Send(foo, 1, MPI_CHAR, ...)
If buffer has type void* then no warning is emitted.
*** JMS What if I deliberately want to receive into the wrong type
(and for some reason, I know it's ok)? Is casting ok enough to
defeat the tag testing? E.g.:
char recvbuf[100];
MPI_Recv((int*) recvbuf, 1, MPI_INT, ...);
Explicit casts are always honored. In this case buffer type is thought
to be int*, hence no warning is emitted.
*** JMS Random suggestion: how about CXX instead of CPP? CPP could
also mean C preprocessor.
Right, CXX is better. Done.
*** JMS Why doesn't ompi_mpi_byte have an attribute? And what does it
mean to not have an attribute? Will the type checking be silently
ignored at compile-time if there's no matching (MPI,foo)
type_tag_for_datatype? (same question applies to other instances
with no tags, like ompi_mpi_packed, but I won't bother repeating
it everywhere)
ompi_mpi_byte doesn't have an attr because I annotated only those
types that have C or C++ equivalents stated in MPI 2.2 specification.
If OpenMPI provides additional guarantees, for example that Fortran
types always have some known equivalent C types, we can annotate them,
too.
If a function is passed a type tag (that is, MPI_Datatype) that does
not have an annotation, then that function call is not checked. That
is, in MPI_Send(buf, 1, MPI_BYTE, ...) buf can be of any pointer type.
*** JMS I'm a bit confused as to why 2int got a tag, but the others
did not. We do have C equivalents for all types -- do you need a
listing of the configure results where we figured out all the C
equivalents? (i.e., I can look them up for you -- our configury
is a bit... deep :-) )
I did not annotate them because those are Fortran types. If there are
some known C equivalents in OpenMPI, I will happily add them. But
please note that if these equivalents are discovered during configure,
we can not hardcode them into mpi.h.in. Some autoconf macros will be
needed probably.
*** JMS Shouldn't alltoallw have
+ OMPI_ATTR_POINTER_WITH_TYPE_TAG(1,4)
+ OMPI_ATTR_POINTER_WITH_TYPE_TAG(5,8);
*** JMS I ask because you have these on PMPI_Alltoallw... but I'm not
quite sure how that works with arrays of datatypes...
You are correct, this diagnostic doesn't work with arrays of MPI_Datatypes.
*** JMS Per my question on MPI_Alltoallw, I'm not quite sure how these
tags work with arrays of datatypes...?
I removed the incorrect attribute on PMPI_Alltoallw.
*** What happens if we're compiling C and std::complex<foo> isn't
defined? I see that <complex> is only defined above #if __cplusplus.
Then OMPI_ATTR_TYPE_TAG_CXX(type) is defined to be empty and these
type tags are not checked.
For complex types there is one important thing to have in mind. IIRC
(but I may be wrong), GCC and Clang guarantee that C and C++ complex
types have the same memory layout, so they can be sent using any (C or
C++) type tag. But we should better ask on Clang and GCC mailing list
about this.
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 <griboz...@gmail.com>*/
ompi-v3.patch
Description: Binary data
