[PATCH] D44984: [HIP] Add hip input kind and codegen for kernel launching

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.


================
Comment at: lib/CodeGen/CGCUDANV.cpp:51-52
   llvm::Constant *getLaunchFn() const;
+  std::string addPrefixToName(CodeGenModule &CGM, StringRef FuncName) const;
+  std::string addUnderscoredPrefixToName(CodeGenModule &CGM,
+                                         StringRef FuncName) const;
----------------
`const CodeGenModule &CGM`


================
Comment at: lib/Frontend/InitPreprocessor.cpp:466-467
     Builder.defineMacro("__ASSEMBLER__");
   if (LangOpts.CUDA)
     Builder.defineMacro("__CUDA__");
+  if (LangOpts.HIP)
----------------
Is `__CUDA__` supposed to be set during HIP compilation?  My guess is that `__HIP__` and `__CUDA__` should be mutually exclusive. 
You do set LangOpts.CUDA during HIP compilation, so this should be changed to `if (CUDA && ! HIP)`


================
Comment at: lib/Sema/SemaDecl.cpp:9051-9054
+    if (II &&
+        ((getLangOpts().HIP && II->isStr("hipConfigureCall")) ||
+         (!getLangOpts().HIP && II->isStr("cudaConfigureCall"))) &&
+        !NewFD->isInvalidDecl() &&
----------------
This would be somewhat easier to read:
```
if (II && II->isStr(getLangOpts().HIP ? "hipConfigureCall" : "cudaConfigureCall") && ...

```


================
Comment at: test/CodeGenCUDA/device-stub.cu:2-8
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
+// RUN:   -fcuda-include-gpubinary %t -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s \
+// RUN:   -fcuda-include-gpubinary %t -o -  -DNOGLOBALS \
 // RUN:   | FileCheck %s -check-prefix=NOGLOBALS
-// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - | FileCheck %s -check-prefix=NOGPUBIN
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm %s -o - \
+// RUN:   | FileCheck %s -check-prefix=NOGPUBIN
----------------
The changes in this file do not seem to have anything related to the code changes in this patch.
Did you intend to add some HIP tests here?


https://reviews.llvm.org/D44984