[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