Skip to content

[SYCL][thinLTO] Split early when thinLTO is enabled #14259

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 4 commits into from
Jun 26, 2024

Conversation

sarnex
Copy link
Contributor

@sarnex sarnex commented Jun 21, 2024

This change implements driver changes to call sycl-post-link during -c when thinLTO is enabled, and updates clang-offload-packager to accept response files (same format as temp file list), which will be generated when we call clang on each output of sycl-post-link.

I will add a test for the clang-offload-packager changes later, because we rely on changes in other PRs to test the full flow. However, the code is already being hit because we compile libdevice with thinLTO when compiling for the new offload model.

For a given file a.cpp that results in two total splits, the following will happen:

  1. We call sycl-post-link on a.bc
  2. We extract the "Code" column from the table output of sycl-post-link
  3. We call llvm-foreach with the regular backend job that would be called normally on each bc file from 2)
  4. We call clang-offload-packager with the output of 3 which is a list of the bc files from each clang run, with a '@' appended to the file path
  5. clang-offload-packager reads each entry inside 4) and adds each to the fat object
  6. clang-linker-wrapper extracts the fat object from 5) and finds all of the files
  7. ... to be implemented

@sarnex sarnex temporarily deployed to WindowsCILock June 21, 2024 22:37 — with GitHub Actions Inactive
@sarnex sarnex temporarily deployed to WindowsCILock June 21, 2024 23:11 — with GitHub Actions Inactive
@sarnex sarnex marked this pull request as ready for review June 24, 2024 16:54
@sarnex sarnex requested review from a team as code owners June 24, 2024 16:54
Signed-off-by: Sarnie, Nick <[email protected]>
@sarnex sarnex temporarily deployed to WindowsCILock June 25, 2024 20:37 — with GitHub Actions Inactive
@asudarsa
Copy link
Contributor

Hi @sarnex

Thanks for adding this change. Will it be possible to provide a short write-up that showcases a small example and show how the compilation proceeds with the changes here?

Thanks

@sarnex
Copy link
Contributor Author

sarnex commented Jun 25, 2024

@asudarsa just added this to the commit description, thanks!

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.

All changes outside 'Driver.cpp' looks good to me. Will let @mdtoguchi comment on that part.

Thanks Nick.

@sarnex sarnex temporarily deployed to WindowsCILock June 25, 2024 21:12 — with GitHub Actions Inactive
Signed-off-by: Sarnie, Nick <[email protected]>
@sarnex sarnex temporarily deployed to WindowsCILock June 25, 2024 22:09 — 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. Even with the simplistic nature of what is being performed, all of the additional actions required to work the table/listfile isn't very pretty.

@sarnex sarnex temporarily deployed to WindowsCILock June 25, 2024 22:41 — with GitHub Actions Inactive
@sarnex
Copy link
Contributor Author

sarnex commented Jun 26, 2024

@mdtoguchi Thanks Mike. If you have an idea on how I could rework this change to be cleaner please let me know, I'm happy to do it in a follow-up PR.

@sarnex
Copy link
Contributor Author

sarnex commented Jun 26, 2024

HIP failure #14300

@sarnex sarnex merged commit b6d5a10 into intel:sycl Jun 26, 2024
12 of 14 checks passed
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.

3 participants