From 8d6239b55ff4fb40867226d03d419c352212d48e Mon Sep 17 00:00:00 2001 From: Sergey Dmitriev Date: Sun, 20 Dec 2020 20:51:24 -0800 Subject: [PATCH 1/2] [Driver] Let all offload builders add inputs to host link job action This patch fixes a problem with loosing OpenMP device binary when program uses both OpenMP and SYCL offloading models. Signed-off-by: Sergey Dmitriev --- clang/lib/Driver/Driver.cpp | 12 +++++------- clang/test/Driver/openmp-sycl-interop.c | 11 +++++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) create mode 100644 clang/test/Driver/openmp-sycl-interop.c diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index d7f7abe93101d..3a8b0fdb2ca38 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4780,7 +4780,7 @@ class OffloadingActionBuilder final { return false; } - Action* makeHostLinkAction() { + void makeHostLinkAction(ActionList &LinkerInputs) { // Build a list of device linking actions. ActionList DeviceAL; for (DeviceActionBuilder *SB : SpecializedBuilders) { @@ -4790,16 +4790,15 @@ class OffloadingActionBuilder final { } if (DeviceAL.empty()) - return nullptr; + return; // Let builders add host linking actions. - Action* HA; for (DeviceActionBuilder *SB : SpecializedBuilders) { if (!SB->isValid()) continue; - HA = SB->appendLinkHostActions(DeviceAL); + if (Action *HA = SB->appendLinkHostActions(DeviceAL)) + LinkerInputs.push_back(HA); } - return HA; } /// Processes the host linker action. This currently consists of replacing it @@ -5184,8 +5183,7 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, // Add a link action if necessary. if (!LinkerInputs.empty()) { - if (Action *Wrapper = OffloadBuilder.makeHostLinkAction()) - LinkerInputs.push_back(Wrapper); + OffloadBuilder.makeHostLinkAction(LinkerInputs); types::ID LinkType(types::TY_Image); if (Args.hasArg(options::OPT_fsycl_link_EQ)) LinkType = types::TY_Archive; diff --git a/clang/test/Driver/openmp-sycl-interop.c b/clang/test/Driver/openmp-sycl-interop.c new file mode 100644 index 0000000000000..d26089d57ff57 --- /dev/null +++ b/clang/test/Driver/openmp-sycl-interop.c @@ -0,0 +1,11 @@ +/// Check that OpenMP and SYCL device binaries are wrapped and linked to the host +/// image when program uses both OpenMP and SYCL offloading models. + +// REQUIRES: clang-driver +// REQUIRES: x86-registered-target + +// RUN: touch %t.o +// RUN: %clang --target=x86_64-host-linux-gnu -fsycl -fopenmp -fopenmp-targets=x86_64-device-linux-gnu -### %t.o 2>&1 \ +// RUN: | FileCheck %s +// CHECK: clang-offload-wrapper{{(.exe)?}}" {{.*}} "-target=spir64" "-kind=sycl" +// CHECK: clang-offload-wrapper{{(.exe)?}}" {{.*}} "-kind=openmp" "-target=x86_64-device-linux-gnu" From fecb365cf229fa1f5e1256d3bb7d3e0ec922f70c Mon Sep 17 00:00:00 2001 From: Sergey Dmitriev Date: Mon, 21 Dec 2020 07:04:51 -0800 Subject: [PATCH 2/2] Addressed review comment Signed-off-by: Sergey Dmitriev --- clang/test/Driver/openmp-sycl-interop.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/clang/test/Driver/openmp-sycl-interop.c b/clang/test/Driver/openmp-sycl-interop.c index d26089d57ff57..cd9b8068d145e 100644 --- a/clang/test/Driver/openmp-sycl-interop.c +++ b/clang/test/Driver/openmp-sycl-interop.c @@ -7,5 +7,8 @@ // RUN: touch %t.o // RUN: %clang --target=x86_64-host-linux-gnu -fsycl -fopenmp -fopenmp-targets=x86_64-device-linux-gnu -### %t.o 2>&1 \ // RUN: | FileCheck %s -// CHECK: clang-offload-wrapper{{(.exe)?}}" {{.*}} "-target=spir64" "-kind=sycl" -// CHECK: clang-offload-wrapper{{(.exe)?}}" {{.*}} "-kind=openmp" "-target=x86_64-device-linux-gnu" +// CHECK: clang-offload-wrapper{{(.exe)?}}" {{.*}}"-o=[[SYCLBC:.+\.bc]]" {{.*}}"-target=spir64" "-kind=sycl" +// CHECK: llc{{.*}}" {{.*}}"-o" "[[SYCLOBJ:.+]]" "[[SYCLBC]]" +// CHECK: clang-offload-wrapper{{(.exe)?}}" {{.*}}"-o" "[[OMPBC:.*\.bc]]" {{.*}}"-kind=openmp" "-target=x86_64-device-linux-gnu" +// CHECK: clang{{.*}}" {{.*}}"-o" "[[OMPOBJ:.+]]" "-x" "ir" "[[OMPBC]]" +// CHECK: ld{{.*}}" "[[OMPOBJ]]" "[[SYCLOBJ]]"