Skip to content

[New offload model] Cleanup the way sycl-post-link options are generated #14177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 18, 2024
Merged

[New offload model] Cleanup the way sycl-post-link options are generated #14177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
a14cac0
[New offload model] Cleanup the way sycl-post-link options are genera…
asudarsa Jun 2, 2024
ce71e8c
Fix windows test fail
asudarsa Jun 14, 2024
68fa250
Fix tests and move user specified options towards the end of the sycl…
asudarsa Jun 14, 2024
ac744f5
Test update
asudarsa Jun 14, 2024
4f1eb5e
Address review comments
asudarsa Jun 14, 2024
164b5d5
Test updates after latest merge with SYCL branch tip
asudarsa Jun 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 29 additions & 37 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10648,31 +10648,34 @@ static void getNonTripleBasedSYCLPostLinkOpts(const ToolChain &TC,
addArgs(PostLinkArgs, TCArgs, {"-lower-esimd-force-stateless-mem=false"});
}

// Add any sycl-post-link options that rely on a specific Triple.
static void
getTripleBasedSYCLPostLinkOpts(const ToolChain &TC, const JobAction &JA,
const llvm::opt::ArgList &TCArgs,
llvm::Triple Triple, ArgStringList &PostLinkArgs,
bool SpecConsts, types::ID OutputType) {
bool NewOffloadDriver = TC.getDriver().getUseNewOffloadingDriver();
// Note: Do not use Triple when NewOffloadDriver is 'true'.
if (!NewOffloadDriver && (OutputType == types::TY_LLVM_BC)) {
// Add any sycl-post-link options that rely on a specific Triple in addition
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main changes in this function are removal of NewOffloadDriver boolean and some comment updates.

// to user supplied options. This function is invoked only for the old
// offloading model. For the new offloading model, a slightly modified version
// of this function is called inside clang-linker-wrapper.
// NOTE: Any changes made here should be reflected in the similarly named
// function in clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have any plans to make this a single function that both can call somehow? Having two functions that need to be kept in sync is not ideal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on this. I will work with Mike Toguchi to identify a common location for this.

static void getTripleBasedSYCLPostLinkOpts(const ToolChain &TC,
const llvm::opt::ArgList &TCArgs,
ArgStringList &PostLinkArgs,
llvm::Triple Triple,
bool SpecConstsSupported,
types::ID OutputType) {
if (OutputType == types::TY_LLVM_BC) {
// single file output requested - this means only perform necessary IR
// transformations (like specialization constant intrinsic lowering) and
// output LLVMIR
addArgs(PostLinkArgs, TCArgs, {"-ir-output-only"});
}
// specialization constants processing is mandatory
if (SpecConsts)
if (SpecConstsSupported)
addArgs(PostLinkArgs, TCArgs, {"-spec-const=native"});
else
addArgs(PostLinkArgs, TCArgs, {"-spec-const=emulation"});

// See if device code splitting is requested. The logic here works along side
// the behavior in setOtherSYCLPostLinkOpts, where the option is added based
// on the user setting of-fsycl-device-code-split.
// the behavior in getNonTripleBasedSYCLPostLinkOpts, where the option is
// added based on the user setting of -fsycl-device-code-split.
if (!TCArgs.hasArg(options::OPT_fsycl_device_code_split_EQ) &&
(NewOffloadDriver || !(Triple.getArchName() == "spir64_fpga")))
(Triple.getArchName() != "spir64_fpga"))
addArgs(PostLinkArgs, TCArgs, {"-split=auto"});

// On Intel targets we don't need non-kernel functions as entry points,
Expand All @@ -10683,18 +10686,17 @@ getTripleBasedSYCLPostLinkOpts(const ToolChain &TC, const JobAction &JA,
options::OPT_fsycl_remove_unused_external_funcs,
false) &&
!isSYCLNativeCPU(TC)) &&
(NewOffloadDriver || (!Triple.isNVPTX() && !Triple.isAMDGPU())))
!Triple.isNVPTX() && !Triple.isAMDGPU())
addArgs(PostLinkArgs, TCArgs, {"-emit-only-kernels-as-entry-points"});

