[PATCH] D120272: [CUDA] Add driver support for compiling CUDA with the new driver

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.


================
Comment at: clang/lib/Driver/Driver.cpp:4099-4102
+  for (auto &Arg : Args.getAllArgValues(options::OPT_offload_arch_EQ))
+    Archs.insert(getCanonicalArchString(C, Args, Arg, Kind));
+  for (auto &Arg : Args.getAllArgValues(options::OPT_no_offload_arch_EQ))
+    Archs.erase(getCanonicalArchString(C, Args, Arg, Kind));
----------------
yaxunl wrote:
> The final set depends on the order of -offload-arch and -no-offload-arch options, e.g. `--offload-arch=gfx906 --no-offload-arch=gfx906` and `--no-offload-arch=gfx906 --offload-arch=gfx906` is different. Also there is `--no-offload-arch=all` which removes all precedent --offload-arch= options.
I see, so we need to iterate the arguments in-order and insert or erase them as they are entered, I'll fix it.


================
Comment at: clang/lib/Driver/Driver.cpp:4107
+      Archs.insert(CudaArchToString(CudaArch::SM_35));
+    else if (Kind == Action::OFK_Cuda)
+      Archs.insert(CudaArchToString(CudaArch::GFX803));
----------------
yaxunl wrote:
> should this be HIP?
Whoops, haven't tested it with HIP yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272