Skip to content

[clang-offload-bundler] Add new unbundling mode 'a' for archives #2840

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
merged 2 commits into from
Dec 3, 2020

Conversation

sndmitriev
Copy link
Contributor

This patch adds support for unbundling archives into archives under a new
file type 'a'. Input file for this mode is expected to be an archive with
fat object files and outputs (one per offload target) will be archives
with extracted device specific parts from the input's objects.

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

This patch adds support for unbundling archives into archives under a new
file type 'a'. Input file for this mode is expected to be an archive with
fat object files and outputs (one per offload target) will be archives
with extracted device specific parts from the input's objects.

Signed-off-by: Sergey Dmitriev <serguei.n.dmitriev@intel.com>
Copy link
Contributor

@elizabethandrews elizabethandrews left a comment

Choose a reason for hiding this comment

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

From a quick look, I don't see anything wrong. But I am also unfamiliar with the offload-bundler and so am not the right person to review this.

bader
bader previously approved these changes Dec 1, 2020
Copy link
Contributor

@RaviNarayanaswamy RaviNarayanaswamy left a comment

Choose a reason for hiding this comment

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

Are these changes same as what the llvm openmp is planning to check into open source

@sndmitriev
Copy link
Contributor Author

Are these changes same as what the llvm openmp is planning to check into open source

No, this is a different change. We have added support for unbundling archives in sycl branch long time ago, but we did not have ‘a’ mode where the extracted target objects are expected to be packed back into archives. So, now I am just adding this missing part.

Co-authored-by: Alexey Bader <alexey.bader@intel.com>
@sndmitriev
Copy link
Contributor Author

I believe LIT fails in checkin testing are not related to these changes. The original patch has successfully passed testing, and LIT failures appeared after correcting typo in comments.

@sndmitriev
Copy link
Contributor Author

I think I have addressed all comments now. Is there anything else that need to be done to get this merged?

@bader
Copy link
Contributor

bader commented Dec 3, 2020

"Waiting on code owner review from AGindinson and/or mdtoguchi."

Copy link
Contributor

@mdtoguchi mdtoguchi left a comment

Choose a reason for hiding this comment

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

LGTM

@bader bader changed the title [clang-offload-bundler] add new unbundling mode 'a' for archives [clang-offload-bundler] Add new unbundling mode 'a' for archives Dec 3, 2020
@bader bader merged commit ad05778 into intel:sycl Dec 3, 2020
@sndmitriev sndmitriev deleted the public/sndmitriev/unbundle-archives branch December 22, 2020 02:25
vmaksimo pushed a commit to vmaksimo/llvm that referenced this pull request Jul 5, 2021
Reference commit:
ad05778 [clang-offload-bundler] Add new unbundling mode 'a' for archives (intel#2840)

Also added an override keyword that was missed during eda76af.

  CONFLICT (content): Merge conflict in clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
  CONFLICT (content): Merge conflict in clang/test/Driver/clang-offload-bundler.c
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.

7 participants