[PATCH] D45212: [HIP] Let CUDA toolchain support HIP language mode

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 3 21:32:18 PDT 2018


rjmccall added a comment.

The other changes I see here seem reasonable, but please do split the patch.



================
Comment at: include/clang/Basic/Cuda.h:61
+  GFX900,
+  GFX902,
   LAST,
----------------
Does this actually have anything to do with HIP?  You have a lot of changes in this patch which seem to just be about supporting more GPU revisions.


================
Comment at: include/clang/Driver/Options.td:556
+def offload_archs : Joined<["--"], "offload-archs=">, Flags<[DriverOption]>,
+  HelpText<"List of offload architectures for CUDA/HIP/OpenMP (e.g. sm_35,gfx803).">;
 def no_cuda_gpu_arch_EQ : Joined<["--"], "no-cuda-gpu-arch=">, Flags<[DriverOption]>,
----------------
Do we absolutely need the non-CUDA-related aliases here?  We generally try to be good about namespacing extension-specific language options.

I understand that you're probably trying to maintain command-line compatibility with some existing toolchain, but if it's possible to avoid this, I would be much happier.


================
Comment at: include/clang/Driver/ToolChain.h:124
   mutable std::unique_ptr<Tool> Clang;
+  mutable std::unique_ptr<Tool> Backend;
   mutable std::unique_ptr<Tool> Assemble;
----------------
"Backend" is a really generic name for this thing that is probably hyper-specific to the CUDA translation model.


https://reviews.llvm.org/D45212





More information about the cfe-commits mailing list