From 4aeb0ee3a22e8a19c07257055d0f86471373d040 Mon Sep 17 00:00:00 2001 From: Artem Gindinson Date: Thu, 2 Sep 2021 12:53:19 +0300 Subject: [PATCH 1/3] [SYCL][Driver] Fix autodetection of the -Xsycl-target-backend triple https://github.com/intel/llvm/pull/4175 introduced automatic addition of the generic spir64 device target when any section of the input objects had this triple assigned to it. As a result, the actual list of toolchains started exceeding the user-provided one by 1 item. After https://github.com/intel/llvm/pull/4239, the above became a problem. The dispatch of -Xsycl-target-* arguments started happening earlier in theflow, which broke the following use-case: ``` clang++ -fsycl -fsycl-targets=spir64_gen gen-obj.o gen-and-spir64-obj.o -Xsycl-target-backend "-device *" ``` A fix for now is to ignore the autodetected spir64 target when propagating the -Xsycl-target-backend arguments. A permanent solution would involve a re-design of -Xsycl-target-backend handling so that it took place only once in the flow, or belating the addition of the autodetected generic triple into the list of device targets. Signed-off-by: Artem Gindinson --- clang/lib/Driver/Driver.cpp | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 6c59d84488458..7a8081e5d5e96 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4676,13 +4676,28 @@ class OffloadingActionBuilder final { else continue; } else if (A->getOption().matches(options::OPT_Xsycl_backend)) { - if (SYCLTripleList.size() > 1) { + // While the user may have provided a single AOT triple, the generic + // spir64 triple could have been added automatically by us upon + // discovering a spir64 section in one of the object/library inputs. + // To allow for automatic detection of the toolchain that needs to + // receive -Xsycl-target-* arguments, drop the autodetected spir64 + // triple from the local copy of the targets list. + // FIXME: Is there a way to add the autodetected generic triple at + // a later stage instead? + auto UserSYCLTripleList = SYCLTripleList; + if (C.getDriver().isSYCLDefaultTripleImplied()) + UserSYCLTripleList.erase( + llvm::remove_if(UserSYCLTripleList, [](llvm::Triple TT) { + return TT.isSPIR() && + TT.getSubArch() == llvm::Triple::NoSubArch; + })); + if (UserSYCLTripleList.size() > 1) { C.getDriver().Diag(diag::err_drv_Xsycl_target_missing_triple) << A->getSpelling(); continue; } // Passing device args: -Xsycl-target-backend -opt=val. - TargetBE = &SYCLTripleList.front(); + TargetBE = &UserSYCLTripleList.front(); Index = Args.getBaseArgs().MakeIndex(A->getValue(0)); } else continue; From 3f58f7a63d2ad437e5256981b707d14e13cdcee6 Mon Sep 17 00:00:00 2001 From: Artem Gindinson Date: Thu, 2 Sep 2021 16:13:17 +0300 Subject: [PATCH 2/3] Add LIT coverage Signed-off-by: Artem Gindinson --- clang/test/Driver/sycl-offload.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/clang/test/Driver/sycl-offload.cpp b/clang/test/Driver/sycl-offload.cpp index c9cf9202273a6..e7ea6fbc11d96 100644 --- a/clang/test/Driver/sycl-offload.cpp +++ b/clang/test/Driver/sycl-offload.cpp @@ -97,9 +97,18 @@ // NO_IMPLIED_DEVICE: clang-offload-bundler{{.*}} "-type=o" "-targets=sycl-spir64-unknown-unknown"{{.*}} "-check-section" // NO_IMPLIED_DEVICE-NOT: clang-offload-bundler{{.*}} "-targets={{.*}}spir64-unknown-unknown{{.*}}" "-unbundle" -/// Passing in the default triple should allow for -Xsycl-target options +/// Passing in the default triple should allow for -Xsycl-target options, both the +/// "=" and the default spelling // RUN: %clangxx -### -target x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64 -Xsycl-target-backend=spir64 -DFOO -Xsycl-target-linker=spir64 -DFOO2 %S/Inputs/SYCL/objlin64.o 2>&1 \ // RUN: | FileCheck -check-prefixes=SYCL_TARGET_OPT %s // RUN: %clangxx -### -target x86_64-unknown-linux-gnu -fsycl -Xsycl-target-backend=spir64 -DFOO -Xsycl-target-linker=spir64 -DFOO2 %S/Inputs/SYCL/objlin64.o 2>&1 \ // RUN: | FileCheck -check-prefixes=SYCL_TARGET_OPT %s // SYCL_TARGET_OPT: clang-offload-wrapper{{.*}} "-compile-opts=-DFOO" "-link-opts=-DFOO2" +// RUN: %clangxx -### -target x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64_x86_64 -Xsycl-target-backend -DFOO %S/Inputs/SYCL/objlin64.o 2>&1 \ +// RUN: | FileCheck -check-prefixes=SYCL_TARGET_OPT_AOT %s +// RUN: %clangxx -### -target x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64_gen -Xsycl-target-backend -DFOO %S/Inputs/SYCL/objlin64.o 2>&1 \ +// RUN: | FileCheck -check-prefixes=SYCL_TARGET_OPT_AOT %s +// RUN: %clangxx -### -target x86_64-unknown-linux-gnu -fsycl -fsycl-targets=spir64_fpga -Xsycl-target-backend -DFOO %S/Inputs/SYCL/objlin64.o 2>&1 \ +// RUN: | FileCheck -check-prefixes=SYCL_TARGET_OPT_AOT %s +// SYCL_TARGET_OPT_AOT-NOT: error: cannot deduce implicit triple value for '-Xsycl-target-backend' +// SYCL_TARGET_OPT_AOT: {{opencl-aot|ocloc|aoc}}{{.*}} "-DFOO" From 902376d4876edb78bb382f3a167e2ccff796f21e Mon Sep 17 00:00:00 2001 From: Artem Gindinson Date: Fri, 3 Sep 2021 09:57:59 +0300 Subject: [PATCH 3/3] Rework to add the default triple after GPU Arch initialization Signed-off-by: Artem Gindinson --- clang/lib/Driver/Driver.cpp | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 3cb30b48a3996..ab34c289df57c 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4686,28 +4686,13 @@ class OffloadingActionBuilder final { else continue; } else if (A->getOption().matches(options::OPT_Xsycl_backend)) { - // While the user may have provided a single AOT triple, the generic - // spir64 triple could have been added automatically by us upon - // discovering a spir64 section in one of the object/library inputs. - // To allow for automatic detection of the toolchain that needs to - // receive -Xsycl-target-* arguments, drop the autodetected spir64 - // triple from the local copy of the targets list. - // FIXME: Is there a way to add the autodetected generic triple at - // a later stage instead? - auto UserSYCLTripleList = SYCLTripleList; - if (C.getDriver().isSYCLDefaultTripleImplied()) - UserSYCLTripleList.erase( - llvm::remove_if(UserSYCLTripleList, [](llvm::Triple TT) { - return TT.isSPIR() && - TT.getSubArch() == llvm::Triple::NoSubArch; - })); - if (UserSYCLTripleList.size() > 1) { + if (SYCLTripleList.size() > 1) { C.getDriver().Diag(diag::err_drv_Xsycl_target_missing_triple) << A->getSpelling(); continue; } // Passing device args: -Xsycl-target-backend -opt=val. - TargetBE = &UserSYCLTripleList.front(); + TargetBE = &SYCLTripleList.front(); Index = Args.getBaseArgs().MakeIndex(A->getValue(0)); } else continue; @@ -4791,6 +4776,7 @@ class OffloadingActionBuilder final { bool HasValidSYCLRuntime = C.getInputArgs().hasFlag( options::OPT_fsycl, options::OPT_fno_sycl, false); bool SYCLfpgaTriple = false; + bool ShouldAddDefaultTriple = true; if (SYCLTargets || SYCLAddTargets) { if (SYCLTargets) { llvm::StringMap FoundNormalizedTriples; @@ -4811,7 +4797,6 @@ class OffloadingActionBuilder final { if (TT.getSubArch() == llvm::Triple::SPIRSubArch_fpga) SYCLfpgaTriple = true; } - addSYCLDefaultTriple(C, SYCLTripleList); } if (SYCLAddTargets) { for (StringRef Val : SYCLAddTargets->getValues()) { @@ -4825,6 +4810,7 @@ class OffloadingActionBuilder final { // populate the AOT binary inputs vector. SYCLAOTInputs.push_back(std::make_pair(TT, TF)); + ShouldAddDefaultTriple = false; } } } else if (HasValidSYCLRuntime) { @@ -4834,7 +4820,6 @@ class OffloadingActionBuilder final { const char *SYCLTargetArch = SYCLfpga ? "spir64_fpga" : "spir64"; SYCLTripleList.push_back( C.getDriver().MakeSYCLDeviceTriple(SYCLTargetArch)); - addSYCLDefaultTriple(C, SYCLTripleList); if (SYCLfpga) SYCLfpgaTriple = true; } @@ -4871,7 +4856,10 @@ class OffloadingActionBuilder final { } DeviceLinkerInputs.resize(ToolChains.size()); - return initializeGpuArchMap(); + bool GpuInitHasErrors = initializeGpuArchMap(); + if (ShouldAddDefaultTriple) + addSYCLDefaultTriple(C, SYCLTripleList); + return GpuInitHasErrors; } bool canUseBundlerUnbundler() const override {