Skip to content

Conversation

@Pennycook
Copy link
Contributor

@Pennycook Pennycook commented Dec 8, 2021

The new proposal makes the following changes:

  • Expands upon the motivation for the extension, and provides some
    non-normative recommendations for when it should be used.

  • Reformats the description of the functions to match the style used
    by ISO C++ and the newest sections of SYCL 2020.

  • Moves all functionality into the this_kernel namespace, to more
    closely align with the this_thread namespace in ISO C++.

  • Documents outstanding issues regarding undefined behavior.

Signed-off-by: John Pennycook john.pennycook@intel.com

@Pennycook Pennycook added the spec extension All issues/PRs related to extensions specifications label Dec 8, 2021
@Pennycook Pennycook requested review from gmlueck and rarutyun December 8, 2021 16:31
@Pennycook Pennycook requested a review from a team as a code owner December 8, 2021 16:31
In preparation for moving the FreeFunctionQueries extension out of
the experimental namespace, this commit makes the following changes:

- Expands upon the motivation for the extension, and provides some
  non-normative recommendations for when it should be used.

- Reformats the description of the functions to match the style used
  by ISO C++ and the newest sections of SYCL 2020.

- Moves all functionality into the this_kernel namespace, to more
  closely align with the this_thread namespace in ISO C++.

- Documents outstanding issues regarding undefined behavior.

Signed-off-by: John Pennycook <john.pennycook@intel.com>
Signed-off-by: John Pennycook <john.pennycook@intel.com>
Copy link
Contributor

@rarutyun rarutyun left a comment

Choose a reason for hiding this comment

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

Overall, looks good but I am planning to look at it deeper after the vacation/holidays (if it is not merged). If this PR is urgent, please don't wait for me since I already put my preliminary review

Pennycook and others added 3 commits January 26, 2022 14:12
Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>
Co-authored-by: Greg Lueck <gregory.m.lueck@intel.com>
Signed-off-by: John Pennycook <john.pennycook@intel.com>
@bader bader requested review from gmlueck and rarutyun January 27, 2022 11:02
gmlueck
gmlueck previously approved these changes Jan 28, 2022
@bader
Copy link
Contributor

bader commented Feb 2, 2022

@Pennycook, please, resolve merge conflicts in README.md file.

@Pennycook
Copy link
Contributor Author

@gmlueck, @bader: The diff doesn't look very nice any more, but if I did everything correctly then de41bf2 just moves these changes into experimental/ with the new filename, and 05f1f77 moves things out into proposed/.

My justification for putting things into proposed/ now is: 1) we've dropped the experimental namespace; and 2) the new API isn't implemented yet.

@bader bader requested a review from gmlueck February 2, 2022 15:46
gmlueck
gmlueck previously approved these changes Feb 2, 2022
@gmlueck
Copy link
Contributor

gmlueck commented Feb 2, 2022

Oh wait. I just noticed that you deleted the existing "sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_FREE_FUNCTION_QUERIES.asciidoc". Was that your intention?

@Pennycook
Copy link
Contributor Author

Oh wait. I just noticed that you deleted the existing "sycl/doc/extensions/experimental/SYCL_EXT_ONEAPI_FREE_FUNCTION_QUERIES.asciidoc". Was that your intention?

I don't think we need to keep the documentation for the old APIs around. Since they were experimental we can remove them without deprecation, and that's what I expect we'll do once the new API is implemented.

@Pennycook
Copy link
Contributor Author

@gmlueck: As discussed offline, I've restored the experimental extension in 07f0cba.

@bader bader requested a review from gmlueck February 2, 2022 20:04
@bader
Copy link
Contributor

bader commented Feb 2, 2022

@Pennycook, is PR description reflects the changes correctly? I'm using it as a commit message, so I'd like to make sure doesn't conflict with recent updates.

@Pennycook Pennycook changed the title [SYCL][Doc] Update FreeFunctionQueries [SYCL][Doc] Add new free function queries proposal Feb 2, 2022
@Pennycook
Copy link
Contributor Author

@bader: I've tweaked things slightly to reflect that we're now adding a new proposal, rather than updating the old extension documentation. I think it can be merged now.

@bader bader merged commit 7a93a49 into intel:sycl Feb 2, 2022
alexbatashev added a commit to alexbatashev/llvm that referenced this pull request Feb 5, 2022
* upstream/sycl: (3571 commits)
  [ESIMD] Doxygen update part III - core APIs. (intel#5472)
  [SYCL][DOC] Move proposed FPGA extensions (intel#5453)
  [SYCL] Add -fsycl-fp32-prec-sqrt flag (intel#5309)
  [SYCL] Emit program build logs for warning levels >= 2 (intel#5319)
  [SYCL] Add clang support for code_location in KernelInfo (intel#5335)
  [SYCL][Doc] Move FPGA extensions (intel#5470)
  [ESIMD] Fix public simd and simd_view APIs. (intel#5465)
  [SYCL] Deprecate sycl::atomics in SYCL 2020 mode (intel#5440)
  [SYCL] Add unit test for PR 5414 (intel#5450)
  [XPTI] Allow arbitrary data types in metadata (intel#4998)
  [SYCL][DOC] Move discard queue events to supported (intel#5452)
  [Driver][SYCL] Initial support for allowing fat static -lname processing (intel#5413)
  [SYCL] Fix dead pointer usage if leaf buffer overflows (intel#5417)
  [SYCL][L0] Fix memory leak in USM prefetch (intel#5461)
  [SYCL][Doc] Add new free function queries proposal (intel#5106)
  [SYCL][ESIMD] Update vc-intrinsics deps to the top of the trunk (intel#5460)
  [SYCL][DOC] Move old spec constant extension spec (intel#5456)
  [SYCL][DOC] Move deprecated extensions (intel#5458)
  [SYCL][DOC] Fix links to old SubGroupMask doc (intel#5459)
  [ESIMD] Doxygen update part II - memory APIs. (intel#5443)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

spec extension All issues/PRs related to extensions specifications

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants