Skip to content

Commit

Permalink
[Driver][SYCL] Use existing option for device-only and some cleanup (#…
Browse files Browse the repository at this point in the history
…15274)

Update -fsycl-device-only usage to be an alias to --offload-device-only.
This requires an additional unique check for SYCL enabling due to the
expectation that -fsycl-device-only also implies -fsycl.

Update associated tests that use -fsycl-device-only -emit-llvm. Our
default is already bitcode output, fixing this up aligns better with
community expectations when using --offload-device-only.

Also do some variable cleanup to better be in sync with community
variable names. And use a more common diagnostic for invalid options
with -fsycl.
  • Loading branch information
mdtoguchi authored Sep 4, 2024
1 parent cba44cd commit fd4b409
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 35 deletions.
2 changes: 0 additions & 2 deletions clang/include/clang/Basic/DiagnosticDriverKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,6 @@ def warn_drv_mismatch_fpga_archive : Warning<
def warn_drv_sycl_native_cpu_and_targets: Warning<
"-fsycl-targets=native_cpu overrides SYCL targets option">,
InGroup<SyclNativeCPUTargets>;
def err_drv_unsupported_opt_sycl : Error<
"option '%0' not supported with SYCL compilation">;
def err_drv_argument_only_allowed_with : Error<
"invalid argument '%0' only allowed with '%1'">;
def err_drv_opt_unsupported_input_type : Error<
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Driver/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -6779,7 +6779,7 @@ def fintelfpga : Flag<["-"], "fintelfpga">,
MarshallingInfoFlag<LangOpts<"IntelFPGA">>,
HelpText<"Perform ahead-of-time compilation for FPGA">;
def fsycl_device_only : Flag<["-"], "fsycl-device-only">,
HelpText<"Compile SYCL kernels for device">;
Alias<offload_device_only>, HelpText<"Compile SYCL kernels for device">;
def fsycl_embed_ir : Flag<["-"], "fsycl-embed-ir">,
HelpText<"Embed LLVM IR for runtime kernel fusion">;
defm sycl_esimd_force_stateless_mem : BoolFOption<"sycl-esimd-force-stateless-mem",
Expand Down
33 changes: 22 additions & 11 deletions clang/lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,7 @@ phases::ID Driver::getFinalPhase(const DerivedArgList &DAL,
FinalPhase = phases::Compile;

// -S only runs up to the backend.
} else if ((PhaseArg = DAL.getLastArg(options::OPT_S)) ||
(PhaseArg = DAL.getLastArg(options::OPT_fsycl_device_only))) {
} else if ((PhaseArg = DAL.getLastArg(options::OPT_S))) {
FinalPhase = phases::Backend;

// -c compilation only runs up to the assembler.
Expand Down Expand Up @@ -855,6 +854,19 @@ static bool addSYCLDefaultTriple(Compilation &C,
return true;
}

// Special function that checks if -fsycl-device-only was passed on the
// command line. -fsycl-device-only is an alias of --offload-device-only.
static bool hasSYCLDeviceOnly(const ArgList &Args) {
if (const Arg *SYCLDeviceOnlyArg =
Args.getLastArg(options::OPT_offload_device_only)) {
while (SYCLDeviceOnlyArg->getAlias())
SYCLDeviceOnlyArg = SYCLDeviceOnlyArg->getAlias();
if (SYCLDeviceOnlyArg->getSpelling().contains("sycl-device-only"))
return true;
}
return false;
}

void Driver::CreateOffloadingDeviceToolChains(Compilation &C,
InputList &Inputs) {

Expand Down Expand Up @@ -1074,7 +1086,7 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation &C,
bool HasValidSYCLRuntime =
C.getInputArgs().hasFlag(options::OPT_fsycl, options::OPT_fno_sycl,
false) ||
C.getInputArgs().hasArg(options::OPT_fsycl_device_only);
hasSYCLDeviceOnly(C.getInputArgs());

Arg *SYCLfpga = C.getInputArgs().getLastArg(options::OPT_fintelfpga);

Expand Down Expand Up @@ -1118,8 +1130,8 @@ void Driver::CreateOffloadingDeviceToolChains(Compilation &C,
if (!HasValidSYCLRuntime)
return;
if (Arg *IncompatArg = C.getInputArgs().getLastArg(OptId))
Diag(clang::diag::err_drv_unsupported_opt_sycl)
<< IncompatArg->getSpelling();
Diag(clang::diag::err_drv_argument_not_allowed_with)
<< IncompatArg->getSpelling() << "-fsycl";
};
// -static-libstdc++ is not compatible with -fsycl.
argSYCLIncompatible(options::OPT_static_libstdcxx);
Expand Down Expand Up @@ -1743,13 +1755,12 @@ Compilation *Driver::BuildCompilation(ArrayRef<const char *> ArgList) {
if (Args.getLastArg(options::OPT_fsycl_dump_device_code_EQ))
DumpDeviceCode = true;

if (const Arg *A = Args.getLastArg(
options::OPT_offload_host_only, options::OPT_offload_device_only,
options::OPT_offload_host_device, options::OPT_fsycl_device_only)) {
if (const Arg *A = Args.getLastArg(options::OPT_offload_host_only,
options::OPT_offload_device_only,
options::OPT_offload_host_device)) {
if (A->getOption().matches(options::OPT_offload_host_only))
Offload = OffloadHost;
else if (A->getOption().matches(options::OPT_offload_device_only) ||
A->getOption().matches(options::OPT_fsycl_device_only))
else if (A->getOption().matches(options::OPT_offload_device_only))
Offload = OffloadDevice;
else
Offload = OffloadHostDevice;
Expand Down Expand Up @@ -3124,7 +3135,7 @@ void Driver::BuildInputs(const ToolChain &TC, DerivedArgList &Args,
Arg *InputTypeArg = nullptr;
bool IsSYCL =
Args.hasFlag(options::OPT_fsycl, options::OPT_fno_sycl, false) ||
Args.hasArg(options::OPT_fsycl_device_only);
hasSYCLDeviceOnly(Args);

// The last /TC or /TP option sets the input type to C or C++ globally.
if (Arg *TCTP = Args.getLastArgNoClaim(options::OPT__SLASH_TC,
Expand Down
31 changes: 15 additions & 16 deletions clang/lib/Driver/ToolChains/Clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5220,8 +5220,8 @@ static void ProcessVSRuntimeLibrary(const ToolChain &TC, const ArgList &Args,
(RTOptionID == options::OPT__SLASH_MT ||
RTOptionID == options::OPT__SLASH_MTd))
// Use of /MT or /MTd is not supported for SYCL.
TC.getDriver().Diag(diag::err_drv_unsupported_opt_sycl)
<< SetArg->getOption().getName();
TC.getDriver().Diag(clang::diag::err_drv_argument_not_allowed_with)
<< SetArg->getOption().getName() << "-fsycl";

enum { addDEBUG = 0x1, addMT = 0x2, addDLL = 0x4 };
auto addPreDefines = [&](unsigned Defines) {
Expand Down Expand Up @@ -5326,7 +5326,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
bool IsHIP = JA.isOffloading(Action::OFK_HIP);
bool IsHIPDevice = JA.isDeviceOffloading(Action::OFK_HIP);
bool IsOpenMPDevice = JA.isDeviceOffloading(Action::OFK_OpenMP);
bool IsSYCLOffloadDevice = JA.isDeviceOffloading(Action::OFK_SYCL);
bool IsSYCLDevice = JA.isDeviceOffloading(Action::OFK_SYCL);
bool IsSYCL = JA.isOffloading(Action::OFK_SYCL);
bool IsExtractAPI = isa<ExtractAPIJobAction>(JA);
bool IsDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) ||
Expand All @@ -5342,14 +5342,13 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
bool IsUsingLTO = D.isUsingLTO(IsDeviceOffloadAction);
auto LTOMode = D.getLTOMode(IsDeviceOffloadAction);
bool IsFPGASYCLOffloadDevice =
IsSYCLOffloadDevice &&
Triple.getSubArch() == llvm::Triple::SPIRSubArch_fpga;
IsSYCLDevice && Triple.getSubArch() == llvm::Triple::SPIRSubArch_fpga;
const bool IsSYCLNativeCPU = isSYCLNativeCPU(TC);

// Perform the SYCL host compilation using an external compiler if the user
// requested.
if (Args.hasArg(options::OPT_fsycl_host_compiler_EQ) && IsSYCL &&
!IsSYCLOffloadDevice) {
!IsSYCLDevice) {
ConstructHostCompilerJob(C, JA, Output, Inputs, Args);
return;
}
Expand Down Expand Up @@ -5485,7 +5484,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,

Arg *SYCLStdArg = Args.getLastArg(options::OPT_sycl_std_EQ);

if (IsSYCLOffloadDevice) {
if (IsSYCLDevice) {
if (Triple.isNVPTX()) {
StringRef GPUArchName = JA.getOffloadingArch();
// TODO: Once default arch is moved to at least SM_53, empty arch should
Expand Down Expand Up @@ -5674,7 +5673,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
llvm::Triple SYCLTriple = TI->second->getTriple();
if (SYCLTriple.getSubArch() == llvm::Triple::SPIRSubArch_fpga) {
HasFPGA = true;
if (!IsSYCLOffloadDevice) {
if (!IsSYCLDevice) {
CmdArgs.push_back("-aux-triple");
CmdArgs.push_back(Args.MakeArgString(SYCLTriple.getTriple()));
}
Expand All @@ -5700,7 +5699,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,

// Add any options that are needed specific to SYCL offload while
// performing the host side compilation.
if (!IsSYCLOffloadDevice) {
if (!IsSYCLDevice) {
// Add the -include option to add the integration header
StringRef Header = D.getIntegrationHeader(Input.getBaseInput());
// Do not add the integration header if we are compiling after the
Expand Down Expand Up @@ -5769,7 +5768,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
D.addSYCLTargetMacroArg(Args, Macro);
}
};
if (IsSYCLOffloadDevice)
if (IsSYCLDevice)
addTargetMacros(RawTriple);
else {
for (auto &Macro : D.getSYCLTargetMacroArgs())
Expand Down Expand Up @@ -5874,13 +5873,13 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-fdirectives-only");
}
} else if (isa<AssembleJobAction>(JA)) {
if (IsSYCLOffloadDevice && !IsSYCLNativeCPU) {
if (IsSYCLDevice && !IsSYCLNativeCPU) {
CmdArgs.push_back("-emit-llvm-bc");
} else {
CmdArgs.push_back("-emit-obj");
CollectArgsForIntegratedAssembler(C, Args, CmdArgs, D);
}
if (IsSYCLOffloadDevice && IsSYCLNativeCPU) {
if (IsSYCLDevice && IsSYCLNativeCPU) {
CmdArgs.push_back("-mllvm");
CmdArgs.push_back("-sycl-native-cpu-backend");
}
Expand Down Expand Up @@ -6767,7 +6766,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 || (IsSYCL && IsSYCLOffloadDevice) || IsHIPDevice)) {
(IsCudaDevice || (IsSYCL && IsSYCLDevice) || IsHIPDevice)) {
const ArgList &HostArgs =
C.getArgsForToolChain(nullptr, StringRef(), Action::OFK_None);
std::string HostCPU =
Expand Down Expand Up @@ -8432,8 +8431,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
// be added so both IR can be captured.
if ((C.getDriver().isSaveTempsEnabled() ||
JA.isHostOffloading(Action::OFK_OpenMP)) &&
!(C.getDriver().embedBitcodeInObject() && !IsUsingLTO) &&
!IsSYCLOffloadDevice && isa<CompileJobAction>(JA))
!(C.getDriver().embedBitcodeInObject() && !IsUsingLTO) && !IsSYCLDevice &&
isa<CompileJobAction>(JA))
CmdArgs.push_back("-disable-llvm-passes");

Args.AddAllArgs(CmdArgs, options::OPT_undef);
Expand Down Expand Up @@ -8539,7 +8538,7 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.AddLastArg(CmdArgs, options::OPT_foffload_implicit_host_device_templates,
options::OPT_fno_offload_implicit_host_device_templates);

if (IsCudaDevice || IsHIPDevice || IsSYCLOffloadDevice) {
if (IsCudaDevice || IsHIPDevice || IsSYCLDevice) {
StringRef InlineThresh =
Args.getLastArgValue(options::OPT_fgpu_inline_threshold_EQ);
if (!InlineThresh.empty()) {
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/sycl-MD-default.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@
// RUN: | FileCheck -check-prefix=CHK-ERROR %s
// RUN: not %clang_cl -### -MTd -fsycl -c %s 2>&1 \
// RUN: | FileCheck -check-prefix=CHK-ERROR %s
// CHK-ERROR: option 'MT{{d*}}' not supported with SYCL compilation
// CHK-ERROR: invalid argument 'MT{{d*}}' not allowed with '-fsycl'
2 changes: 1 addition & 1 deletion clang/test/Driver/sycl-offload-old-model.c
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@
// RUN: | FileCheck -check-prefix=CHK-INCOMPATIBILITY %s -DINCOMPATOPT=-ffreestanding
// RUN: not %clang -### -fsycl --no-offload-new-driver -static-libstdc++ %s 2>&1 \
// RUN: | FileCheck -check-prefix=CHK-INCOMPATIBILITY %s -DINCOMPATOPT=-static-libstdc++
// CHK-INCOMPATIBILITY: error: option '[[INCOMPATOPT]]' not supported with SYCL compilation
// CHK-INCOMPATIBILITY: error: invalid argument '[[INCOMPATOPT]]' not allowed with '-fsycl'

/// Using -fsyntax-only with -fsycl --no-offload-new-driver should not emit IR
// RUN: %clang -### -fsycl --no-offload-new-driver -fsyntax-only %s 2>&1 \
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/sycl-offload.c
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@
// RUN: | FileCheck -check-prefix=CHK-INCOMPATIBILITY %s -DINCOMPATOPT=-ffreestanding
// RUN: not %clang -### -fsycl --offload-new-driver -static-libstdc++ %s 2>&1 \
// RUN: | FileCheck -check-prefix=CHK-INCOMPATIBILITY %s -DINCOMPATOPT=-static-libstdc++
// CHK-INCOMPATIBILITY: error: option '[[INCOMPATOPT]]' not supported with SYCL compilation
// CHK-INCOMPATIBILITY: error: invalid argument '[[INCOMPATOPT]]' not allowed with '-fsycl'

/// Using -fsyntax-only with -fsycl --offload-new-driver should not emit IR
// RUN: %clang -### -fsycl --offload-new-driver -fsyntax-only %s 2>&1 \
Expand Down
2 changes: 1 addition & 1 deletion sycl/test/basic_tests/accessor/no_offset_error.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %clangxx -fsycl-device-only -Xclang -verify -Xclang -verify-ignore-unexpected=note -emit-llvm -o - %s
// RUN: %clangxx -fsycl-device-only -Xclang -verify -Xclang -verify-ignore-unexpected=note -o - %s

#include <sycl/sycl.hpp>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %clangxx -O2 -fsycl -fsycl-device-only -emit-llvm %s -o %t
// RUN: %clangxx -O2 -fsycl -fsycl-device-only %s -o %t
// RUN: sycl-post-link -properties -split-esimd -lower-esimd -O2 -S %t -o %t.table
// RUN: FileCheck %s -input-file=%t_esimd_0.ll
// Checks that we set 0 as VCSLMSize when slm_init is used with
Expand Down

0 comments on commit fd4b409

Please sign in to comment.