Skip to content

Commit

Permalink
[Driver][SYCL] Address issue with improper bundler call with -fsycl-l…
Browse files Browse the repository at this point in the history
…ink (#13002)

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.
  • Loading branch information
mdtoguchi authored Mar 19, 2024
1 parent 161c25d commit c63b49d
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 22 deletions.
29 changes: 7 additions & 22 deletions clang/lib/Driver/Driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5048,7 +5048,7 @@ class OffloadingActionBuilder final {
SYCLDeviceActions.clear();

if (WrapDeviceOnlyBinary)
return ABRT_Success;
return ABRT_Ignore_Host;
auto *Link =
C.MakeAction<LinkJobAction>(SYCLLinkBinaryList, types::TY_Image);
SYCLLinkBinary =
Expand Down Expand Up @@ -6747,9 +6747,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 unbundling for both host
// and device, where only the device is needed.
auto UnbundlingHostAction = C.MakeAction<OffloadUnbundlingJobAction>(
A, (HasSPIRTarget && HostAction->getType() == types::TY_Archive)
? types::TY_Tempfilelist
Expand Down Expand Up @@ -7323,7 +7322,6 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
ExtractAPIJobAction *ExtractAPIAction = nullptr;
ActionList LinkerInputs;
ActionList MergerInputs;
ActionList DeviceAOTLinkerInputs;
ActionList HostActions;
llvm::SmallVector<const Arg *, 6> LinkerInputArgs;
llvm::SmallVector<phases::ID, phases::MaxNumberOfPhases> PL;
Expand Down Expand Up @@ -7360,17 +7358,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) &&
Expand Down Expand Up @@ -7516,11 +7503,7 @@ void Driver::BuildActions(Compilation &C, DerivedArgList &Args,
if (!UseNewOffloadingDriver &&
getFinalPhase(Args, &FinalPhaseArg) == phases::Link &&
!Args.hasArg(options::OPT_fsycl_link_targets_EQ)) {
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);
Expand All @@ -7529,7 +7512,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()) {
Expand Down Expand Up @@ -9017,6 +9001,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 ||
Expand Down
21 changes: 21 additions & 0 deletions clang/test/Driver/sycl-offload.c
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,27 @@
// 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: 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]]"
// 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 \
Expand Down

0 comments on commit c63b49d

Please sign in to comment.