Skip to content
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

[SYCL][DOC] Move proposed FPGA extensions #5453

Merged
merged 3 commits into from Feb 4, 2022

Conversation

gmlueck
Copy link
Contributor

@gmlueck gmlueck commented Feb 1, 2022

These two extension specifications are proposed changes to existing
extensions. Move them to the "proposed" directory. You can see the
proposed changes via:

$ git diff HEAD:sycl/doc/extensions/{supported,proposed}/SYCL_EXT_INTEL_DATAFLOW_PIPES.asciidoc
$ git diff HEAD:sycl/doc/extensions/{supported,proposed}/SYCL_EXT_INTEL_FPGA_LSU.md

These two extension specifications are proposed changes to existing
extensions.  Move them to the "proposed" directory.  You can see the
proposed changes via:

```
$ git diff HEAD:sycl/doc/extensions/{supported,proposed}/SYCL_EXT_INTEL_DATAFLOW_PIPES.asciidoc
$ git diff HEAD:sycl/doc/extensions/{supported,proposed}/SYCL_EXT_INTEL_FPGA_LSU.md
```
@gmlueck gmlueck requested a review from a team February 1, 2022 21:06
jbrodman
jbrodman previously approved these changes Feb 2, 2022
@Pennycook
Copy link
Contributor

I might have missed it, but is there anything in these proposed extensions that identify them as being a revision of a supported extension? Is it worth adding a banner at the top of such extensions, like:

IMPORTANT: This proposal is an update of an existing extension.  See [link-to-extension] for details of what is currently supported.

@gmlueck
Copy link
Contributor Author

gmlueck commented Feb 2, 2022

I might have missed it, but is there anything in these proposed extensions that identify them as being a revision of a supported extension? Is it worth adding a banner at the top of such extensions, like:

Do you think this is necessary? The file name is the same in the "supported" and "proposed" directory, so this should be a pretty good clue that it is an update to an existing extension.

I'd like to avoid extra work when possible, both when drafting the proposal and when the new proposal is implemented. I can see it being a common mistake to forget to add a banner like this (when drafting a proposal) or to forget to remove the banner (when implementing the proposal).

@Pennycook
Copy link
Contributor

Do you think this is necessary? The file name is the same in the "supported" and "proposed" directory, so this should be a pretty good clue that it is an update to an existing extension.

My concern is that there's no easy way to tell if something in the "proposed" directory is also in the "supported" directory. Frequent contributors to the project will understand and know to check, but if somebody is sent a link to a proposal, would we expect them to check the "supported" directory just in case? Or if somebody finds the "proposed" directory first, how do we stop them from concluding that everything in a given extension document is unsupported?

I'd like to avoid extra work when possible, both when drafting the proposal and when the new proposal is implemented. I can see it being a common mistake to forget to add a banner like this (when drafting a proposal) or to forget to remove the banner (when implementing the proposal).

This would be more work, but maybe we could consider something like git hooks and/or a GitHub action here. If the banner has a fixed form that's used by all extensions, we could automatically block PRs where the banner appears in the "supported" directory. Similarly, we could add something to check if two extensions with the same name exist in multiple directories. Maybe we could even get the hook to add the banner if it's missing?

Add a notice to the top of these documents stating that they are
proposals to existing extensions.
@gmlueck
Copy link
Contributor Author

gmlueck commented Feb 3, 2022

@Pennycook: I added a notice to the top of each file, and included a link to the existing (supported) extension. See 485c98f.

@bader bader merged commit ad9ac98 into intel:sycl Feb 4, 2022
@gmlueck gmlueck deleted the gmlueck/mv-fpga-proposals branch February 4, 2022 18:56
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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants