Skip to content

[SYCL] Link only needed symbols from fat static libraries on device side #2970

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.

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

Conversation

sndmitriev
Copy link
Contributor

This patch changes SYCL linking to link only needed symbols from unbundled
fat static libraries instead of linking whole device archive. This is the
last change in the series of patches for improving offload linking process.

Signed-off-by: Sergey Dmitriev serguei.n.dmitriev@intel.com

This patch changes SYCL linking to link only needed symbols from unbundled
fat static libraries instead of linking whole device archive. This is the
last change in the series of patches for improving offload linking process.

Signed-off-by: Sergey Dmitriev <serguei.n.dmitriev@intel.com>
Co-authored-by: Artem Gindinson <artem.gindinson@intel.com>
AGindinson
AGindinson previously approved these changes Dec 30, 2020
Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

LGTM overall.

Comment on lines 152 to 165
auto AddLinkCommand = [this, &C, &JA, Exec](const char *Output,
const ArgStringList &Inputs,
const ArgStringList &Options) {
ArgStringList CmdArgs;
copy(Options, std::back_inserter(CmdArgs));
copy(Inputs, std::back_inserter(CmdArgs));
CmdArgs.push_back("-o");
CmdArgs.push_back(Output);
// TODO: temporary workaround for a problem with warnings reported by
// llvm-link when driver links LLVM modules with empty modules
CmdArgs.push_back("--suppress-warnings");
C.addCommand(std::make_unique<Command>(
JA, *this, ResponseFileSupport::AtFileUTF8(), Exec, CmdArgs, None));
};
Copy link
Contributor

@AGindinson AGindinson Dec 30, 2020

Choose a reason for hiding this comment

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

Nit & optional: the lambda could be made more concise if refactored to return CmdArgs - Compilation::addCommand() could be called at the main function's scope with the lambda call passed as a corresponding argument. This should avoid extra copying & allow reducing the closure fields' number (to 0, basically).

Signed-off-by: Sergey Dmitriev <serguei.n.dmitriev@intel.com>
@romanovvlad romanovvlad merged commit 5c3e538 into intel:sycl Dec 30, 2020
@sndmitriev sndmitriev deleted the public/sndmitriev/link-only-needed-archives branch January 4, 2021 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants