From 9496bb7d46ce550514f9056e0a225f42c0276113 Mon Sep 17 00:00:00 2001 From: Marcos Maronas Date: Thu, 2 Oct 2025 16:04:40 +0200 Subject: [PATCH 1/2] [SYCL][clang-linker-wrapper] Fix argument passing to ocloc. --- .../ClangLinkerWrapper.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp index 63ad6ce730418..9bbb255cd6339 100644 --- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp +++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp @@ -942,7 +942,24 @@ static void addBackendOptions(const ArgList &Args, SmallVector &CmdArgs, bool IsCPU) { StringRef OptC = Args.getLastArgValue(OPT_sycl_backend_compile_options_from_image_EQ); - OptC.split(CmdArgs, " ", /*MaxSplit=*/-1, /*KeepEmpty=*/false); + if (!IsCPU) { + // ocloc -options args need to be comma separated, e.g. `-options + // "-g,-cl-opt-disable"`. Otherwise, only the first arg is processed by + // ocloc as an arg for -options, and the rest are processed as standalone + // flags, possibly leading to errors. + std::pair OptionsArgs = OptC.split("-options "); + // Only add if not empty, an empty arg can lead to ocloc errors. + if (!OptionsArgs.first.empty()) + CmdArgs.push_back(OptionsArgs.first); + if (!OptionsArgs.second.empty()) { + CmdArgs.push_back("-options"); + std::string Replace = OptionsArgs.second.str(); + std::replace(Replace.begin(), Replace.end(), ' ', ','); + CmdArgs.push_back(Args.MakeArgString(Replace)); + } + } else { + OptC.split(CmdArgs, " ", /*MaxSplit=*/-1, /*KeepEmpty=*/false); + } StringRef OptL = Args.getLastArgValue(OPT_sycl_backend_link_options_from_image_EQ); OptL.split(CmdArgs, " ", /*MaxSplit=*/-1, /*KeepEmpty=*/false); From 9949bb813620ad856e92a24ad27e159f89fa6068 Mon Sep 17 00:00:00 2001 From: Marcos Maronas Date: Thu, 2 Oct 2025 20:04:07 +0200 Subject: [PATCH 2/2] Address code review. --- .../ClangLinkerWrapper.cpp | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp index 9bbb255cd6339..9648635dc0d38 100644 --- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp +++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp @@ -942,23 +942,28 @@ static void addBackendOptions(const ArgList &Args, SmallVector &CmdArgs, bool IsCPU) { StringRef OptC = Args.getLastArgValue(OPT_sycl_backend_compile_options_from_image_EQ); - if (!IsCPU) { + if (IsCPU) { + OptC.split(CmdArgs, " ", /*MaxSplit=*/-1, /*KeepEmpty=*/false); + } else { // ocloc -options args need to be comma separated, e.g. `-options // "-g,-cl-opt-disable"`. Otherwise, only the first arg is processed by // ocloc as an arg for -options, and the rest are processed as standalone // flags, possibly leading to errors. - std::pair OptionsArgs = OptC.split("-options "); + // split function here returns a pair with everything before the separator + // ("-options") in the first member of the pair, and everything after the + // separator in the second part of the pair. The separator is not included + // in any of them. + auto [BeforeOptions, AfterOptions] = OptC.split("-options "); // Only add if not empty, an empty arg can lead to ocloc errors. - if (!OptionsArgs.first.empty()) - CmdArgs.push_back(OptionsArgs.first); - if (!OptionsArgs.second.empty()) { + if (!BeforeOptions.empty()) + CmdArgs.push_back(BeforeOptions); + if (!AfterOptions.empty()) { + // Separator not included by the split function, so explicitly added here. CmdArgs.push_back("-options"); - std::string Replace = OptionsArgs.second.str(); + std::string Replace = AfterOptions.str(); std::replace(Replace.begin(), Replace.end(), ' ', ','); CmdArgs.push_back(Args.MakeArgString(Replace)); } - } else { - OptC.split(CmdArgs, " ", /*MaxSplit=*/-1, /*KeepEmpty=*/false); } StringRef OptL = Args.getLastArgValue(OPT_sycl_backend_link_options_from_image_EQ);