yaxunl added inline comments.
================ Comment at: clang/include/clang/Basic/Cuda.h:109 + // Generic processor model is for testing only. + return A >= CudaArch::GFX600 && A <= CudaArch::GFX1035; } ---------------- can we use A < CudaArch::Generic instead? to avoid updating this line each time we add a new gfx arch. ================ Comment at: clang/include/clang/Driver/Options.td:1136 +def offload_EQ : CommaJoined<["--"], "offload=">, Flags<[NoXarchOption]>, + HelpText<"Specify comma-separated list of offloading targets.">; + ---------------- linjamaki wrote: > tra wrote: > > `comma-separated list of offloading targets.` is, unfortunately, somewhat > > ambiguous. > > Does it mean "how the offload will be done". I.e. HSA, OpenMP, SPIRV, CUDA? > > Or does it mean specific hardware we need to generate the code for? > > The code suggests it's a variant of the former, but the option description > > does not. > > > > E.g. `offload_arch_EQ ` also uses the term "offloading target" but with a > > different meaning. > > > I’m not sure how to rephrase the option description to be more clear. In the > [1] the `--offload` option is envisioned to be quite flexible/expressive - it > can take in target triples, offload kinds, processors, aliases for processor > sets, etc. > > FYI, I have imagined that the `--offload` option would take in explicit > offload kind and target triple combinations as the basis. For example, > something like this: > > > ``` > --offload=hip-amdgcn-amd-amdhsa,openmp-x86_64-pc-linux-gnu > ``` > > And top of that, there would be predefined strings/shortcuts/aliases that > expand to the basic form. For example, > `--offload=sm_70,openmp-host` could expand to something like: > > > ``` > --offload=cuda-nvptx64-nvidia-cuda,openmp-x86_64-pc-linux-gnu > -Xoffload=cuda-nvptx64-nvidia-cuda -march=sm_70 ... > > ``` > Then there is a feature as discussed in [1] that the offload kind can be > dropped if it can be inferred by other means (e.g. from `-x hip` option). > The description better matches the current implementation. By this patch, `--offload=` only supports specifying target triple for HIP and assumes default processor. The description should reflect that. In the future, as `--offload=` supports more values, the description may be updated. Also, `--offload=` is designed to be mutually exclusive with `--offload-arch=`. Probably we should check and diagnose that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110622/new/ https://reviews.llvm.org/D110622 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
