[WIP][SYCL][Driver] Initial support to enable --offload-arch option for SYCL. #3

srividya-sundaram · 2025-07-01T16:53:16Z

This patch is an attempt to have a single target triple string (spirv64-intel-sycl) to describe all the Intel devices (currently GPUs and CPUs) and the corresponding offloading device architecture is specified by the --offload-arch command-line argument, for the AOT compilation flow.

Example:

clang -fsycl --offload-arch=bmg_g21_gpu syclfile.cpp 
clang -fsycl --offload-arch=graniterapids_cpu syclfile.cpp

would imply spirv64-intel-sycl as target triple string for both the Intel CPU and GPU.

For JIT compilation, the default SYCL target triple string would be spirv-unknown-unknown

AOT flow : spirv32/64-intel-sycl
JIT flow:    spirv32/64-unknown-unknown

To Do:
Implement target macro additions to SYCL device compilation flow.
Fix default SYCL target triple string code for JIT compilation.

mdtoguchi · 2025-07-02T23:02:07Z

clang/include/clang/Driver/Driver.h

+  /// added to the host compilation step.
+  void addSYCLTargetMacro(const llvm::opt::ArgList &Args,
+    StringRef Macro) const {
+    SYCLTargetMacro.push_back(Args.MakeArgString(Macro));


I realize that this patch doesn't currently have the macro addition steps - but this may be a good opportunity to reduce macro duplication that is added to the host compilation by only adding unique macro values to the SYCLTargetMacro array.

mdtoguchi · 2025-07-02T23:04:33Z

clang/lib/Driver/Driver.cpp


+
+static std::optional<llvm::Triple>
+getINTELOffloadTargetTriple(const Driver &D, const ArgList &Args,


The name doesn't seem to fit the triple being returned. The value here is a spirv value, so is this more of a 'default SYCL JIT' triple?

asudarsa · 2025-07-03T23:42:15Z

clang/include/clang/Basic/DiagnosticDriverKinds.td

    : Warning<"OpenACC directives will result in no runtime behavior; use "
              "-fclangir to enable runtime effect">,
      InGroup<SourceUsesOpenACC>;
+def err_drv_sycl_offload_arch_missing_value :


Hi @srividya-sundaram

I will take a look in a bit.

Thanks

asudarsa · 2025-07-04T00:17:02Z

clang/include/clang/Driver/Driver.h

+  /// Vector of Macros that need to be added to the Host compilation in a
+  /// SYCL based offloading scenario.  These macros are gathered during
+  /// construction of the device compilations.
+  mutable std::vector<std::string> SYCLTargetMacro;


Suggested change

mutable std::vector<std::string> SYCLTargetMacro;

mutable std::vector<std::string> SYCLTargetMacros;

asudarsa · 2025-07-04T00:27:44Z

clang/lib/Driver/Driver.cpp

+    return llvm::Triple(HostTriple.isArch64Bit() ? "spirv64-intel-sycl"
+                                                 : "spirv32-intel-sycl");
+  }
+  return std::nullopt;


Should we emit something if user specifies -offload= for SYCL offloading? Or atleast add an assert?

Currently we emit a diagnostic for empty --offload-arch

This question was about -offload. What will happen if user says '-fsycl -offload=abc'?

asudarsa · 2025-07-04T00:36:40Z

clang/lib/Driver/Driver.cpp

-    TargetTriple.setVendor(llvm::Triple::UnknownVendor);
-    TargetTriple.setOS(llvm::Triple::UnknownOS);
+    TargetTriple.setVendor(llvm::Triple::Intel);
+    TargetTriple.setOS(llvm::Triple::SYCL);


Hmm..This is a bit confusing. Is it correct to set OS as SYCL? Thanks

We don't need to set it here as this will be used to set the target triple string for the JIT flow (spirv64-unknown-unknown). I need to do some refactor for the JIT flow.

So, it looks like spirv64-unknown-unknown is used for AOT and spirv64-intel-sycl is being used for JIT? Why use a different triple? Why not just rely on arch=<val> in the package and have the clang-linker-wrapper determine AOT based on that? The triple is used for the device compilation which shouldn't need to know if the generated LLVM-IR file is going to be used for JIT or AOT.

I agree. clang-linker-wrapper can determine AOT/JIT based on arch=.
Also, just a nit. I suppose you meant to write: "So, it looks like spirv64-unknown-unknown is used for JIT and spirv64-intel-sycl is being used for AOT?"

Thanks

Right - I must have read the testing wrong. For some reason when I was looking at the testing, I was associating the target and triple backwards. Regardless, it looks like we are in agreement on using a single triple and having the clang-linker-wrapper understand the AOT target based on the arch= value.

@mdtoguchi
Why not just rely on arch=<val> in the package and have the clang-linker-wrapper determine AOT based on that
This is still the case.
The proposal to have spirv64-intel-sycl as the target triple string for AOT is to have a more descriptive string that has info about the vendor and OS as opposed to having 'unknown' for both. This also matches what is done for CUDA and HIP.
This also updates triple values in clang-offload-packager call and other places where -triple is passed:
"--image=file=test.bc,triple=spirv64-intel-sycl,arch=bmg_g21_gpu,kind=sycl"

We could probably drop 'sycl' and use spirv64-intel-unknown which is already used for OpenMP AOT.

I'm wondering why we aren't aligning with OpenMP Intel AOT, CUDA, and HIP. I don't see any issues with using a more descriptive target triple string for SYCL AOT to Intel targets.

There should be no issues with moving to a more descriptive triple, but the usage of the triple during the device compilation isn't Intel specific. It is just generating generic IR. We were already using the spirv64-unknown-unknown for JIT, so I don't see why we should move away from that for AOT. We are able to use the same generated device binary with spirv64-unknown-unknown for JIT and AOT. triple=spirv64-unknown-unknown,arch=bmg_g21_gpu,kind=sycl should be plenty of information for the clang-linker-wrapper to decipher what needs to be done with the packaged binary.

An LLVM target triple is a string that describes the target architecture, operating system, and vendor for which LLVM is compiling code.

For SYCL AOT to Intel GPUs or CPUS, a target triple string such as spirv64-intel-sycl/unknown describes that we are compiling code for an Intel target compatible with SPIRV.

For JIT, the generated SPIRV is not tied to Intel targets, so it seems reasonable to have 'unknown' for the target vendor and OS.

AFAIK, even with SYCL offloading to CUDA targets, the generated LLVM IR is generic and the NVPTX Back End adds additional CUDA specific libraries.

We could still generate generic SPIV/LLVM IR and yet have a target triple string that describes for which LLVM is compiling code.

Thanks @srividya-sundaram. I'm OK with using spirv64-intel-unknown for AOT.

asudarsa · 2025-07-07T19:14:18Z

Hi @srividya-sundaram and @mdtoguchi

In llvm#146594 which was just merged, they use --offload-targets. Should we also use that instead of --offload-arch?

Thanks

srividya-sundaram · 2025-07-07T19:37:08Z

Hi @srividya-sundaram and @mdtoguchi

In llvm#146594 which was just merged, they use --offload-targets. Should we also use that instead of --offload-arch?

Thanks

@asudarsa
The HelpText in his patch for --offload-targets reads : Specify a list of target architectures to use for offloading.
Although, the example usage in the tests seems to use the target triple string as the value.
--offload-targets=amdgcn-amd-amdhsa
If we need to pass info about the offloading device architecture, do we still need to pass --offload-arch?
Example:
clang -fsycl --offload-targets=spirv64 --offload-arch=pvc syclfile.cpp
This is not clear to me from his patch.

asudarsa · 2025-07-07T23:44:22Z

Hi @srividya-sundaram and @mdtoguchi
In llvm#146594 which was just merged, they use --offload-targets. Should we also use that instead of --offload-arch?
Thanks

@asudarsa The HelpText in his patch for --offload-targets reads : Specify a list of target architectures to use for offloading. Although, the example usage in the tests seems to use the target triple string as the value. --offload-targets=amdgcn-amd-amdhsa If we need to pass info about the offloading device architecture, do we still need to pass --offload-arch? Example: clang -fsycl --offload-targets=spirv64 --offload-arch=pvc syclfile.cpp This is not clear to me from his patch.

AH. I understand now. In the upstream PR, they are trying to simplify the way we specify offload targets by using two options instead of one - offload-targets and offload-arch. For Intel targets, I think --offload-targets is easy to decipher. spirv64 for JIT and spirv64-intel-sycl for AOT. So user need not specify.

Thanks

asudarsa · 2025-07-07T23:48:42Z

clang/include/clang/Basic/DiagnosticDriverKinds.td

    : Warning<"OpenACC directives will result in no runtime behavior; use "
              "-fclangir to enable runtime effect">,
      InGroup<SourceUsesOpenACC>;
+def err_drv_sycl_offload_arch_missing_value :


Why are these warnings SYCL specific?

Thanks

asudarsa · 2025-07-07T23:52:25Z

clang/include/clang/Basic/OffloadArch.h

           // public one.
  // Intel CPUs
-  GRANITERAPIDS,
+  GRANITERAPIDS_CPU,


Is this the format we will follow going forward? Processor name + "_" + CPU/GPU? I am ok with it.
Adding @bader for comment.

Thanks

[SYCL][Driver] Initial support to enable --offload-arch option for SYCL.

ef0ddb1

srividya-sundaram marked this pull request as draft July 1, 2025 16:55

srividya-sundaram added 2 commits July 1, 2025 14:28

Add triple deduction logic for Intel targets.

c96d672

Add toolchain creation.

0012d95

srividya-sundaram requested a review from mdtoguchi July 2, 2025 19:57

mdtoguchi reviewed Jul 2, 2025

View reviewed changes

srividya-sundaram added 2 commits July 3, 2025 10:38

Add Intel specific target triple string.

ffb828a

Add tests.

9cae196

srividya-sundaram changed the title ~~[SYCL][Driver] Initial support to enable --offload-arch option for SYCL.~~ [WIP][SYCL][Driver] Initial support to enable --offload-arch option for SYCL. Jul 3, 2025

asudarsa reviewed Jul 3, 2025

View reviewed changes

asudarsa reviewed Jul 4, 2025

View reviewed changes

asudarsa reviewed Jul 7, 2025

View reviewed changes



		static std::optional<llvm::Triple>
		getINTELOffloadTargetTriple(const Driver &D, const ArgList &Args,

	mutable std::vector<std::string> SYCLTargetMacro;
	mutable std::vector<std::string> SYCLTargetMacros;

[WIP][SYCL][Driver] Initial support to enable --offload-arch option for SYCL. #3

Are you sure you want to change the base?

[WIP][SYCL][Driver] Initial support to enable --offload-arch option for SYCL. #3

Uh oh!

Conversation

srividya-sundaram commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

asudarsa commented Jul 7, 2025

Uh oh!

srividya-sundaram commented Jul 7, 2025

Uh oh!

asudarsa commented Jul 7, 2025

Uh oh!

asudarsa Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

srividya-sundaram commented Jul 1, 2025 •

edited

Loading

asudarsa Jul 7, 2025 •

edited

Loading