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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
