From a50fd2d233a30c80e300cd6ed29db4cbce64416b Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Tue, 12 Mar 2024 14:16:53 -0700 Subject: [PATCH 1/4] [Driver][SYCL] Address issue with improper bundler call with -fsycl-link The initial change to modify -fsycl-link for AOT introduced a modification to the general toolchain flow that is more aligned with the host link step. This caused a disconnect with the unbundling step from objects where we were unbundling for host and target, but not providing the needed output files. Fix this up to allow for both the host and target output files to be available. There is an additional fix that will be needed to restrict this (i.e. only unbundle the target, and not the host), but functionally what we have in place takes care of the immediate problem at hand. --- clang/lib/Driver/Driver.cpp | 29 +++++++---------------------- clang/test/Driver/sycl-offload.c | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 802937157da89..2504193c76a99 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -5050,7 +5050,7 @@ class OffloadingActionBuilder final { for (auto SDA : SYCLDeviceActions) SYCLLinkBinaryList.push_back(SDA); if (WrapDeviceOnlyBinary) - return ABRT_Success; + return ABRT_Ignore_Host; auto *Link = C.MakeAction(SYCLLinkBinaryList, types::TY_Image); SYCLLinkBinary = @@ -6749,9 +6749,8 @@ class OffloadingActionBuilder final { return false; if (HasFPGATarget && !updateInputForFPGA(A, InputArg, Args)) return false; - // FIXME - unbundling action is being split into two different actions - // when unbundling objects. One action for the host, the other for the - // device. + // FIXME - unbundling action with -fsycl-link is unbunding for both host + // and device, where only the device is needed. auto UnbundlingHostAction = C.MakeAction( A, (HasSPIRTarget && HostAction->getType() == types::TY_Archive) ? types::TY_Tempfilelist @@ -7325,7 +7324,6 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, ExtractAPIJobAction *ExtractAPIAction = nullptr; ActionList LinkerInputs; ActionList MergerInputs; - ActionList DeviceAOTLinkerInputs; ActionList HostActions; llvm::SmallVector LinkerInputArgs; llvm::SmallVector PL; @@ -7362,17 +7360,6 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, if (Phase == phases::Link) { assert(Phase == PL.back() && "linking must be final compilation step."); - // When performing -fsycl-link the current inputs are not expected to - // be passed to the final host link step. Instead, take these inputs - // and redirect them to the associated wrapping step to create the - // final object. - if (C.getInputArgs().hasArg(options::OPT_fsycl_link_EQ) && - !Args.hasArg(options::OPT_fintelfpga)) { - DeviceAOTLinkerInputs.push_back(Current); - Current = nullptr; - break; - } - // We don't need to generate additional link commands if emitting AMD // bitcode or compiling only for the offload device if (!(C.getInputArgs().hasArg(options::OPT_hip_link) && @@ -7515,11 +7502,7 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, Arg *FinalPhaseArg; if (!UseNewOffloadingDriver && getFinalPhase(Args, &FinalPhaseArg) == phases::Link) { - if (LinkerInputs.empty() && DeviceAOTLinkerInputs.empty()) - OffloadBuilder->appendDeviceLinkActions(Actions); - - if (!DeviceAOTLinkerInputs.empty() && - Args.hasArg(options::OPT_fsycl_link_EQ) && + if (Args.hasArg(options::OPT_fsycl_link_EQ) && !Args.hasArg(options::OPT_fintelfpga)) { ActionList LAList; OffloadBuilder->makeHostLinkDeviceOnlyAction(LAList); @@ -7528,7 +7511,8 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args, LA = OffloadBuilder->processHostLinkAction(LA); Actions.push_back(LA); } - } + } else if (LinkerInputs.empty()) + OffloadBuilder->appendDeviceLinkActions(Actions); } if (!LinkerInputs.empty()) { @@ -8999,6 +8983,7 @@ InputInfoList Driver::BuildJobsForActionNoCache( InputInfo CurI; bool IsFPGAObjLink = (JA->getType() == types::TY_Object && + EffectiveTriple.getSubArch() == llvm::Triple::SPIRSubArch_fpga && C.getInputArgs().hasArg(options::OPT_fsycl_link_EQ)); if (C.getDriver().getOffloadStaticLibSeen() && (JA->getType() == types::TY_Archive || diff --git a/clang/test/Driver/sycl-offload.c b/clang/test/Driver/sycl-offload.c index eaa8e86f0e270..8dfdf583e99cd 100644 --- a/clang/test/Driver/sycl-offload.c +++ b/clang/test/Driver/sycl-offload.c @@ -326,6 +326,29 @@ // CHK-LINK-UB: 10: backend, {9}, assembler, (host-sycl) // CHK-LINK-UB: 11: assembler, {10}, object, (host-sycl) +/// Check -fsycl-link tool calls +// RUN: %clangxx -### --target=x86_64-unknown-linux-gnu -fsycl -o %t.out \ +// RUN: -fsycl-targets=spir64_gen -fsycl-link \ +// RUN: -fno-sycl-device-lib=all %t.o 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHK-FSYCL-LINK-UB,CHK-FSYCL-LINK-UB-LIN %s +// RUN: %clang_cl -### --target=x86_64-pc-windows-msvc -fsycl -o %t.out \ +// RUN: -fsycl-targets=spir64_gen -fsycl-link \ +// RUN: -fno-sycl-device-lib=all %t.o 2>&1 \ +// RUN: | FileCheck -check-prefixes=CHK-FSYCL-LINK-UB,CHK-FSYCL-LINK-UB-WIN %s +// CHK-FSYCL-LINK-UB: clang-offload-bundler{{.*}} "-type=o" "-targets=host{{.*}},sycl-spir64_gen-unknown-unknown" "-input=[[INPUT:.+\.o]]" "-output={{.*}}" "-output=[[DEVICE_O:.+]]" "-unbundle" +// CHK-FSYCL-LINK-UB: spirv-to-ir-wrapper{{.*}} "[[DEVICE_O]]" "-o" "[[DEVICE_BC:.+\.bc]]" +// CHK-FSYCL-LINK-UB-LIN: llvm-link{{.*}} "[[DEVICE_BC]]" "-o" "[[LINKED_BC:.+\.bc]]" +// CHK-FSYCL-LINK-UB-LIN: llvm-link{{.*}} "-only-needed" "[[LINKED_BC]]"{{.*}} "[[FINAL_BC:.+\.bc]]" +// CHK-FSYCL-LINK-UB-WIN: llvm-link{{.*}} "[[DEVICE_BC]]" "-o" "[[FINAL_BC:.+\.bc]]" +// CHK-FSYCL-LINK-UB: sycl-post-link{{.*}} "-o" "[[POST_LINK_TABLE:.+\.table]]" "[[FINAL_BC]]" +// CHK-FSYCL-LINK-UB: file-table-tform{{.*}} "-o" "[[TFORM_TABLE:.+\.txt]]" "[[POST_LINK_TABLE]]" +// CHK-FSYCL-LINK-UB: llvm-spirv{{.*}} "-o" "[[SPIRV:.+\.txt]]"{{.*}} "[[TFORM_TABLE]]" +// CHK-FSYCL-LINK-UB-LIN: ocloc{{.*}} "-output" "[[OCLOC_OUT:.+\.out]] +// CHK-FSYCL-LINK-UB-WIN: ocloc{{.*}} "-output" "[[OCLOC_OUT:.+\.exe]] +// CHK-FSYCL-LINK-UB: file-table-tform{{.*}} "-o" "[[TFORM_TABLE2:.+\.table]]" "[[POST_LINK_TABLE]]" "[[OCLOC_OUT]]" +// CHK-FSYCL-LINK-UB: clang-offload-wrapper{{.*}} "-o" "[[WRAPPER_OUT:.+\.bc]]"{{.*}} "-batch" "[[TFORM_TABLE2]]" +// CHK-FSYCL-LINK-UB: clang{{.*}} "-cc1"{{.*}} "-o" "{{.*}}.out" "-x" "ir" "[[WRAPPER_OUT]]" + /// Check -fsycl-link AOT unbundle // RUN: %clang -### -ccc-print-phases -target x86_64-unknown-linux-gnu \ // RUN: -fsycl -o %t.out -fsycl-link -fno-sycl-instrument-device-code \ From 475701b5ae6ec145934ea306ca2410cb00591fb8 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Tue, 12 Mar 2024 15:19:56 -0700 Subject: [PATCH 2/4] Fix test, should not be dependent on device libs --- clang/test/Driver/sycl-offload.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/clang/test/Driver/sycl-offload.c b/clang/test/Driver/sycl-offload.c index 8dfdf583e99cd..09a1fda74a880 100644 --- a/clang/test/Driver/sycl-offload.c +++ b/clang/test/Driver/sycl-offload.c @@ -337,10 +337,8 @@ // RUN: | FileCheck -check-prefixes=CHK-FSYCL-LINK-UB,CHK-FSYCL-LINK-UB-WIN %s // CHK-FSYCL-LINK-UB: clang-offload-bundler{{.*}} "-type=o" "-targets=host{{.*}},sycl-spir64_gen-unknown-unknown" "-input=[[INPUT:.+\.o]]" "-output={{.*}}" "-output=[[DEVICE_O:.+]]" "-unbundle" // CHK-FSYCL-LINK-UB: spirv-to-ir-wrapper{{.*}} "[[DEVICE_O]]" "-o" "[[DEVICE_BC:.+\.bc]]" -// CHK-FSYCL-LINK-UB-LIN: llvm-link{{.*}} "[[DEVICE_BC]]" "-o" "[[LINKED_BC:.+\.bc]]" -// CHK-FSYCL-LINK-UB-LIN: llvm-link{{.*}} "-only-needed" "[[LINKED_BC]]"{{.*}} "[[FINAL_BC:.+\.bc]]" -// CHK-FSYCL-LINK-UB-WIN: llvm-link{{.*}} "[[DEVICE_BC]]" "-o" "[[FINAL_BC:.+\.bc]]" -// CHK-FSYCL-LINK-UB: sycl-post-link{{.*}} "-o" "[[POST_LINK_TABLE:.+\.table]]" "[[FINAL_BC]]" +// CHK-FSYCL-LINK-UB: llvm-link{{.*}} "[[DEVICE_BC]]" +// CHK-FSYCL-LINK-UB: sycl-post-link{{.*}} "-o" "[[POST_LINK_TABLE:.+\.table]]" // CHK-FSYCL-LINK-UB: file-table-tform{{.*}} "-o" "[[TFORM_TABLE:.+\.txt]]" "[[POST_LINK_TABLE]]" // CHK-FSYCL-LINK-UB: llvm-spirv{{.*}} "-o" "[[SPIRV:.+\.txt]]"{{.*}} "[[TFORM_TABLE]]" // CHK-FSYCL-LINK-UB-LIN: ocloc{{.*}} "-output" "[[OCLOC_OUT:.+\.out]] From 7d8a6f871c0b482c72bd22a1697ca383e8994af4 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Thu, 14 Mar 2024 07:48:09 -0700 Subject: [PATCH 3/4] Fix spelling error in comment --- clang/lib/Driver/Driver.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 2504193c76a99..bfc7f828d3e43 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -6749,7 +6749,7 @@ class OffloadingActionBuilder final { return false; if (HasFPGATarget && !updateInputForFPGA(A, InputArg, Args)) return false; - // FIXME - unbundling action with -fsycl-link is unbunding for both host + // FIXME - unbundling action with -fsycl-link is unbundling for both host // and device, where only the device is needed. auto UnbundlingHostAction = C.MakeAction( A, (HasSPIRTarget && HostAction->getType() == types::TY_Archive) From c4ff28e7f4a539a75852ee6153e6d83df1428d58 Mon Sep 17 00:00:00 2001 From: Michael D Toguchi Date: Tue, 19 Mar 2024 09:07:52 -0700 Subject: [PATCH 4/4] Fix problem with test found in Windows testing, missing end quote in var --- clang/test/Driver/sycl-offload.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/test/Driver/sycl-offload.c b/clang/test/Driver/sycl-offload.c index 09a1fda74a880..636d9e89b8092 100644 --- a/clang/test/Driver/sycl-offload.c +++ b/clang/test/Driver/sycl-offload.c @@ -341,8 +341,8 @@ // CHK-FSYCL-LINK-UB: sycl-post-link{{.*}} "-o" "[[POST_LINK_TABLE:.+\.table]]" // CHK-FSYCL-LINK-UB: file-table-tform{{.*}} "-o" "[[TFORM_TABLE:.+\.txt]]" "[[POST_LINK_TABLE]]" // CHK-FSYCL-LINK-UB: llvm-spirv{{.*}} "-o" "[[SPIRV:.+\.txt]]"{{.*}} "[[TFORM_TABLE]]" -// CHK-FSYCL-LINK-UB-LIN: ocloc{{.*}} "-output" "[[OCLOC_OUT:.+\.out]] -// CHK-FSYCL-LINK-UB-WIN: ocloc{{.*}} "-output" "[[OCLOC_OUT:.+\.exe]] +// CHK-FSYCL-LINK-UB-LIN: ocloc{{.*}} "-output" "[[OCLOC_OUT:.+\.out]]" +// CHK-FSYCL-LINK-UB-WIN: ocloc{{.*}} "-output" "[[OCLOC_OUT:.+\.exe]]" // CHK-FSYCL-LINK-UB: file-table-tform{{.*}} "-o" "[[TFORM_TABLE2:.+\.table]]" "[[POST_LINK_TABLE]]" "[[OCLOC_OUT]]" // CHK-FSYCL-LINK-UB: clang-offload-wrapper{{.*}} "-o" "[[WRAPPER_OUT:.+\.bc]]"{{.*}} "-batch" "[[TFORM_TABLE2]]" // CHK-FSYCL-LINK-UB: clang{{.*}} "-cc1"{{.*}} "-o" "{{.*}}.out" "-x" "ir" "[[WRAPPER_OUT]]"