[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:4132-4134
+      Archs.insert(CudaArchToString(CudaArch::SM_35));
+    else if (Kind == Action::OFK_HIP)
+      Archs.insert(CudaArchToString(CudaArch::GFX803));
----------------
tra wrote:
> JonChesterfield wrote:
> > tra wrote:
> > > If we do not have constants for the default CUDA/HIP arch yet, we should probably add them and use them here.
> > Defaulting hip to gfx803 is unlikely to be helpful. It won't run on other architectures, i.e. there's no conservative default that will run on most things. I guess that's an existing quirk of the hip toolchain?
> I agree that there's no "safe" choice of GPU target. It applies to CUDA, as well, at least for GPU binaries.
> That said, we still want `clang -c foo.cu` to work.
> 
> For CUDA we use the oldest variant still supported by the vendor. It produces PTX assembly which we embed along with the GPU binary. 
> That PTX is valid for newer GPU archtectures and CUDA runtime will be able to compile it for the GPU the app happens to run on. It's not ideal, but it works.
> 
> While for AMDGPU we do not have such forward compatibility mode as we have with CUDA, being able to compile for *something* by default is still useful, IMO.
> 
> 
I just copied GFX803 because that's what the previous driver used. I agree we should just default to something, maybe in the AMD case we can issue a warning telling them to use the flag to properly specify it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120272