-
Notifications
You must be signed in to change notification settings - Fork 795
[Driver][SYCL] Address issue with code splitting and FPGA Archives #15794
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
Conversation
When enabling device code splitting with FPGA archive generation (-fsycl-link) the resulting device images were not packed correctly. We were archiving the resulting -batch wrapped image, which in turn would not be extracted properly when being consumed. To address this, update how the device images are created for insertion into the archive to be individual split binaries as opposed to using the wrapper -batch behavior. This is performed for both the early and image type archives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tools change (removing XFAIL) LGTM, no idea about the driver part :)
also sorry for moving that test :)
@intel/dpcpp-clang-driver-reviewers - ping for review |
@@ -10263,8 +10263,18 @@ void OffloadWrapper::ConstructJob(Compilation &C, const JobAction &JA, | |||
const InputInfo &I = Inputs[0]; | |||
assert(I.isFilename() && "Invalid input."); | |||
|
|||
if (I.getType() == types::TY_Tempfiletable || | |||
I.getType() == types::TY_Tempfilelist || IsEmbeddedIR) | |||
// TODO: The embedded compilation step after the wrapping step restricts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this a TODO? Is this not more of a restriction?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a restriction - but in my opinion it would be best to pull the compile step out of the wrapping step which provides a cleaner picture of the toolchain. We shouldn't expect the wrapping step to do the compilation too (it isn't embedded for OpenMP)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. No blocking changes requested.
Thanks
@intel/llvm-gatekeepers, this is ready for merge - thanks! |
When enabling device code splitting with FPGA archive generation (-fsycl-link) the resulting device images were not packed correctly. We were archiving the resulting -batch wrapped image, which in turn would not be extracted properly when being consumed.
To address this, update how the device images are created for insertion into the archive to be individual split binaries as opposed to using the wrapper -batch behavior. This is performed for both the early and image type archives.