if (!NewOffloadDriver && !Triple.isAMDGCN())
if (!Triple.isAMDGCN())
addArgs(PostLinkArgs, TCArgs, {"-emit-param-info"});
// Enable program metadata
if ((!NewOffloadDriver && (Triple.isNVPTX() || Triple.isAMDGCN())) ||
isSYCLNativeCPU(TC))
if (Triple.isNVPTX() || Triple.isAMDGCN() || isSYCLNativeCPU(TC))
addArgs(PostLinkArgs, TCArgs, {"-emit-program-metadata"});
if (OutputType != types::TY_LLVM_BC) {
assert(OutputType == types::TY_Tempfiletable);
bool SplitEsimdByDefault = !NewOffloadDriver && Triple.isSPIROrSPIRV();
bool SplitEsimdByDefault = Triple.isSPIROrSPIRV();
bool SplitEsimd = TCArgs.hasFlag(
options::OPT_fsycl_device_code_split_esimd,
options::OPT_fno_sycl_device_code_split_esimd, SplitEsimdByDefault);
Expand All @@ -10714,7 +10716,7 @@ getTripleBasedSYCLPostLinkOpts(const ToolChain &TC, const JobAction &JA,
if (TCArgs.hasFlag(options::OPT_fsycl_add_default_spec_consts_image,
options::OPT_fno_sycl_add_default_spec_consts_image,
false) &&
(IsAOT || NewOffloadDriver))
IsAOT)
addArgs(PostLinkArgs, TCArgs,
{"-generate-device-image-default-spec-consts"});
}
Expand All @@ -10738,7 +10740,7 @@ void SYCLPostLink::ConstructJob(Compilation &C, const JobAction &JA,

llvm::Triple T = getToolChain().getTriple();
getNonTripleBasedSYCLPostLinkOpts(getToolChain(), JA, TCArgs, CmdArgs);
getTripleBasedSYCLPostLinkOpts(getToolChain(), JA, TCArgs, T, CmdArgs,
getTripleBasedSYCLPostLinkOpts(getToolChain(), TCArgs, CmdArgs, T,
SYCLPostLink->getRTSetsSpecConstants(),
SYCLPostLink->getTrueType());

Expand All @@ -10749,8 +10751,6 @@ void SYCLPostLink::ConstructJob(Compilation &C, const JobAction &JA,
if (T.getSubArch() == llvm::Triple::SPIRSubArch_gen && Device.data())
OutputArg = ("intel_gpu_" + Device + "," + OutputArg).str();

addArgs(CmdArgs, TCArgs, {"-o", OutputArg});

const toolchains::SYCLToolChain &TC =
static_cast<const toolchains::SYCLToolChain &>(getToolChain());

Expand All @@ -10759,6 +10759,8 @@ void SYCLPostLink::ConstructJob(Compilation &C, const JobAction &JA,
options::OPT_Xdevice_post_link_EQ,
JA.getOffloadingArch());

addArgs(CmdArgs, TCArgs, {"-o", OutputArg});

// Add input file
assert(Inputs.size() == 1 && Inputs.front().isFilename() &&
"single input file expected");
Expand Down Expand Up @@ -11111,24 +11113,14 @@ void LinkerWrapper::ConstructJob(Compilation &C, const JobAction &JA,
// --sycl-post-link-options="options" provides a string of options to be
// passed along to the sycl-post-link tool during device link.
SmallString<128> PostLinkOptString;
if (Args.hasArg(options::OPT_Xdevice_post_link)) {
for (const auto &A : Args.getAllArgValues(options::OPT_Xdevice_post_link))
appendOption(PostLinkOptString, A);
}
ArgStringList PostLinkArgs;
bool IsSYCLNativeCPU = driver::isSYCLNativeCPU(Args);
types::ID OutputType = TargetTriple.isSPIROrSPIRV() || IsSYCLNativeCPU
? types::TY_Tempfiletable
: types::TY_LLVM_BC;
bool SpecConsts = TargetTriple.isSPIROrSPIRV();
getNonTripleBasedSYCLPostLinkOpts(getToolChain(), JA, Args, PostLinkArgs);
// Some options like -spec-consts=* depend on target triple as well as some
// user options. So, these options are partly computed here and then
// updated inside the clang-linker-wrapper.
getTripleBasedSYCLPostLinkOpts(getToolChain(), JA, Args, TargetTriple,
PostLinkArgs, SpecConsts, OutputType);
for (const auto &A : PostLinkArgs)
appendOption(PostLinkOptString, A);
if (Args.hasArg(options::OPT_Xdevice_post_link)) {
for (const auto &A : Args.getAllArgValues(options::OPT_Xdevice_post_link))
appendOption(PostLinkOptString, A);
}
if (!PostLinkOptString.empty())
CmdArgs.push_back(
Args.MakeArgString("--sycl-post-link-options=" + PostLinkOptString));
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/linker-wrapper-sycl-win.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// CHK-CMDS: "{{.*}}spirv-to-ir-wrapper.exe" {{.*}} -o [[FIRSTLLVMLINKIN:.*]].bc --llvm-spirv-opts=--spirv-preserve-auxdata --llvm-spirv-opts=--spirv-target-env=SPV-IR --llvm-spirv-opts=--spirv-builtin-format=global
// CHK-CMDS-NEXT: "{{.*}}llvm-link.exe" [[FIRSTLLVMLINKIN:.*]].bc -o [[FIRSTLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}llvm-link.exe" -only-needed [[FIRSTLLVMLINKOUT]].bc {{.*}}.bc {{.*}}.bc -o [[SECONDLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}sycl-post-link.exe" SYCL_POST_LINK_OPTIONS{{.*}} -o [[SYCLPOSTLINKOUT:.*]].table [[SECONDLLVMLINKOUT]].bc
// CHK-CMDS-NEXT: "{{.*}}sycl-post-link.exe"{{.*}} SYCL_POST_LINK_OPTIONS -o [[SYCLPOSTLINKOUT:.*]].table [[SECONDLLVMLINKOUT]].bc
// LLVM-SPIRV is not called in dry-run
// CHK-CMDS-NEXT: offload-wrapper: input: [[LLVMSPIRVOUT:.*]].table, output: [[WRAPPEROUT:.*]].bc
// CHK-CMDS-NEXT: "{{.*}}llc.exe" -filetype=obj -o [[LLCOUT:.*]].o [[WRAPPEROUT]].bc
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/linker-wrapper-sycl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
// CHK-CMDS: "{{.*}}spirv-to-ir-wrapper" {{.*}} -o [[FIRSTLLVMLINKIN:.*]].bc --llvm-spirv-opts=--spirv-preserve-auxdata --llvm-spirv-opts=--spirv-target-env=SPV-IR --llvm-spirv-opts=--spirv-builtin-format=global
// CHK-CMDS-NEXT: "{{.*}}llvm-link" [[FIRSTLLVMLINKIN:.*]].bc -o [[FIRSTLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}llvm-link" -only-needed [[FIRSTLLVMLINKOUT]].bc {{.*}}.bc {{.*}}.bc -o [[SECONDLLVMLINKOUT:.*]].bc --suppress-warnings
// CHK-CMDS-NEXT: "{{.*}}sycl-post-link" SYCL_POST_LINK_OPTIONS {{.*}} -o [[SYCLPOSTLINKOUT:.*]].table [[SECONDLLVMLINKOUT]].bc
// CHK-CMDS-NEXT: "{{.*}}sycl-post-link"{{.*}} SYCL_POST_LINK_OPTIONS -o [[SYCLPOSTLINKOUT:.*]].table [[SECONDLLVMLINKOUT]].bc
// LLVM-SPIRV is not called in dry-run
// CHK-CMDS-NEXT: offload-wrapper: input: [[LLVMSPIRVOUT:.*]].table, output: [[WRAPPEROUT:.*]].bc
// CHK-CMDS-NEXT: "{{.*}}llc" -filetype=obj -o [[LLCOUT:.*]].o [[WRAPPEROUT]].bc
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Driver/sycl-offload-new-driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
// RUN: %clangxx --target=x86_64-unknown-linux-gnu -fsycl --offload-new-driver \
// RUN: -Xdevice-post-link -post-link-opt -### %s 2>&1 \
// RUN: | FileCheck -check-prefix WRAPPER_OPTIONS_POSTLINK %s
// WRAPPER_OPTIONS_POSTLINK: clang-linker-wrapper{{.*}} "--sycl-post-link-options=-post-link-opt -O2 -device-globals -spec-const=native -split=auto -emit-only-kernels-as-entry-points -symbols -emit-exported-symbols -emit-imported-symbols -lower-esimd"
// WRAPPER_OPTIONS_POSTLINK: clang-linker-wrapper{{.*}} "--sycl-post-link-options=-O2 -device-globals -post-link-opt"

// -fsycl-device-only behavior
// RUN: %clangxx --target=x86_64-unknown-linux-gnu -fsycl --offload-new-driver \
Expand Down
16 changes: 16 additions & 0 deletions clang/test/Driver/sycl-post-link-options-win.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// REQUIRES: system-windows
/// Verify same set of sycl-post-link options generated for old and new offloading model
// RUN: %clangxx -### --target=x86_64-pc-windows-msvc -fsycl \
// RUN: -Xdevice-post-link -O0 %s 2>&1 \
// RUN: | FileCheck -check-prefix OPTIONS_POSTLINK_JIT_OLD %s
// OPTIONS_POSTLINK_JIT_OLD: sycl-post-link{{.*}} "-O2" "-device-globals" "-spec-const=native" "-split=auto" "-emit-only-kernels-as-entry-points" "-emit-param-info" "-symbols" "-emit-exported-symbols" "-emit-imported-symbols" "-split-esimd" "-lower-esimd" "-O0"

// RUN: %clang -cc1 %s -triple x86_64-pc-windows-msvc -emit-obj -o %t.elf.o
// RUN: clang-offload-packager -o %t.out --image=file=%t.elf.o,kind=sycl,triple=spir64
// RUN: %clang -cc1 %s -triple x86_64-pc-windows-msvc -emit-obj -o %t.o \
// RUN: -fembed-offload-object=%t.out
// RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-pc-windows-msvc \
// RUN: -sycl-device-library-location=%S/Inputs -sycl-device-libraries=libsycl-crt.new.obj \
// RUN: --sycl-post-link-options="-O2 -device-globals -O0" \
// RUN: --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck --check-prefix OPTIONS_POSTLINK_JIT_NEW %s
// OPTIONS_POSTLINK_JIT_NEW: sycl-post-link{{.*}} -spec-const=native -split=auto -emit-only-kernels-as-entry-points -emit-param-info -symbols -emit-exported-symbols -emit-imported-symbols -split-esimd -lower-esimd -O2 -device-globals -O0
16 changes: 16 additions & 0 deletions clang/test/Driver/sycl-post-link-options.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// REQUIRES: system-linux
/// Verify same set of sycl-post-link options generated for old and new offloading model
// RUN: %clangxx --target=x86_64-unknown-linux-gnu -fsycl -### \
// RUN: -Xdevice-post-link -O0 %s 2>&1 \
// RUN: | FileCheck -check-prefix OPTIONS_POSTLINK_JIT_OLD %s
// OPTIONS_POSTLINK_JIT_OLD: sycl-post-link{{.*}} "-O2" "-device-globals" "-spec-const=native" "-split=auto" "-emit-only-kernels-as-entry-points" "-emit-param-info" "-symbols" "-emit-exported-symbols" "-emit-imported-symbols" "-split-esimd" "-lower-esimd" "-O0"

// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.elf.o
// RUN: clang-offload-packager -o %t.out --image=file=%t.elf.o,kind=sycl,triple=spir64
// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o \
// RUN: -fembed-offload-object=%t.out
// RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-unknown-linux-gnu \
// RUN: -sycl-device-library-location=%S/Inputs -sycl-device-libraries=libsycl-crt.new.o \
// RUN: --sycl-post-link-options="-O2 -device-globals -O0" \
// RUN: --linker-path=/usr/bin/ld %t.o -o a.out 2>&1 | FileCheck --check-prefix OPTIONS_POSTLINK_JIT_NEW %s
// OPTIONS_POSTLINK_JIT_NEW: sycl-post-link{{.*}} -spec-const=native -split=auto -emit-only-kernels-as-entry-points -emit-param-info -symbols -emit-exported-symbols -emit-imported-symbols -split-esimd -lower-esimd -O2 -device-globals -O0
142 changes: 66 additions & 76 deletions clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,78 +522,69 @@ static Expected<StringRef> convertSPIRVToIR(StringRef Filename,
return *TempFileOrErr;
}

// Update sycl-post-link options based on target triple.
static void updateCmdArgs(SmallVector<StringRef, 8> &CmdArgs,
llvm::Triple Triple) {
// Get an argument in CmdArgs that contains Str. If there is no such
// argument, an empty argument is returned
auto getArg = [&](const StringRef &Str) {
for (auto Arg : CmdArgs)
if (Arg.contains(Str))
return Arg;
return StringRef("");
};
// Add a new argument Arg to CmdArgs if not present already.
auto addArg = [&](const StringRef &Arg) {
if (getArg(Arg).empty())
CmdArgs.push_back(Arg);
};
// Replace an argument in CmdArgs that contains Str with NewArg. If no such
// argument is present, add the NewArg to CmdArgs.
auto replaceOrAddArg = [&](const StringRef &NewArg, const StringRef &Str) {
for (auto &Arg : CmdArgs)
if (Arg.contains(Str)) {
Arg = NewArg;
return;
}
CmdArgs.push_back(NewArg);
};
// Remove argument containing Str from CmdArgs.
auto removeArg = [&](const StringRef &Str) {
CmdArgs.erase(
std::remove_if(CmdArgs.begin(), CmdArgs.end(),
[&](StringRef Arg) { return Arg.contains(Str); }),
CmdArgs.end());
};

// specialization constants processing.
bool IsAOTGPU = Triple.isNVPTX() || Triple.isAMDGCN() || Triple.isSPIRAOT();
if (!IsAOTGPU)
replaceOrAddArg("-spec-const=native", "-spec-const");
else
replaceOrAddArg("-spec-const=emulation", "-spec-const");

// -emit-only-kernels-as-entry-points is set by the user and is enabled only
// for Intel targets.
auto EmitOnlyKernelsAsEntryPointsArg =
getArg("-emit-only-kernels-as-entry-points");
if ((!EmitOnlyKernelsAsEntryPointsArg.empty()) && !Triple.isNVPTX() &&
!Triple.isAMDGPU())
addArg("-emit-only-kernels-as-entry-points");
else
removeArg("-emit-only-kernels-as-entry-points");

if (!(Triple.isAMDGCN()))
addArg("-emit-param-info");

if (Triple.isNVPTX() || Triple.isAMDGCN())
addArg("-emit-program-metadata");

if (Triple.isSPIROrSPIRV()) {
addArg("-symbols");
addArg("-emit-exported-symbols");
addArg("-split-esimd");
addArg("-lower-esimd");
}

// Here, IsAOT includes x86_64 device as well.
bool IsAOT =
IsAOTGPU || Triple.getSubArch() == llvm::Triple::SPIRSubArch_x86_64;
auto GenDeviceImageArg = getArg("-generate-device-image-default-spec-consts");
if ((!GenDeviceImageArg.empty()) && IsAOT)
addArg("-generate-device-image-default-spec-consts");
// Add any sycl-post-link options that rely on a specific Triple in addition
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed all string parsing logic.

// to user supplied options.
// NOTE: Any changes made here should be reflected in the similarly named
// function in clang/lib/Driver/ToolChains/Clang.cpp.
static void
getTripleBasedSYCLPostLinkOpts(const ArgList &Args,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I might have missed something, but can you remind me why we need two functions, one that cares about the triple and one that doesn't? It seem like it would be simpler if we had a single function because then we won't need to rely on things the previous one did.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's due to how clang-linker-wrapper processes images. clang-linker-wrapper receives a set of images each with its own target triple. So, we do not have a singular target triple at the call site of clang-linker-wrapper. The options which do not depend on the target triple can be computed once and passed into the linker wrapper. The options that depend on the target triple will need to be computed for each device image group ( group corresponds to a target triple) inside the clang-linker-wrapper. There are few other ways to implement this. one option will be to pass all required information to the clang-linker-wrapper and compute all of them in one place. Passing in all the required information seemed overkill. Current approach is agreeable, I Hope.

Thanks

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming it doesnt get too much more complex i think it's okay

SmallVector<StringRef, 8> &PostLinkArgs,
const llvm::Triple Triple) {
const llvm::Triple HostTriple(Args.getLastArgValue(OPT_host_triple_EQ));
bool SYCLNativeCPU = (HostTriple == Triple);
bool SpecConstsSupported = (!Triple.isNVPTX() && !Triple.isAMDGCN() ||
!Triple.isSPIRAOT() && !SYCLNativeCPU);
if (SpecConstsSupported)
PostLinkArgs.push_back("-spec-const=native");
else
removeArg("-generate-device-image-default-spec-consts");
PostLinkArgs.push_back("-spec-const=emulation");

// See if device code splitting is already requested. If not requested, then
// set -split=auto for non-FPGA targets.
bool NoSplit = true;
for (auto Arg : PostLinkArgs)
if (Arg.contains("-split=")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if instead we can rely on the user settings and use Args.hasArg or something rather than a string check based on what the other function did.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one scenario where I felt the current approach is simpler than passing -fsycl-device-code-split option along to the clang-linker-wrapper. It opens up additional issues like passing -fsycl-device-code-split as well as -split=? etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the tradeoff is then we have multiple places changing for the tool option name itself and not the clang-linker-wrapper arg, so if we want to change the tool option or something then we have multiple places to update, but i don't think that's a huge deal

NoSplit = false;
break;
}
if (NoSplit && (Triple.getSubArch() != llvm::Triple::SPIRSubArch_fpga))
PostLinkArgs.push_back("-split=auto");

// On Intel targets we don't need non-kernel functions as entry points,
// because it only increases amount of code for device compiler to handle,
// without any actual benefits.
// TODO: Try to extend this feature for non-Intel GPUs.
if ((!Args.hasFlag(OPT_no_sycl_remove_unused_external_funcs,
OPT_sycl_remove_unused_external_funcs, false) &&
!SYCLNativeCPU) &&
!Triple.isNVPTX() && !Triple.isAMDGPU())
PostLinkArgs.push_back("-emit-only-kernels-as-entry-points");

if (!Triple.isAMDGCN())
PostLinkArgs.push_back("-emit-param-info");
// Enable program metadata
if (Triple.isNVPTX() || Triple.isAMDGCN() || SYCLNativeCPU)
PostLinkArgs.push_back("-emit-program-metadata");

bool SplitEsimdByDefault = Triple.isSPIROrSPIRV();
bool SplitEsimd =
Args.hasFlag(OPT_sycl_device_code_split_esimd,
OPT_no_sycl_device_code_split_esimd, SplitEsimdByDefault);

// Symbol file and specialization constant info generation is mandatory -
// add options unconditionally
PostLinkArgs.push_back("-symbols");
PostLinkArgs.push_back("-emit-exported-symbols");
PostLinkArgs.push_back("-emit-imported-symbols");
if (SplitEsimd)
PostLinkArgs.push_back("-split-esimd");
PostLinkArgs.push_back("-lower-esimd");

bool IsAOT = Triple.isNVPTX() || Triple.isAMDGCN() || Triple.isSPIRAOT();
if (Args.hasFlag(OPT_sycl_add_default_spec_consts_image,
OPT_no_sycl_add_default_spec_consts_image, false) &&
IsAOT)
PostLinkArgs.push_back("-generate-device-image-default-spec-consts");
}

// Run sycl-post-link tool
Expand All @@ -610,16 +601,15 @@ static Expected<StringRef> runSYCLPostLink(ArrayRef<StringRef> InputFiles,
if (!TempFileOrErr)
return TempFileOrErr.takeError();

SmallVector<StringRef, 8> CmdArgs;
CmdArgs.push_back(*SYCLPostLinkPath);
const llvm::Triple Triple(Args.getLastArgValue(OPT_triple_EQ));
getTripleBasedSYCLPostLinkOpts(Args, CmdArgs, Triple);
StringRef SYCLPostLinkOptions;
if (Arg *A = Args.getLastArg(OPT_sycl_post_link_options_EQ))
SYCLPostLinkOptions = A->getValue();

SmallVector<StringRef, 8> CmdArgs;
CmdArgs.push_back(*SYCLPostLinkPath);
SYCLPostLinkOptions.split(CmdArgs, " ", /* MaxSplit = */ -1,
/* KeepEmpty = */ false);
const llvm::Triple Triple(Args.getLastArgValue(OPT_triple_EQ));
updateCmdArgs(CmdArgs, Triple);
CmdArgs.push_back("-o");
CmdArgs.push_back(*TempFileOrErr);
for (auto &File : InputFiles)
Expand Down
Loading
Loading