-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL][Doc] Add Nov'23 release notes #12365
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
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.
Haven't yet looked into "Bug Fixes" section
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.
Minor suggestions to SYCL-Graph related items, mostly around consistently using sycl_ext_oneapi_graph
to reference the extension rather than a mix of SYCL-Graph/SYCL-Graphs/graph/graphs
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.
Review for the remaining sections
sycl/ReleaseNotes.md
Outdated
### SYCL Compiler | ||
- Fix backend option passing issues with multiple targets [12ca2db0e30b155c7d6bf14320dd1ba468ac9294] | ||
- Fix -fgpu-rdc option for CUDA [f7595ac7527e67a59e33550851366db7d886ac3f] | ||
- Fix triple in offload mismatch warning [0fd9a4eb17a2db0fed151ba9d975804322966bcc] |
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.
sycl/ReleaseNotes.md
Outdated
- Deprecate experimental functions: quot,div,mod,dp*,line,lrp [76976a22ba2e86059d655aa7d5f71250160f4864] | ||
- Remove ESIMD Emulator plugin [1ece6da6b1139ad29d1ed44a05759e9b5974d3e8] | ||
- Deprecate `sycl::abs(genfloat)` [d15d4409b2cf7e023bf6f462fac5630699f1c0a7] | ||
- Don't include `<complex>/<cmath>` from `sycl.hpp`, unless `-fpreview-breaking-changes` [6ed0ab887d18b18209a7d1c31b8bb8240a820f79] [d22c9c58d2e7c4936158dde6056281ef2cf0cdf2] |
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.
Tagging @aelovikov-intel and @steffenlarsen here for suggestions. Strictly speaking, this change didn't happen, because it is hidden under a flag. Should we somehow specially annotate such changes as "future API/ABI breackages"?
In any case, "unless -fpreview-breaking-changes
" -> "if -fpreview-breaking-changes
is set"
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.
I do think it would be best to mark such changes as being behind the preview flag somehow.
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.
We had an approval just to drop the includes unconditionally. I just didn't push through with #11528.
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.
There are few of my previous comments which haven't been addressed yet, but overall this version looks much better to me.
I think the main remaining comment from my side is having a well-defined strategy about documenting changes that go under -fpreview-breaking-changes
flag, tagging @steffenlarsen here. Once that's settled, I'm ok to approve.
sycl/ReleaseNotes.md
Outdated
- Improved error message when `queue::memcpy` receives a null pointer by reporting source code info(file/line number) [626c4b5078c1] | ||
- Improved accuracy of double `tanpi` host implementation [041a74c0cbda] | ||
- Added more support for AMD subgroup functions [288aeaef03a9] | ||
- `annotated_ptr` API fixes [954730e7a823] [fbad42a398ef] [43336a15fcaf] |
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 a little vague. Is it possible to briefly describe what the fixes are?
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, thanks for working on this!
I would suggest that we wait till Friday in case someone else wants to chime in with some extra review/suggestions and then merge the PR
sycl/ReleaseNotes.md
Outdated
|
||
## New Features | ||
### SYCL Compiler | ||
- Added `-fpreview-breaking-changes` option [08febcfcb64a] [99b97eef4278] |
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.
@steffenlarsen, @stdale-intel, should we elaborate here a bit more about the option purpose? In any case we can probably add a reference to a section where we describe changes we did under that flag
No description provided.