[PATCH] D100609: [Offload][OpenMP][CUDA] Allow fembed-bitcode for device offload

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4442-4446
+      std::string CPU = getCPUName(Args, Triple, /*FromAs*/ false);
+      if (!CPU.empty()) {
+        CmdArgs.push_back("-target-cpu");
+        CmdArgs.push_back(Args.MakeArgString(CPU));
+      }
----------------
tra wrote:
> This duplicates the same code a bit further down in the function. I think you should just set `-target-cpu` for everyone before diving into `if(embedBitcodeInObject)`.
Fair. I'll update it.


================
Comment at: clang/test/Driver/embed-bitcode-nvptx.cu:1
+// RUN: %clang -Xclang -triple -Xclang nvptx64 -S -Xclang -target-feature -Xclang +ptx70 -fembed-bitcode=all --cuda-device-only -nocudalib -nocudainc %s -o - | FileCheck %s
+// REQUIRES: nvptx-registered-target
----------------
tra wrote:
> This command line looks extremely odd to me.
> If you are compiling with `--cuda-device-only`, then clang should've already set the right triple and the features.
> 
> Could you tell me more about what is the intent of the compilation and why you use this particular set of options?
> I.e. why not just do `clang -x cuda --offload-arch=sm_70 --cuda-device-only -nocudalib -nocudainc`.
> 
> Could you tell me more about what is the intent of the compilation and why you use this particular set of options?

because I never compiled cuda really ;)

I'll go with your options.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100609/new/

https://reviews.llvm.org/D100609