From 11e38ef5b8b4503f2ac5e8415a0885fc6875b237 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Tue, 3 Sep 2024 11:19:49 -0700 Subject: [PATCH] [Driver][SYCL] Use existing option for device-only and some cleanup 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. --- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/Driver.cpp | 33 ++++++++++++------- clang/lib/Driver/ToolChains/Clang.cpp | 31 +++++++++-------- clang/test/Driver/sycl-MD-default.cpp | 2 +- clang/test/Driver/sycl-offload-old-model.c | 2 +- clang/test/Driver/sycl-offload.c | 2 +- .../basic_tests/accessor/no_offset_error.cpp | 2 +- .../esimd/slm_init_specconst_size.cpp | 2 +- 8 files changed, 43 insertions(+), 33 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index c0aa9be7752f3..d29bc4c3abdc9 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -6779,7 +6779,7 @@ def fintelfpga : Flag<["-"], "fintelfpga">, MarshallingInfoFlag>, HelpText<"Perform ahead-of-time compilation for FPGA">; def fsycl_device_only : Flag<["-"], "fsycl-device-only">, - HelpText<"Compile SYCL kernels for device">; + Alias, 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", diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index a4017afc4c514..d08f1bca0395b 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -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. @@ -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) { @@ -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); @@ -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); @@ -1743,13 +1755,12 @@ Compilation *Driver::BuildCompilation(ArrayRef 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; @@ -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, diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 6b31d36983687..5df458a280190 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -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) { @@ -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(JA); bool IsDeviceOffloadAction = !(JA.isDeviceOffloading(Action::OFK_None) || @@ -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; } @@ -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 @@ -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())); } @@ -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 @@ -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()) @@ -5874,13 +5873,13 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back("-fdirectives-only"); } } else if (isa(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"); } @@ -6779,7 +6778,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 = @@ -8444,8 +8443,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(JA)) + !(C.getDriver().embedBitcodeInObject() && !IsUsingLTO) && !IsSYCLDevice && + isa(JA)) CmdArgs.push_back("-disable-llvm-passes"); Args.AddAllArgs(CmdArgs, options::OPT_undef); @@ -8551,7 +8550,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()) { diff --git a/clang/test/Driver/sycl-MD-default.cpp b/clang/test/Driver/sycl-MD-default.cpp index c9c24934cd531..3fe3767f6a14f 100644 --- a/clang/test/Driver/sycl-MD-default.cpp +++ b/clang/test/Driver/sycl-MD-default.cpp @@ -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' diff --git a/clang/test/Driver/sycl-offload-old-model.c b/clang/test/Driver/sycl-offload-old-model.c index 5a0e5e62c0cf4..bc310be7ae58f 100644 --- a/clang/test/Driver/sycl-offload-old-model.c +++ b/clang/test/Driver/sycl-offload-old-model.c @@ -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 \ diff --git a/clang/test/Driver/sycl-offload.c b/clang/test/Driver/sycl-offload.c index 5d67283494ea3..60db00bea9c4f 100644 --- a/clang/test/Driver/sycl-offload.c +++ b/clang/test/Driver/sycl-offload.c @@ -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 \ diff --git a/sycl/test/basic_tests/accessor/no_offset_error.cpp b/sycl/test/basic_tests/accessor/no_offset_error.cpp index 39ced3dc00a73..a2c0b6a03a9f4 100644 --- a/sycl/test/basic_tests/accessor/no_offset_error.cpp +++ b/sycl/test/basic_tests/accessor/no_offset_error.cpp @@ -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 diff --git a/sycl/test/check_device_code/esimd/slm_init_specconst_size.cpp b/sycl/test/check_device_code/esimd/slm_init_specconst_size.cpp index f12404afc81fe..a2f76ff0981ab 100644 --- a/sycl/test/check_device_code/esimd/slm_init_specconst_size.cpp +++ b/sycl/test/check_device_code/esimd/slm_init_specconst_size.cpp @@ -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