[PATCH] D44984: [HIP] Add hip file type and codegen for kernel launching

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 13 00:07:04 PDT 2018


rjmccall added inline comments.


================
Comment at: lib/CodeGen/CGCUDANV.cpp:98
+std::string CGNVCUDARuntime::addPrefixToName(CodeGenModule &CGM,
+                                             std::string FuncName) const {
+  if (CGM.getLangOpts().HIP)
----------------
Can you take these as StringRefs or Twines?




================
Comment at: lib/CodeGen/CGCUDANV.cpp:104
+std::string CGNVCUDARuntime::addPrefixToNameBar(CodeGenModule &CGM,
+                                                std::string FuncName) const {
+  if (CGM.getLangOpts().HIP)
----------------
I think "addUnderscoredPrefixToName" would be better.


================
Comment at: lib/CodeGen/CGCUDANV.cpp:134
 llvm::Constant *CGNVCUDARuntime::getLaunchFn() const {
   // cudaError_t cudaLaunch(char *)
+  if (CGM.getLangOpts().HIP)
----------------
Please move this comment down into the else clause (and terminate it with a semicolon) and add your own declaration comment in your clause.


================
Comment at: lib/Frontend/CompilerInvocation.cpp:2109
+      Opts.HIP = true;
+  }
+
----------------
Why is this done here?  We infer the language mode from the input kind somewhere else.


https://reviews.llvm.org/D44984





More information about the cfe-commits mailing list