Skip to content
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

[New offload driver][Device lib] Add SYCL device library files for all targets #14102

Merged

Conversation

asudarsa
Copy link
Contributor

@asudarsa asudarsa commented Jun 9, 2024

clang-linker-wrapper is not target-specific. i.e. it is not called for a single target device. It is called only once.
Currently, clang-linker-wrapper is called only with device images with spir64 targets. So, the existing approach to capture the first target triple in the list of triples and use it for gathering sycl-device-library files is valid. As we plan to add support for more targets (AOT), we need to gather sycl-device-libraries for all targets. This PR addresses this change.

Also, the triple should not be passed to the linker wrapper. The linker wrapper should get the triples from device images.

Thanks

@asudarsa asudarsa requested a review from a team as a code owner June 9, 2024 01:08
@asudarsa asudarsa requested a review from maksimsab June 9, 2024 01:08
// TODO(NOM1): Support target triples in a more generic way.
// TODO(NOM3): Investigate why passing spirv64-unknown-unknown does not
// work.
if (TargetTriple.isSPIR())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should not pass target triple to linker wrapper. This information is available inside the device images.

@asudarsa asudarsa force-pushed the add_sycl_device_lib_files_for_all_targets branch from 7c48cf7 to 46cdf02 Compare June 9, 2024 01:30
clang/lib/Driver/ToolChains/Clang.cpp Outdated Show resolved Hide resolved
@asudarsa asudarsa force-pushed the add_sycl_device_lib_files_for_all_targets branch from 46cdf02 to e0c6fe2 Compare June 10, 2024 19:17
@asudarsa
Copy link
Contributor Author

@intel/llvm-gatekeepers Can you please merge? This is ready for merging.

Thanks

Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Driver/sycl-offload-new-driver.c is failing in post-commit after #14091. I am blocking this PR until the patch has either been reverted or the test failure has been addressed.

Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

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

Revert was merged. Retracting my blocking comment.

…l targets

Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
@asudarsa asudarsa force-pushed the add_sycl_device_lib_files_for_all_targets branch from e0c6fe2 to e6b6acd Compare June 11, 2024 15:22
@asudarsa
Copy link
Contributor Author

@intel/llvm-gatekeepers
This is ready to be merged. Thanks

@againull againull merged commit 6ecce4f into intel:sycl Jun 11, 2024
14 checks passed
ianayl pushed a commit to ianayl/sycl that referenced this pull request Jun 13, 2024
…l targets (intel#14102)

clang-linker-wrapper is not target-specific. i.e. it is not called for a
single target device. It is called only once.
Currently, clang-linker-wrapper is called only with device images with
spir64 targets. So, the existing approach to capture the first target
triple in the list of triples and use it for gathering
sycl-device-library files is valid. As we plan to add support for more
targets (AOT), we need to gather sycl-device-libraries for all targets.
This PR addresses this change.

Also, the triple should not be passed to the linker wrapper. The linker
wrapper should get the triples from device images.

Thanks

---------

Signed-off-by: Arvind Sudarsanam <arvind.sudarsanam@intel.com>
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