Hahnfeld added a comment.

Only commenting on parts that I'm a bit familiar with. In general, does it make 
sense to split this patch, are there different "stages" of support? Like 1) 
being able to compile an empty file, 2) generate optimized code, 3) allow using 
math functions?



================
Comment at: lib/Driver/ToolChains/Cuda.cpp:403-415
+  // If Verbose, save input for llc in /tmp and print all symbols
+  if (Args.hasArg(options::OPT_v)) {
+    ArgStringList nmArgs;
+    nmArgs.push_back(ResultingBitcodeF);
+    nmArgs.push_back("-debug-syms");
+    const char *nmExec = Args.MakeArgString(C.getDriver().Dir + "/llvm-nm");
+    C.addCommand(llvm::make_unique<Command>(JA, *this, nmExec, nmArgs, 
Inputs));
----------------
This never gets cleaned up!


================
Comment at: lib/Driver/ToolChains/Cuda.cpp:531-534
+  SmallString<256> OutputFileName(Output.getFilename());
+  if (JA.isOffloading(Action::OFK_OpenMP))
+    llvm::sys::path::replace_extension(OutputFileName, "cubin");
+  CmdArgs.push_back(Args.MakeArgString(OutputFileName));
----------------
That is already done in `TC.getInputFilename(Output)` (since rC318763), the 
same function call that you are removing here...


================
Comment at: lib/Driver/ToolChains/Cuda.cpp:639-640
+    CmdArgs2.push_back(Args.MakeArgString(Output.getFilename()));
+    const char *Exec2 =
+        Args.MakeArgString(C.getDriver().Dir + "/clang-fixup-fatbin");
+    C.addCommand(
----------------
`clang-fixup-fatbin` is not upstreamed and won't work. Sounds like a horrible 
name btw...


================
Comment at: lib/Driver/ToolChains/Cuda.cpp:788-793
+  // Do not add -link-cuda-bitcode or ptx42 features if gfx
+  for (Arg *A : DriverArgs)
+    if (A->getOption().matches(options::OPT_cuda_gpu_arch_EQ) &&
+        StringRef(A->getValue()).startswith("gfx"))
+      return;
+
----------------
You should use `GpuArch` which comes from `DriverArgs.getLastArgValue`: The 
last `-march` overrides previous arguments.


https://reviews.llvm.org/D42800



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to