-
Notifications
You must be signed in to change notification settings - Fork 798
[SYCL] Remove manifest from offload binary structure #20748
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
Exposing abi-breaking changes from intel#20062 Co-authored-by: Steffen Larsen <steffen.larsen@intel.com>
|
|
||
| // --- Prepare test data | ||
| // - create the first binary image | ||
| // End-to-end test for clang-offload-wrapper executable: |
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.
Is this a copy of clang/test/Driver/clang-offload-wrapper-exe-preview.cpp?
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.
Yes, that's right.
| // RUN: clang-offload-wrapper --help | FileCheck %s --check-prefix CHECK-HELP | ||
| // CHECK-HELP: OVERVIEW: A tool to create a wrapper bitcode for offload target binaries. | ||
| // CHECK-HELP: Takes offload target binaries and optional manifest files as input | ||
| // CHECK-HELP: Takes offload target binaries as input |
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.
This is some strange formatting. Do we need to update the help text somewhere?
| "A tool to create a wrapper bitcode for offload target binaries.\n" | ||
| #ifndef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| "Takes offload target binaries and optional manifest files as input\n" | ||
| #else | ||
| "Takes offload target binaries as input\n" | ||
| #endif // __INTEL_PREVIEW_BREAKING_CHANGES | ||
| "and produces bitcode file containing target binaries packaged as data\n" | ||
| "and initialization code which registers target binaries in the offload\n" | ||
| #ifndef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| "runtime. Manifest files format and contents are not restricted and are\n" | ||
| "a subject of agreement between the device compiler and the native\n" | ||
| "runtime for that device. When present, manifest file name should\n" | ||
| "immediately follow the corresponding device image filename on the\n" | ||
| "command line. Options annotating a device binary have effect on all\n" | ||
| #else | ||
| "runtime. Options annotating a device binary have effect on all\n" | ||
| #endif // __INTEL_PREVIEW_BREAKING_CHANGES | ||
| "subsequent input, until redefined.\n" |
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.
Answering my earlier question - we need to reformat this paragraph. I'm not sure what width we're trying to achieve here, but something like this:
"A tool to create a wrapper bitcode for offload target binaries.\n"
"Takes offload target binaries as input and produces bitcode file\n"
"containing target binaries packaged as data and initialization code\n"
"which registers target binaries in the offload runtime. Options\n"
"annotating a device binary have effect on all subsequent input,\n"
"until redefined.\n"
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.
Thank you, fixed.
| { | ||
| std::ofstream File{LockFile}; | ||
| } |
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.
This and below seem unrelated.
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.
Excluded this change.
YuriPlyakhin
left a comment
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.
Thank you! LGTM.
hchilama
left a comment
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 - Driver
Exposing abi-breaking changes from #20062
Co-authored-by: Steffen Larsen steffen.larsen@intel.com