[cfe-commits] [PATCH] Compile-time MPI_Datatype checking, type arguments in attributes

[Hal Finkel]

Dmitri,

First, I think that this is a great idea!

Some quick comments:

1. How much of this is realy MPI specific? Maybe I'm wrong, but the
general idea is to add type checking for APIs that take void* along with
some kind of tag indicating the actual data type. MPI may be a primary
use case, but that general situation seems not uncommon. I wonder if we
could take 'mpi' out of the syntax and replace it with some kind of
domain tag. For example, instead of mpi_typed_arg(1,3) we might name it
pwt_arg("mpi",1,3) [pwt = pointer with tag] and use pwt_type("mpi",int).

2. Supporting MPICH-derived implementations will be important. Maybe we
could add a pragma to allow the definition of integer-type tags. [I may
be slightly biased here ;) -- a bunch of the MPICH developers work down
the hall from me -- but it would be great to have something like this
installed for our users here]. Especially if we make this slightly more
generic, supporting integer/enum tags will be necessary.

 -Hal

On Sat, 2012-02-11 at 22:09 +0200, Dmitri Gribenko wrote:
> Hello,
> 
> === Objective ===
> Implement a set of annotations for MPI libraries and a diagnostic for
> clang that checks that specified buffer type matches the passed
> MPI_Datatype.
> 
> === Background ===
> Many MPI functions take a void* `buffer' and an MPI_Datatype (like
> MPI_INT, MPI_LONG etc) that describes how to send/receive that buffer.
>  For example,
> 
> int MPI_Send(void *buf, int count, MPI_Datatype datatype, int dest,
>     int tag, MPI_Comm comm);
> 
> Users call it like this:
> int *int_buf = ...;
> MPI_Send(int_buf, 1, MPI_INT, ...);
> 
> Thus, user is on its own to ensure that buffer type matches the MPI_Datatype.
> 
> === Annotation design ===
> I decided to annotate MPI functions that have buffer arguments and
> declarations of built-in `MPI_Datatype's.  Functions are easy: we use
> mpi_typed_arg(buffer-arg-index, type-arg-index) attribute:
> 
> int MPI_Send(void *buf, int count, MPI_Datatype datatype, ...etc...)
>     __attribute__(( mpi_typed_arg(1,3) ));
> 
> `MPI_Datatype's are trickier.  From the grammar of GNU attributes in
> lib/Parse/ParseDecl.cpp we see that there is no easy way to pass a
> type argument to an attribute.  For example, this is not currently
> parseable by clang:
> 
> #define MPI_INT ((MPI_Datatype) &ompi_mpi_int)
> extern struct ompi_predefined_datatype_t ompi_mpi_int
>         __attribute__(( mpi_datatype(int) ));
> 
> Matching C datatype for MPI_DATATYPE_NULL is void.
> 
> #define MPI_DATATYPE_NULL ((MPI_Datatype) &ompi_mpi_int)
> extern struct ompi_predefined_datatype_t ompi_mpi_void
>         __attribute__(( mpi_datatype(void) ));
> 
> While checking a function call we recognize mpi_datatype attribute if
> datatype argument is either:
> * an OpenMPI-style expression `((MPI_Datatype) &ident)';
> * a user-specified type `ident';
> where ident has mpi_datatype attribute.
> 
> We emit a diagnostic in 2 cases.
> Case 1.  (Any type except MPI_DATATYPE_NULL)  All of the following is true:
> * buffer is not of `void *' type;
> * buffer's element type doesn't match specified MPI_Datatype.
> 
> Case 2.  (MPI_DATATYPE_NULL)  All of the following is true:
> * buffer is not of `void *' type;
> * buffer is not a null pointer constant;
> * mpi_datatype(void) was specified.
> 
> === Clang modifications ===
> We need to allow arbitrary types as attribute arguments.  If I
> understand everything correctly, GNU attribute grammar becomes
> ambiguous in that case, so attribute argument parsing will be driven
> by attribute name.  Fortunately, that is compatible with C++11
> attributes where attribute argument list is just a sequence of tokens
> with balanced delimiters.
> 
> I added AttributeList::TypeArg member, the required AttributeList
> constructor and factory method.  In Attr.td a new field,
> HasTypeArgument, was added and a tablegen pass was implemented.  The
> parser uses the generated table to decide if it should parse a list of
> expressions or a type name.
> 
> The check itself is called from Sema::CheckFunctionCall, which seems
> to be designed for similar purposes.
> 
> === Compatibility ===
> The annotation-based approach is compatible with OpenMPI and LAM MPI.
> I already have a prototype patch for OpenMPI.
> 
> MPICH is not compatible without refactoring their mpi.h header (they
> have MPI_XXX macros defined to magic numbers).
> 
> === Miscellaneous ===
> There is an `iboutletcollection' attribute that takes a type argument.
>  It would be good if this implementation of type arguments could be
> used for that, too.
> 
> Please see attached patch.  Parsing diagnostics are not very good (see
> testcase), advice is welcome.
> 
> Dmitri
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-- 
Hal Finkel
Postdoctoral Appointee
Leadership Computing Facility
Argonne National Laboratory