From 3f53a40717eaefa79a9cbd6c4c3a7c1e6eb51e4f Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Fri, 24 Apr 2020 17:56:55 -0700 Subject: [PATCH 1/2] [Driver][SYCL] Improve -Xsycl-target option parsing Initial implementation of -Xsycl-target-backend "opts" had a simple parsing of relying on spaces. Any more complex option passing that requires args containing spaces did not pass correctly. Update parsing to be a bit smarter. Signed-off-by: Michael D Toguchi --- clang/lib/Driver/ToolChains/SYCL.cpp | 28 ++++++++++++++-------------- clang/test/Driver/sycl-offload.c | 4 ++++ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/clang/lib/Driver/ToolChains/SYCL.cpp b/clang/lib/Driver/ToolChains/SYCL.cpp index 5d1909643cc39..8a65c24f40085 100644 --- a/clang/lib/Driver/ToolChains/SYCL.cpp +++ b/clang/lib/Driver/ToolChains/SYCL.cpp @@ -13,6 +13,7 @@ #include "clang/Driver/Driver.h" #include "clang/Driver/DriverDiagnostic.h" #include "clang/Driver/Options.h" +#include "llvm/Support/CommandLine.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -421,6 +422,17 @@ SYCLToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, return DAL; } +void parseTargetOpts(StringRef ArgString, const llvm::opt::ArgList &Args, + llvm::opt::ArgStringList &CmdArgs) { + // Tokenize the string. + SmallVector TargetArgs; + llvm::BumpPtrAllocator A; + llvm::StringSaver S(A); + llvm::cl::TokenizeGNUCommandLine(ArgString, S, TargetArgs); + for (StringRef const &TA : TargetArgs) + CmdArgs.push_back(Args.MakeArgString(TA)); +} + // Expects a specific type of option (e.g. -Xsycl-target-backend) and will // extract the arguments. void SYCLToolChain::TranslateTargetOpt(const llvm::opt::ArgList &Args, @@ -455,13 +467,7 @@ void SYCLToolChain::TranslateTargetOpt(const llvm::opt::ArgList &Args, // Triple found, add the next argument in line. ArgString = A->getValue(1); - // Do a simple parse of the args to pass back - SmallVector TargetArgs; - // TODO: Improve parsing, as this only handles arguments separated by - // spaces. - ArgString.split(TargetArgs, ' ', -1, false); - for (const auto &TA : TargetArgs) - CmdArgs.push_back(Args.MakeArgString(TA)); + parseTargetOpts(ArgString, Args, CmdArgs); A->claim(); } } @@ -485,13 +491,7 @@ void SYCLToolChain::TranslateBackendTargetArgs(const llvm::opt::ArgList &Args, } if (A->getOption().matches(options::OPT_Xs_separate)) { StringRef ArgString(A->getValue()); - // Do a simple parse of the args to pass back - SmallVector TargetArgs; - // TODO: Improve parsing, as this only handles arguments separated by - // spaces. - ArgString.split(TargetArgs, ' ', -1, false); - for (const auto &TA : TargetArgs) - CmdArgs.push_back(Args.MakeArgString(TA)); + parseTargetOpts(ArgString, Args, CmdArgs); A->claim(); continue; } diff --git a/clang/test/Driver/sycl-offload.c b/clang/test/Driver/sycl-offload.c index a1705bf9b7625..bccba12995420 100644 --- a/clang/test/Driver/sycl-offload.c +++ b/clang/test/Driver/sycl-offload.c @@ -671,6 +671,10 @@ // CHK-TOOLS-CPU-OPTS: opencl-aot{{.*}} "-DFOO1" "-DFOO2" // CHK-TOOLS-CPU-OPTS-NOT: clang-offload-wrapper{{.*}} "-compile-opts={{.*}} +// RUN: %clang -### -target x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64_x86_64-unknown-unknown-sycldevice -Xsycl-target-backend "--bo='\"-DFOO1 -DFOO2\"'" %s 2>&1 \ +// RUN: | FileCheck -check-prefix=CHK-TOOLS-CPU-OPTS3 %s +// CHK-TOOLS-CPU-OPTS3: opencl-aot{{.*}} "--bo=\"-DFOO1 -DFOO2\"" + // RUN: %clang -### -target x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64-unknown-unknown-sycldevice -Xsycl-target-backend "-DFOO1 -DFOO2" %s 2>&1 \ // RUN: | FileCheck -check-prefix=CHK-TOOLS-OPTS %s // CHK-TOOLS-OPTS: clang-offload-wrapper{{.*}} "-compile-opts=\"-DFOO1 -DFOO2\"" From aed5d83a92a4bee382d50d265f9faf9e1065cc25 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Mon, 27 Apr 2020 08:46:53 -0700 Subject: [PATCH 2/2] [Driver] Address review comments Signed-off-by: Michael D Toguchi --- clang/lib/Driver/ToolChains/SYCL.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/Driver/ToolChains/SYCL.cpp b/clang/lib/Driver/ToolChains/SYCL.cpp index 8a65c24f40085..a4a5f593ecadb 100644 --- a/clang/lib/Driver/ToolChains/SYCL.cpp +++ b/clang/lib/Driver/ToolChains/SYCL.cpp @@ -422,14 +422,14 @@ SYCLToolChain::TranslateArgs(const llvm::opt::DerivedArgList &Args, return DAL; } -void parseTargetOpts(StringRef ArgString, const llvm::opt::ArgList &Args, - llvm::opt::ArgStringList &CmdArgs) { +static void parseTargetOpts(StringRef ArgString, const llvm::opt::ArgList &Args, + llvm::opt::ArgStringList &CmdArgs) { // Tokenize the string. SmallVector TargetArgs; llvm::BumpPtrAllocator A; llvm::StringSaver S(A); llvm::cl::TokenizeGNUCommandLine(ArgString, S, TargetArgs); - for (StringRef const &TA : TargetArgs) + for (StringRef TA : TargetArgs) CmdArgs.push_back(Args.MakeArgString(TA)); }