[Driver][SYCL] Add initial SYCL offload compilation support #117268
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…lvm#107493) Introduces the SYCL based toolchain and initial toolchain construction when using the '-fsycl' option. This option will enable SYCL based offloading, creating a SPIR-V based IR file packaged into the compiled host object. This includes early support for creating the host/device object using the new offloading model. The device object is created using the spir64-unknown-unknown target triple. New/Updated Options: -fsycl Enables SYCL offloading for host and device -fsycl-device-only Enables device only compilation for SYCL -fsycl-host-only Enables host only compilation for SYCL RFC Reference: https://discourse.llvm.org/t/rfc-sycl-driver-enhancements/74092 Was reverted due to buildbot issues. Contains additional fixes to pull in the SYCL header dependencies to other toolchains.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Michael Toguchi (mdtoguchi) Changes…(#107493) Introduces the SYCL based toolchain and initial toolchain construction when using the '-fsycl' option. This option will enable SYCL based offloading, creating a SPIR-V based IR file packaged into the compiled host object. This includes early support for creating the host/device object using the new offloading model. The device object is created using the spir64-unknown-unknown target triple. New/Updated Options: RFC Reference: Was reverted due to buildbot issues. Contains additional fixes to pull in the SYCL header dependencies to other toolchains. Patch is 46.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117268.diff 23 Files Affected:
diff --git a/clang/include/clang/Driver/Action.h b/clang/include/clang/Driver/Action.h
index 04fa8b01b418f8..feeabae89d6b1c 100644
--- a/clang/include/clang/Driver/Action.h
+++ b/clang/include/clang/Driver/Action.h
@@ -94,6 +94,7 @@ class Action {
OFK_Cuda = 0x02,
OFK_OpenMP = 0x04,
OFK_HIP = 0x08,
+ OFK_SYCL = 0x10,
};
static const char *getClassName(ActionClass AC);
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 5167c3c39e315a..1da3c9a1118ca8 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -182,7 +182,8 @@ def opencl_Group : OptionGroup<"<opencl group>">, Group<f_Group>,
DocName<"OpenCL options">;
def sycl_Group : OptionGroup<"<SYCL group>">, Group<f_Group>,
- DocName<"SYCL options">;
+ DocName<"SYCL options">,
+ Visibility<[ClangOption, CLOption]>;
def cuda_Group : OptionGroup<"<CUDA group>">, Group<f_Group>,
DocName<"CUDA options">,
@@ -6782,16 +6783,21 @@ defm : FlangIgnoredDiagOpt<"frontend-loop-interchange">;
defm : FlangIgnoredDiagOpt<"target-lifetime">;
// C++ SYCL options
+let Group = sycl_Group in {
def fsycl : Flag<["-"], "fsycl">,
- Visibility<[ClangOption, CLOption]>,
- Group<sycl_Group>, HelpText<"Enables SYCL kernels compilation for device">;
+ HelpText<"Enable SYCL C++ extensions">;
def fno_sycl : Flag<["-"], "fno-sycl">,
- Visibility<[ClangOption, CLOption]>,
- Group<sycl_Group>, HelpText<"Disables SYCL kernels compilation for device">;
+ HelpText<"Disable SYCL C++ extensions">;
+def fsycl_device_only : Flag<["-"], "fsycl-device-only">,
+ Alias<offload_device_only>, HelpText<"Compile SYCL code for device only">;
+def fsycl_host_only : Flag<["-"], "fsycl-host-only">,
+ Alias<offload_host_only>, HelpText<"Compile SYCL code for host only. Has no "
+ "effect on non-SYCL compilations">;
def sycl_link : Flag<["--"], "sycl-link">, Flags<[HelpHidden]>,
- Visibility<[ClangOption, CLOption]>,
- Group<sycl_Group>, HelpText<"Perform link through clang-sycl-linker via the target "
+ HelpText<"Perform link through clang-sycl-linker via the target "
"offloading toolchain.">;
+} // let Group = sycl_Group
+
// OS-specific options
let Flags = [TargetSpecific] in {
defm android_pad_segment : BooleanFFlag<"android-pad-segment">, Group<f_Group>;
diff --git a/clang/include/clang/Driver/ToolChain.h b/clang/include/clang/Driver/ToolChain.h
index 5347e29be91439..4efef49346fa4d 100644
--- a/clang/include/clang/Driver/ToolChain.h
+++ b/clang/include/clang/Driver/ToolChain.h
@@ -762,6 +762,10 @@ class ToolChain {
virtual void AddHIPIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const;
+ /// Add arguments to use system-specific SYCL includes.
+ virtual void AddSYCLIncludeArgs(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args) const;
+
/// Add arguments to use MCU GCC toolchain includes.
virtual void AddIAMCUIncludeArgs(const llvm::opt::ArgList &DriverArgs,
llvm::opt::ArgStringList &CC1Args) const;
diff --git a/clang/lib/Driver/Action.cpp b/clang/lib/Driver/Action.cpp
index 849bf6035ebd2e..23dbcebc9a1ca1 100644
--- a/clang/lib/Driver/Action.cpp
+++ b/clang/lib/Driver/Action.cpp
@@ -111,6 +111,8 @@ std::string Action::getOffloadingKindPrefix() const {
return "device-openmp";
case OFK_HIP:
return "device-hip";
+ case OFK_SYCL:
+ return "device-sycl";
// TODO: Add other programming models here.
}
@@ -128,6 +130,8 @@ std::string Action::getOffloadingKindPrefix() const {
Res += "-hip";
if (ActiveOffloadKindMask & OFK_OpenMP)
Res += "-openmp";
+ if (ActiveOffloadKindMask & OFK_SYCL)
+ Res += "-sycl";
// TODO: Add other programming models here.
@@ -164,6 +168,8 @@ StringRef Action::GetOffloadKindName(OffloadKind Kind) {
return "openmp";
case OFK_HIP:
return "hip";
+ case OFK_SYCL:
+ return "sycl";
// TODO: Add other programming models here.
}
@@ -320,7 +326,7 @@ void OffloadAction::DeviceDependences::add(Action &A, const ToolChain &TC,
DeviceBoundArchs.push_back(BoundArch);
// Add each active offloading kind from a mask.
- for (OffloadKind OKind : {OFK_OpenMP, OFK_Cuda, OFK_HIP})
+ for (OffloadKind OKind : {OFK_OpenMP, OFK_Cuda, OFK_HIP, OFK_SYCL})
if (OKind & OffloadKindMask)
DeviceOffloadKinds.push_back(OKind);
}
diff --git a/clang/lib/Driver/CMakeLists.txt b/clang/lib/Driver/CMakeLists.txt
index 4fd10bf671512f..ea2fc967cad476 100644
--- a/clang/lib/Driver/CMakeLists.txt
+++ b/clang/lib/Driver/CMakeLists.txt
@@ -77,6 +77,7 @@ add_clang_library(clangDriver
ToolChains/RISCVToolchain.cpp
ToolChains/Solaris.cpp
ToolChains/SPIRV.cpp
+ ToolChains/SYCL.cpp
ToolChains/TCE.cpp
ToolChains/UEFI.cpp
ToolChains/VEToolchain.cpp
diff --git a/clang/lib/Driver/Compilation.cpp b/clang/lib/Driver/Compilation.cpp
index 4d4080507175c2..a39952e3c280ab 100644
--- a/clang/lib/Driver/Compilation.cpp
+++ b/clang/lib/Driver/Compilation.cpp
@@ -214,10 +214,11 @@ static bool ActionFailed(const Action *A,
if (FailingCommands.empty())
return false;
- // CUDA/HIP can have the same input source code compiled multiple times so do
- // not compiled again if there are already failures. It is OK to abort the
- // CUDA pipeline on errors.
- if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP))
+ // CUDA/HIP/SYCL can have the same input source code compiled multiple times
+ // so do not compile again if there are already failures. It is OK to abort
+ // the CUDA/HIP/SYCL pipeline on errors.
+ if (A->isOffloading(Action::OFK_Cuda) || A->isOffloading(Action::OFK_HIP) ||
+ A->isOffloading(Action::OFK_SYCL))
return true;
for (const auto &CI : FailingCommands)
diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index a0f4329e36136b..d16c4cd33e15c4 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -43,6 +43,7 @@
#include "ToolChains/PS4CPU.h"
#include "ToolChains/RISCVToolchain.h"
#include "ToolChains/SPIRV.h"
+#include "ToolChains/SYCL.h"
#include "ToolChains/Solaris.h"
#include "ToolChains/TCE.h"
#include "ToolChains/UEFI.h"
@@ -780,6 +781,41 @@ Driver::OpenMPRuntimeKind Driver::getOpenMPRuntime(const ArgList &Args) const {
return RT;
}
+static const char *getDefaultSYCLArch(Compilation &C) {
+ // If -fsycl is supplied we will assume SPIR-V
+ if (C.getDefaultToolChain().getTriple().isArch32Bit())
+ return "spirv32";
+ return "spirv64";
+}
+
+static llvm::Triple getSYCLDeviceTriple(StringRef TargetArch) {
+ SmallVector<StringRef, 5> SYCLAlias = {"spir", "spir64", "spirv", "spirv32",
+ "spirv64"};
+ if (std::find(SYCLAlias.begin(), SYCLAlias.end(), TargetArch) !=
+ SYCLAlias.end()) {
+ llvm::Triple TargetTriple;
+ TargetTriple.setArchName(TargetArch);
+ TargetTriple.setVendor(llvm::Triple::UnknownVendor);
+ TargetTriple.setOS(llvm::Triple::UnknownOS);
+ return TargetTriple;
+ }
+ return llvm::Triple(TargetArch);
+}
+
+static bool addSYCLDefaultTriple(Compilation &C,
+ SmallVectorImpl<llvm::Triple> &SYCLTriples) {
+ // Check current set of triples to see if the default has already been set.
+ for (const auto &SYCLTriple : SYCLTriples) {
+ if (SYCLTriple.getSubArch() == llvm::Triple::NoSubArch &&
+ SYCLTriple.isSPIROrSPIRV())
+ return false;
+ }
+ // Add the default triple as it was not found.
+ llvm::Triple DefaultTriple = getSYCLDeviceTriple(getDefaultSYCLArch(C));
+ SYCLTriples.insert(SYCLTriples.begin(), DefaultTriple);
+ return true;
+}
+
void Driver::CreateOffloadingDeviceToolChains(Compilation &C,
InputList &Inputs) {
@@ -993,6 +1029,42 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation &C,
return;
}
+ //
+ // SYCL
+ //
+ // We need to generate a SYCL toolchain if the user specified -fsycl.
+ bool IsSYCL = C.getInputArgs().hasFlag(options::OPT_fsycl,
+ options::OPT_fno_sycl, false);
+
+ auto argSYCLIncompatible = [&](OptSpecifier OptId) {
+ if (!IsSYCL)
+ return;
+ if (Arg *IncompatArg = C.getInputArgs().getLastArg(OptId))
+ Diag(clang::diag::err_drv_argument_not_allowed_with)
+ << IncompatArg->getSpelling() << "-fsycl";
+ };
+ // -static-libstdc++ is not compatible with -fsycl.
+ argSYCLIncompatible(options::OPT_static_libstdcxx);
+ // -ffreestanding cannot be used with -fsycl
+ argSYCLIncompatible(options::OPT_ffreestanding);
+
+ llvm::SmallVector<llvm::Triple, 4> UniqueSYCLTriplesVec;
+
+ if (IsSYCL) {
+ addSYCLDefaultTriple(C, UniqueSYCLTriplesVec);
+
+ // We'll need to use the SYCL and host triples as the key into
+ // getOffloadingDeviceToolChain, because the device toolchains we're
+ // going to create will depend on both.
+ const ToolChain *HostTC = C.getSingleOffloadToolChain<Action::OFK_Host>();
+ for (const auto &TargetTriple : UniqueSYCLTriplesVec) {
+ auto SYCLTC = &getOffloadingDeviceToolChain(
+ C.getInputArgs(), TargetTriple, *HostTC, Action::OFK_SYCL);
+ assert(SYCLTC && "Could not create offloading device tool chain.");
+ C.addOffloadDeviceToolChain(SYCLTC, Action::OFK_SYCL);
+ }
+ }
+
//
// TODO: Add support for other offloading programming models here.
//
@@ -4183,6 +4255,7 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
bool UseNewOffloadingDriver =
C.isOffloadingHostKind(Action::OFK_OpenMP) ||
+ C.isOffloadingHostKind(Action::OFK_SYCL) ||
Args.hasFlag(options::OPT_foffload_via_llvm,
options::OPT_fno_offload_via_llvm, false) ||
Args.hasFlag(options::OPT_offload_new_driver,
@@ -4593,6 +4666,8 @@ Driver::getOffloadArchs(Compilation &C, const llvm::opt::DerivedArgList &Args,
Archs.insert(OffloadArchToString(OffloadArch::HIPDefault));
else if (Kind == Action::OFK_OpenMP)
Archs.insert(StringRef());
+ else if (Kind == Action::OFK_SYCL)
+ Archs.insert(StringRef());
} else {
Args.ClaimAllArgs(options::OPT_offload_arch_EQ);
Args.ClaimAllArgs(options::OPT_no_offload_arch_EQ);
@@ -4617,7 +4692,7 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
OffloadAction::DeviceDependences DDeps;
const Action::OffloadKind OffloadKinds[] = {
- Action::OFK_OpenMP, Action::OFK_Cuda, Action::OFK_HIP};
+ Action::OFK_OpenMP, Action::OFK_Cuda, Action::OFK_HIP, Action::OFK_SYCL};
for (Action::OffloadKind Kind : OffloadKinds) {
SmallVector<const ToolChain *, 2> ToolChains;
@@ -4654,6 +4729,15 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
if (DeviceActions.empty())
return HostAction;
+ // FIXME: Do not collapse the host side for Darwin targets with SYCL offload
+ // compilations. The toolchain is not properly initialized for the target.
+ if (isa<CompileJobAction>(HostAction) && Kind == Action::OFK_SYCL &&
+ HostAction->getType() != types::TY_Nothing &&
+ C.getSingleOffloadToolChain<Action::OFK_Host>()
+ ->getTriple()
+ .isOSDarwin())
+ HostAction->setCannotBeCollapsedWithNextDependentAction();
+
auto PL = types::getCompilationPhases(*this, Args, InputType);
for (phases::ID Phase : PL) {
@@ -4662,6 +4746,11 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
break;
}
+ // Assemble actions are not used for the SYCL device side. Both compile
+ // and backend actions are used to generate IR and textual IR if needed.
+ if (Kind == Action::OFK_SYCL && Phase == phases::Assemble)
+ continue;
+
auto TCAndArch = TCAndArchs.begin();
for (Action *&A : DeviceActions) {
if (A->getType() == types::TY_Nothing)
@@ -4900,6 +4989,7 @@ Action *Driver::ConstructPhaseAction(
return C.MakeAction<BackendJobAction>(Input, Output);
}
if (Args.hasArg(options::OPT_emit_llvm) ||
+ TargetDeviceOffloadKind == Action::OFK_SYCL ||
(((Input->getOffloadingToolChain() &&
Input->getOffloadingToolChain()->getTriple().isAMDGPU()) ||
TargetDeviceOffloadKind == Action::OFK_HIP) &&
@@ -6591,6 +6681,18 @@ const ToolChain &Driver::getOffloadingDeviceToolChain(
HostTC, Args);
break;
}
+ case Action::OFK_SYCL:
+ switch (Target.getArch()) {
+ case llvm::Triple::spir:
+ case llvm::Triple::spir64:
+ case llvm::Triple::spirv32:
+ case llvm::Triple::spirv64:
+ TC = std::make_unique<toolchains::SYCLToolChain>(*this, Target, HostTC,
+ Args);
+ break;
+ default:
+ break;
+ }
default:
break;
}
diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp
index 0d426a467e9a3b..dc950e2afa8aea 100644
--- a/clang/lib/Driver/ToolChain.cpp
+++ b/clang/lib/Driver/ToolChain.cpp
@@ -1485,6 +1485,9 @@ void ToolChain::AddCudaIncludeArgs(const ArgList &DriverArgs,
void ToolChain::AddHIPIncludeArgs(const ArgList &DriverArgs,
ArgStringList &CC1Args) const {}
+void ToolChain::AddSYCLIncludeArgs(const ArgList &DriverArgs,
+ ArgStringList &CC1Args) const {}
+
llvm::SmallVector<ToolChain::BitCodeLibraryInfo, 12>
ToolChain::getDeviceLibs(const ArgList &DriverArgs) const {
return {};
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index d3eec9fea0d498..5110cca167aace 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -24,6 +24,7 @@
#include "Hexagon.h"
#include "MSP430.h"
#include "PS4CPU.h"
+#include "SYCL.h"
#include "clang/Basic/CLWarnings.h"
#include "clang/Basic/CharInfo.h"
#include "clang/Basic/CodeGenOptions.h"
@@ -122,6 +123,13 @@ forAllAssociatedToolChains(Compilation &C, const JobAction &JA,
} else if (JA.isDeviceOffloading(Action::OFK_OpenMP))
Work(*C.getSingleOffloadToolChain<Action::OFK_Host>());
+ if (JA.isHostOffloading(Action::OFK_SYCL)) {
+ auto TCs = C.getOffloadToolChains<Action::OFK_SYCL>();
+ for (auto II = TCs.first, IE = TCs.second; II != IE; ++II)
+ Work(*II->second);
+ } else if (JA.isDeviceOffloading(Action::OFK_SYCL))
+ Work(*C.getSingleOffloadToolChain<Action::OFK_Host>());
+
//
// TODO: Add support for other offloading programming models here.
//
@@ -1070,14 +1078,16 @@ void Clang::AddPreprocessingOptions(Compilation &C, const JobAction &JA,
Args.AddLastArg(CmdArgs, options::OPT_MP);
Args.AddLastArg(CmdArgs, options::OPT_MV);
- // Add offload include arguments specific for CUDA/HIP. This must happen
+ // Add offload include arguments specific for CUDA/HIP/SYCL. This must happen
// before we -I or -include anything else, because we must pick up the
- // CUDA/HIP headers from the particular CUDA/ROCm installation, rather than
- // from e.g. /usr/local/include.
+ // CUDA/HIP/SYCL headers from the particular CUDA/ROCm/SYCL installation,
+ // rather than from e.g. /usr/local/include.
if (JA.isOffloading(Action::OFK_Cuda))
getToolChain().AddCudaIncludeArgs(Args, CmdArgs);
if (JA.isOffloading(Action::OFK_HIP))
getToolChain().AddHIPIncludeArgs(Args, CmdArgs);
+ if (JA.isOffloading(Action::OFK_SYCL))
+ getToolChain().AddSYCLIncludeArgs(Args, CmdArgs);
// If we are offloading to a target via OpenMP we need to include the
// openmp_wrappers folder which contains alternative system headers.
@@ -5032,17 +5042,21 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
// second input. Module precompilation accepts a list of header files to
// include as part of the module. API extraction accepts a list of header
// files whose API information is emitted in the output. All other jobs are
- // expected to have exactly one input.
+ // expected to have exactly one input. SYCL compilation only expects a
+ // single input.
bool IsCuda = JA.isOffloading(Action::OFK_Cuda);
bool IsCudaDevice = JA.isDeviceOffloading(Action::OFK_Cuda);
bool IsHIP = JA.isOffloading(Action::OFK_HIP);
bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
+ bool IsSYCL = JA.isOffloading(Action::OFK_SYCL);
+ bool IsSYCLDevice = JA.isDeviceOffloading(Action::OFK_SYCL);
bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
bool IsExtractAPI = isa<ExtractAPIJobAction>(JA);
bool IsDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) ||
JA.isDeviceOffloading(Action::OFK_Host));
bool IsHostOffloadingAction =
JA.isHostOffloading(Action::OFK_OpenMP) ||
+ JA.isHostOffloading(Action::OFK_SYCL) ||
(JA.isHostOffloading(C.getActiveOffloadKinds()) &&
Args.hasFlag(options::OPT_offload_new_driver,
options::OPT_no_offload_new_driver, false));
@@ -5092,10 +5106,10 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
bool IsWindowsMSVC = RawTriple.isWindowsMSVCEnvironment();
bool IsIAMCU = RawTriple.isOSIAMCU();
- // Adjust IsWindowsXYZ for CUDA/HIP compilations. Even when compiling in
+ // Adjust IsWindowsXYZ for CUDA/HIP/SYCL compilations. Even when compiling in
// device mode (i.e., getToolchain().getTriple() is NVPTX/AMDGCN, not
// Windows), we need to pass Windows-specific flags to cc1.
- if (IsCuda || IsHIP)
+ if (IsCuda || IsHIP || IsSYCL)
IsWindowsMSVC |= AuxTriple && AuxTriple->isWindowsMSVCEnvironment();
// C++ is not supported for IAMCU.
@@ -5179,11 +5193,33 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
if (const Arg *PF = Args.getLastArg(options::OPT_mprintf_kind_EQ))
PF->claim();
- if (Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false)) {
- CmdArgs.push_back("-fsycl-is-device");
+ if (IsSYCL) {
+ if (IsSYCLDevice) {
+ // Host triple is needed when doing SYCL device compilations.
+ llvm::Triple AuxT = C.getDefaultToolChain().getTriple();
+ std::string NormalizedTriple = AuxT.normalize();
+ CmdArgs.push_back("-aux-triple");
+ CmdArgs.push_back(Args.MakeArgString(NormalizedTriple));
- if (Arg *A = Args.getLastArg(options::OPT_sycl_std_EQ)) {
- A->render(Args, CmdArgs);
+ // We want to compile sycl kernels.
+ CmdArgs.push_back("-fsycl-is-device");
+
+ // Set O2 optimization level by default
+ if (!Args.getLastArg(options::OPT_O_Group))
+ CmdArgs.push_back("-O2");
+ } else {
+ // Add any options that are needed specific to SYCL offload while
+ // performing the host side compilation.
+
+ // Let the front-end host compilation flow know about SYCL offload
+ // compilation.
+ CmdArgs.push_back("-fsycl-is-host");
+ }
+
+ // Set options for both host and device.
+ Arg *SYCLStdArg = Args.getLastArg(options::OPT_sycl_std_EQ);
+ if (SYCLStdArg) {
+ SYCLStdArg->render(Args, CmdArgs);
} else {
// Ensure the default version in SYCL mode is 2020.
CmdArgs.push_back("-sycl-std=2020");
@@ -6129,7 +6165,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
// Prepare `-aux-target-cpu` and `-aux-target-feature` unless
// `--gpu-use-aux-triple-only` is specified.
if (!Args.getLastArg(options::OPT_gpu_use_aux_triple_only) &&
- (IsCudaDevice || IsHIPDevice)) {
+ (IsCudaDevice || IsHIPDevice || IsSYCLDevice)) {
const ArgList &HostArgs =
C.getArgsForToolChain(nullptr, StringRef(), Action::OFK_None);
std::string HostCPU =
diff --git a/clang/lib/Driver/ToolChains/Darwin.cpp b/clang/lib/Driver/ToolChains/Darwin.cpp
index 87380869f6fdab..f225c5b16a7c0a 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -965,7 +965,8 @@ MachO::MachO(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
/// Darwin - Darwin tool chain for i386 and x86_64.
Darwin::Darwin(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
...
[truncated]
|
tahonermann
reviewed
Nov 22, 2024
Contributor
tahonermann
left a comment
tahonermann
left a comment
There was a problem hiding this comment.
This is looking good to me. I added some questions and noted a couple of potential concerns for which I'm not well equipped to have an opinion on. Perhaps others can offer their thoughts on those.
clang/lib/Driver/ToolChains/SYCL.cpp
Outdated
Comment on lines
31
to
33
| SmallString<128> IncludePath(D.Dir); | ||
| llvm::sys::path::append(IncludePath, ".."); | ||
| llvm::sys::path::append(IncludePath, "include"); |
Contributor
There was a problem hiding this comment.
I remain concerned about the potential for this include path to be added multiple times since it isn't SYCL specific. Is this known to be required? That directory will have "c++", "clang", "clang-c", "llvm", and "llvm-c" directories. Does, e.g., #include <clang/AST/AST.h> work without the user having to manually add an additional include path? I'm sympathetic to wanting #include <sycl/sycl.hpp> to just work when -fsycl is passed. Is there another way we can accomplish that? An ugly way that we definitely shouldn't do (and wouldn't work everywhere anyway) would be to add a path to ../include/sycl and then add a sycl -> . symlink in that directory. I likewise don't like the idea of creating an include/sycl/sycl directory structure, but that would work too. Other ideas?
Contributor
Author
There was a problem hiding this comment.
I would guess this depends on how ultimately the include/sycl headers are installed and how we should point to them. As the headers are not currently part of the build, this is basically just a location in which to try and find the SYCL specific headers (this is what we are doing in intel/llvm).
Contributor
There was a problem hiding this comment.
I think we should work this out with the SYCL library folks before making this change then since duplicate paths in the header search path can be problematic and, when they are, difficult to work around.
Contributor
Author
There was a problem hiding this comment.
Updates made to remove the header inclusions - FIXME in it's place until this is resolved with the library folks.
clang/lib/Driver/ToolChains/SYCL.cpp
Outdated
Comment on lines
84
to
85
| // All sanitizer options are not currently supported, except | ||
| // AddressSanitizer. |
Contributor
There was a problem hiding this comment.
Is it accurate to say that address sanitizer is currently supported? Given that device code generation hasn't landed upstream yet, I'm a little skeptical :) Are there additional patches to AddressSanitizer that will need to land before this comment is a true statement? If so, perhaps we should remove this special case and add it back once support is actually present.
Contributor
Author
There was a problem hiding this comment.
Right - I will remove this special casing until the support is upstreamed.
Contributor
Author
|
Ping |
tahonermann
requested changes
Dec 9, 2024
clang/lib/Driver/ToolChains/SYCL.cpp
Outdated
Comment on lines
31
to
33
| SmallString<128> IncludePath(D.Dir); | ||
| llvm::sys::path::append(IncludePath, ".."); | ||
| llvm::sys::path::append(IncludePath, "include"); |
Contributor
There was a problem hiding this comment.
I think we should work this out with the SYCL library folks before making this change then since duplicate paths in the header search path can be problematic and, when they are, difficult to work around.
tahonermann
approved these changes
Dec 10, 2024
Contributor
tahonermann
left a comment
tahonermann
left a comment
There was a problem hiding this comment.
Thanks, @mdtoguchi, this looks good to me!
Contributor
|
@MaskRay, could you please take a look at this PR and give it your stamp of approval if you have no concerns? |
Contributor
Author
|
Ping |
MaskRay
reviewed
Dec 25, 2024
clang/lib/Driver/Driver.cpp
Outdated
| static llvm::Triple getSYCLDeviceTriple(StringRef TargetArch) { | ||
| SmallVector<StringRef, 5> SYCLAlias = {"spir", "spir64", "spirv", "spirv32", | ||
| "spirv64"}; | ||
| if (std::find(SYCLAlias.begin(), SYCLAlias.end(), TargetArch) != |
Member
There was a problem hiding this comment.
llvm::is_contained({..., ..., ...}, TargetArch)
MaskRay
requested changes
Dec 25, 2024
clang/lib/Driver/Driver.cpp
Outdated
| return RT; | ||
| } | ||
|
|
||
| static const char *getDefaultSYCLArch(Compilation &C) { |
Member
There was a problem hiding this comment.
Remove this trivial function and inline its use
clang/lib/Driver/Driver.cpp
Outdated
| return; | ||
| } | ||
|
|
||
| // |
Member
There was a problem hiding this comment.
We don't use start a comment block with //\n. Just delete the part before We need to generate a SYCL toolchain if the user specified -fsycl.
clang/lib/Driver/Driver.cpp
Outdated
| break; | ||
| } | ||
| case Action::OFK_SYCL: | ||
| switch (Target.getArch()) { |
clang/lib/Driver/ToolChains/Gnu.h
Outdated
| const llvm::opt::ArgList &DriverArgs, | ||
| llvm::opt::ArgStringList &CC1Args) const override; | ||
|
|
||
| void AddSYCLIncludeArgs(const llvm::opt::ArgList &DriverArgs, |
Member
There was a problem hiding this comment.
since this is new function. name it add... instead of Add....
Yes, clangDriver naming is very inconsistent, but we try to make them consistent for new functions.
| // RUN: %s 2>&1 \ | ||
| // RUN: | FileCheck -check-prefixes=CHK-PHASES %s | ||
| // CHK-PHASES: 0: input, "[[INPUT:.+\.cpp]]", c++, (host-sycl) | ||
| // CHK-PHASES: 1: preprocessor, {0}, c++-cpp-output, (host-sycl) |
Member
There was a problem hiding this comment.
Use CHK-PHASES-NEXT whenever applicable
|
|
||
| /// Check the phases graph with -fsycl. Use of -fsycl enables offload | ||
| // RUN: %clang -ccc-print-phases --target=x86_64-unknown-linux-gnu \ | ||
| // RUN: -fsycl %s 2>&1 \ |
| /// well as clang-offload-packager inputs. | ||
| // RUN: %clang -### -fsycl -c --target=x86_64-unknown-linux-gnu %s 2>&1 \ | ||
| // RUN: | FileCheck -check-prefix=CHK-DEVICE-TRIPLE %s | ||
| // CHK-DEVICE-TRIPLE: clang{{.*}} "-triple" "spirv64-unknown-unknown" |
Member
There was a problem hiding this comment.
When testing cc1 commands, we sometimes test "-cc1", but we don't test clang{{.*}}, since the string could be different in a bazel build. https://github.com/maskray/config/wiki/LLVM#clanglibdriver
| /// Check -fsycl-is-device is passed when compiling for the device. | ||
| /// Check -fsycl-is-host is passed when compiling for host. | ||
| // RUN: %clang -### -fsycl -c %s 2>&1 \ | ||
| // RUN: | FileCheck -check-prefixes=CHK-FSYCL-IS-DEVICE,CHK-FSYCL-IS-HOST %s |
Member
There was a problem hiding this comment.
indent continuation lines by 2. please fix this everywhere in this PR.
Member
- remove simple function and inline usage - use if statement for type - use is_contained
mdtoguchi
changed the title
Contributor
Author
|
@MaskRay - thank you for the review. I have made the updates as suggested, please have another look when time permits. |
MaskRay
reviewed
Jan 3, 2025
clang/lib/Driver/ToolChains/SYCL.cpp
Outdated
| } | ||
|
|
||
| // Unsupported options for SYCL device compilation. | ||
| static std::vector<OptSpecifier> getUnsupportedOpts() { |
Member
There was a problem hiding this comment.
return ArrayRef.
UnsupportedOpts below can be a plain array
MaskRay
reviewed
Jan 3, 2025
| @@ -0,0 +1,52 @@ | |||
| /// | |||
Member
There was a problem hiding this comment.
Thanks for adding a file-level comment. We just write /// xxxx instead of ///\n/// xxxx\n///
MaskRay
reviewed
Jan 3, 2025
|
|
||
| void toolchains::MinGW::addSYCLIncludeArgs(const ArgList &DriverArgs, | ||
| ArgStringList &CC1Args) const { | ||
| SYCLInstallation->addSYCLIncludeArgs(DriverArgs, CC1Args); |
Member
There was a problem hiding this comment.
is this needed or tested?
If you add an overload to a specific ToolChain, ensure that this ToolChain gets coverage. Otherwise someone could find the function unneeded and delete it.
Contributor
Author
There was a problem hiding this comment.
Initial testing found the need for this due to the general check for the updated header search locations. As reviews progressed with @tahonermann, it was suggested to not add the explicit header search locations as it was not determined yet (SYCL specific headers and libraries not yet part of the build) so now there is nothing being tested here for this target. This can be cleaned up and added again later when the headers are actually searched.
MaskRay
approved these changes
Jan 4, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/8566 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/12282 Here is the relevant piece of the build log for the reference |
Contributor
|
This PR introduced a new compile warning when compiling using clang. Please fix it. |
Contributor
|
We're seeing SYCL tests fail on Linux builders after this patch https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-host-linux-x64/b8726474372098645345/overview Can you take a look, and revert if the fix will take a while? |
Contributor
Author
|
I have posted a fix to address the sanitizer builds and build warning: #121822 |
Contributor
|
I see a new warning from this patch: |
Contributor
|
also: |
Contributor
Author
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Introduces the SYCL based toolchain and initial toolchain construction when using the '-fsycl' option. This option will enable SYCL based offloading, creating a SPIR-V based IR file packaged into the compiled host object.
This includes early support for creating the host/device object using the new offloading model. The device object is created using the spir64-unknown-unknown target triple.
New/Updated Options:
-fsycl Enables SYCL offloading for host and device
-fsycl-device-only
Enables device only compilation for SYCL
-fsycl-host-only
Enables host only compilation for SYCL
RFC Reference:
https://discourse.llvm.org/t/rfc-sycl-driver-enhancements/74092
Was reverted due to buildbot issues. Contains additional fixes to pull in the SYCL header dependencies to other toolchains.
This is a reland of: #107493