[SYCL] [Driver] Add offload-arch support for SYCL offloading #15624
Conversation
…am/llvm into offload-arch-aot
srividya-sundaram
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
srividya-sundaram
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
srividya-sundaram
had a problem deploying
to
WindowsCILock
GitHub Actions
Error
srividya-sundaram
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| } | ||
| // If the user specified --offload-arch, deduce the offloading | ||
| // target triple(s) from the set of architecture(s). | ||
| // Create a toolchain for each valid triple. |
There was a problem hiding this comment.
Maybe add some additional comment about why IsHIP and IsCuda is not supported here.
| false)) { | ||
| Diag(clang::diag::err_drv_sycl_offload_arch_new_driver); | ||
| return; | ||
| } |
There was a problem hiding this comment.
if condition and comment are indented 2 spaces too far.
| Arch); | ||
| } else if (IsSYCLSupportedIntelGPUArch(StringToOffloadArchSYCL(Arch))) { | ||
| StringRef IntelGPUArch; | ||
| IntelGPUArch = mapIntelGPUArchName(Arch).data(); |
There was a problem hiding this comment.
The mapping here is different than the other arch values - perhaps add a comment explaining what it is doing.
| } | ||
| // If the set is empty then we failed to find a native architecture. | ||
| if (Archs.empty()) { | ||
| Diag(clang::diag::err_drv_invalid_sycl_target) << "native"; |
There was a problem hiding this comment.
This error doesn't make any sense to the end user: SYCL target is invalid: 'native'. As we are validating --offload-arch values, something that reflects that usage would make more sense. Can you take advantage of the existing err_drv_bad_offload_arch_combo diagnostic?
Also, please add a test for this case.
| // Check if the given Arch value is a valid SYCL supported Intel CPU. | ||
| static inline bool IsSYCLSupportedIntelCPUArch(SYCLSupportedOffloadArchs Arch) { | ||
| return Arch >= SYCLSupportedOffloadArchs::SKYLAKEAVX512 && | ||
| Arch < SYCLSupportedOffloadArchs::BDW; |
There was a problem hiding this comment.
For consistency with the others:
| Arch < SYCLSupportedOffloadArchs::BDW; | |
| Arch <= SYCLSupportedOffloadArchs::GRANITERAPIDS; |
srividya-sundaram
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
Implement
--offload-archoption to enable SYCL offloading to Intel CPUs, Intel GPUs, NVidia and AMD GPUs.