Skip to content

[SYCL][clang-offload-bundler] Add support for BC files in archives. #11034

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 24 commits into from
Oct 23, 2023
Merged

[SYCL][clang-offload-bundler] Add support for BC files in archives. #11034

merged 24 commits into from
Oct 23, 2023

Conversation

LU-JOHN
Copy link
Contributor

@LU-JOHN LU-JOHN commented Aug 31, 2023

Add support for -list, -check-section, and -unbundle for bundled BC files in archives.

@LU-JOHN LU-JOHN requested a review from a team as a code owner August 31, 2023 14:59
@LU-JOHN LU-JOHN marked this pull request as draft August 31, 2023 15:00
@asudarsa asudarsa changed the title Add support for BC files in archives. [SYCL][clang-offload-bundler] Add support for BC files in archives. Aug 31, 2023
@asudarsa
Copy link
Contributor

Add support for -list, -check-section, and -unbundle for bundled BC files in archives.

Can you please elaborate here on why this change is required? For a generic target audience. Thanks

@LU-JOHN
Copy link
Contributor Author

LU-JOHN commented Aug 31, 2023

Can you please elaborate here on why this change is required? For a generic target audience. Thanks

The ability to maintain the symbols and properties as generated when creating early FPGA images is required when moving from early archives to image archives or executables. This information is retained in the wrapped device image. This image needs to be retained in the FPGA early archive as a bundled wrapped file (wrapped files are .bc files) where the compiler can obtain this wrapped file and process it for final image generation and providing these symbols and properties in the final image of the executable.

@LU-JOHN LU-JOHN marked this pull request as ready for review October 2, 2023 18:36
@LU-JOHN LU-JOHN requested a review from a team as a code owner October 2, 2023 18:36
@LU-JOHN LU-JOHN temporarily deployed to WindowsCILock October 19, 2023 21:29 — with GitHub Actions Inactive
@LU-JOHN LU-JOHN temporarily deployed to WindowsCILock October 19, 2023 23:54 — with GitHub Actions Inactive
@LU-JOHN LU-JOHN temporarily deployed to WindowsCILock October 20, 2023 14:42 — with GitHub Actions Inactive
@LU-JOHN LU-JOHN temporarily deployed to WindowsCILock October 20, 2023 15:52 — with GitHub Actions Inactive
@LU-JOHN LU-JOHN temporarily deployed to WindowsCILock October 20, 2023 16:24 — with GitHub Actions Inactive
@LU-JOHN LU-JOHN temporarily deployed to WindowsCILock October 20, 2023 17:14 — with GitHub Actions Inactive
auto CheckOrErr = CheckIfObjectFileContainsExcludedTargets(C);
if (!CheckOrErr)
return CheckOrErr.takeError();
auto CheckOrErr = CheckIfObjectFileContainsExcludedTargets(C);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @maksimsab

We are trying to add support here for cases where the archive file will contain .bc file. Previously, we handle only those archive files which have .o files.
Can you please comment if we will need this check for archives containing .bc files?

Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know at the moment whether we will need it in the future.
This particular piece of code was related to handling of "FPGA" part. Frankly speaking, we don't have a concrete design for them.

Copy link
Contributor

@asudarsa asudarsa left a comment

Choose a reason for hiding this comment

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

These changes look good to me. Minor tweaks requested.

Thanks

Signed-off-by: Lu, John <[email protected]>
@LU-JOHN LU-JOHN temporarily deployed to WindowsCILock October 23, 2023 16:20 — with GitHub Actions Inactive
@LU-JOHN LU-JOHN temporarily deployed to WindowsCILock October 23, 2023 17:32 — with GitHub Actions Inactive
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.

OK for driver

@againull againull merged commit 89f689e into intel:sycl Oct 23, 2023
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.

5 participants