On Apr 13, 2011, at 12:47 PM, Rolf vandeVaart wrote: > An initial implementation can be viewed at: > https://bitbucket.org/rolfv/ompi-trunk-cuda-3
Random comments on the code... 1. I see changes like this: mca_btl_sm_la_LIBADD += \ $(top_ompi_builddir)/ompi/mca/common/cuda/libmca_common_cuda.la But I don't see any common/cuda function calls in the SM BTL. Why the link? 2. I see a new "opal_output(-1,.." in btl_tcp.c. If it's a developer-only opal_output, it should be compiled out by default. 3. In ompi_free_list.c, you call posix_memalign(), protected by OMPI_CUDA_SUPPORT. Does posix_memalign() exist in Windows, and/or does OMPI_CUDE_SUPPORT exclude Windows? 4. Along with what Pasha said, it seems odd to put a CUDA-specific value in mpool.h (MCA_MPOOL_FLAGS_CUDA_MEM). --> Some explanation is required for this comment. My gut reaction is to have portable code in OMPI, such that we can support multiple registration-necessary memory pools. That being said, NVIDIA is the first mover here; is there any other interest in ever wanting to be able to register other kinds of memory, too? Or should we let NVIDIA do it this way on the assumption that it will be years before anyone *might* want to use some other multi-memory-registration scheme? I can see both sides of the coin here... 5. In pml_ob1_sendreq.h, you set size to 0 if OMPI_CUDA_SUPPORT. That means that any OMPI compiled with CUDA support will have this value -- regardless if they're using accelerators or not. Shouldn't there be a compile-time check AND a run-time check for this kind of thing? 6. Instead of #if OMPI_CUDA_SUPPORT to select which memcpy to use, why not have a different opal memcopy MCA component for the cuda memcpy? Would that make a bunch of convertor #if OMPI_CUDA_SUPPORT's go away? 7. Using the name OMPI_* down in OPAL doesn't seem like a good idea (there's still some OMPI_* preprocessor names down in there that haven't yet been converted to OPAL_*, but adding new OMPI_* names down there doesn't seem to be a good idea). -- Jeff Squyres jsquy...@cisco.com For corporate legal information go to: http://www.cisco.com/web/about/doing_business/legal/cri/
