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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked an inline comment as done.
yaxunl added inline comments.


================
Comment at: include/clang/Basic/Cuda.h:61
+  GFX900,
+  GFX902,
   LAST,
----------------
rjmccall wrote:
> 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.
This patch not only adds support of HIP language mode but also adds support of amdgpu to CUDA toolchain.

Currently HIP extension is only supported by amdgpu although in the future it may be supported by other targets.


================
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]>,
----------------
rjmccall wrote:
> 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.
There were discussions about a uniform clang option for offloading sub-arcs 

http://lists.llvm.org/pipermail/llvm-dev/2017-February/109896.html 

the consensus seem to be -offload-arch or -offload-archs.

This patch attempts to make that transition to use this new option.

We can separate this change to a different patch though.


================
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;
----------------
rjmccall wrote:
> "Backend" is a really generic name for this thing that is probably hyper-specific to the CUDA translation model.
Agree. This tool actually links bunch of bitcode libraries (so called device libraries). For non-GPU targets, this is usually unnecessary since they support ISA-level linking. However most GPU targets do not support that, therefore they need this stage.

How about renaming it as BitcodeLink?


https://reviews.llvm.org/D45212