[PATCH] D15534: [CUDA] renamed cuda_runtime.h wrapper to __clang_cuda_runtime_wrapper.h

Chandler Carruth via cfe-commits cfe-commits at lists.llvm.org
Tue Dec 15 16:55:50 PST 2015


chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

The substance of the patch LGTM. My nit picking is just on the wording of the comment. =] Submit whenever.


================
Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:25-26
@@ +24,4 @@
+/*
+ * WARNING: This header is intended to be directly -include'd by
+ * compiler and is not supposed to be included by users.
+ *
----------------
"by compiler" -> "by the compiler"

================
Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:28-29
@@ +27,4 @@
+ *
+ * CUDA headers are implemented in a way that currently makes it
+ * impossible to use them by clang directly. They present different
+ * view of cuda-supplied functions depending on where in nvcc's
----------------
You say above that they'll be included by clang directly?

I think instead of "impossible to use them by clang directly" you want to say something more along the lines of "impossible for user code to #include directly when compiling with clang".

================
Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:30-34
@@ +29,7 @@
+ * impossible to use them by clang directly. They present different
+ * view of cuda-supplied functions depending on where in nvcc's
+ * compilation pipeline the headers are included from. Neither of
+ * these modes provides function definitions with correct attributes,
+ * so we have to abuse preprocessor in order to shape CUDA headers
+ * into something clang can use.
+ *
----------------
I would consistently capitalize "CUDA" when not talking about a particular header like cuda_runtime.h, and "Clang" and "NVCC" unless talking about running a command.

Some other nits:

"included from" -> "included"

"so we have to abuse preprocessor in order to" -> "so we use the preprocessor to"

"shape CUDA headers into something" -> "force the headers into a form that"


http://reviews.llvm.org/D15534





More information about the cfe-commits mailing list