Skip to content

Conversation

@mdtoguchi
Copy link
Contributor

This change represents 2 changes.

  1. Update the build to create LLVM-IR based device libraries.

This is done by using the existing -fsycl-device-only functionality to create the device libraries that are used during device link.

  1. Update the compiler driver to use the LLVM-IR based device libraries.

When performing the device linking instead of performing a full unbundle of the device libraries and using the device binary that was extracted.

This streamlines the device linking behaviors, completely removing the cumbersome unbundling step from the process, freeing up a number of exteral tool calls.

This change represents 2 changes.
1.  Update the build to create LLVM-IR based device libraries.

This is done by using the existing -fsycl-device-only functionality to
create the device libraries that are used during device link.

2.  Update the compiler driver to use the LLVM-IR based device
    libraries.

When performing the device linking instead of performing a full unbundle
of the device libraries and using the device binary that was extracted.

This streamlines the device linking behaviors, completely removing the
cumbersome unbundling step from the process, freeing up a number of
exteral tool calls.
C.MakeAction<OffloadAction>(
Dep, SYCLDeviceLibsUnbundleAction->getType());
DeviceLinkObjects.push_back(SYCLDeviceLibsDependenciesAction);
if (TC->getTriple().isNVPTX()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need a separate path for NVPTX backend?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are specific cubin files that are in the fat objects that are used. I only created .bc device libraries for the SYCL specific linking.

@asudarsa
Copy link
Contributor

Windows test fails might just require test update

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.

Changes look good to me. It will help to get this checked in soon. One important pointer is the deviation for NVPTX backends. It will help to understand why we need that deviation and if it can be avoided.

Thanks

@mdtoguchi
Copy link
Contributor Author

Windows test fails might just require test update

I have updated the windows tests accordingly and also consolidated the behaviors for input files as the inputs for Windows and Linux are both .bc files instead of differing object files.

@mdtoguchi mdtoguchi marked this pull request as ready for review April 27, 2024 00:57
@mdtoguchi mdtoguchi requested a review from a team as a code owner April 27, 2024 00:57
@mdtoguchi mdtoguchi requested a review from a team as a code owner April 27, 2024 00:57
@mdtoguchi mdtoguchi requested a review from uditagarwal97 April 27, 2024 00:57
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.

LGTM. Thanks

Copy link
Contributor

@uditagarwal97 uditagarwal97 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mdtoguchi
Copy link
Contributor Author

@intel/dpcpp-clang-driver-reviewers, could this be reviewed?

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