[PATCH] D16638: [CUDA] Added device-side std::{malloc/free}

[Justin Lebar via cfe-commits]

jlebar added inline comments.

================
Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:215
@@ +214,3 @@
+// Device-side CUDA system calls.
+// http://docs.nvidia.com/cuda/ptx-writers-guide-to-interoperability/index.html#system-calls
+
----------------
It seems that only vprintf, free, malloc, and __assertfail are syscalls?  If so please rejigger this comment so that it's clear what it is and isn't covering.

================
Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:232
@@ +231,3 @@
+// Clang will convert printf into vprintf, but we still need
+// device-side declaration for it.
+__device__ int printf(const char *, ...);
----------------
I'd prefer to keep the information from the previous comment: This declaration is there for type-safety, not because things will fail to compile if it is removed.

Otherwise someone may look at this, delete this definition, see that everything still works, and conclude that they can safely remove this line.

================
Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:234
@@ +233,3 @@
+__device__ int printf(const char *, ...);
+}
+
----------------
Please put "// extern "C"

================
Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:236
@@ +235,3 @@
+
+// We also need device-side std::malloc and std::free
+namespace std {
----------------
Nit: Period

================
Comment at: lib/Headers/__clang_cuda_runtime_wrapper.h:242
@@ +241,3 @@
+}
+}
+
----------------
Please add "//namespace std"


http://reviews.llvm.org/D16638