Skip to content

[SYCL][Docs] Mention preview macro for early ABI breaks #18422

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

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions sycl/doc/developer/ABIPolicyGuide.md
Original file line number Diff line number Diff line change
@@ -100,6 +100,13 @@ instead of replacing existing APIs. Also, please, avoid any changes, mentioned i
between breaking and non-breaking changes. If unsure, do not hesitate to ask code
owners for help.

If ABI-breaking changes are being planned prior to the ABI-breaking window
opening, the corresponding ABI-breaking changes (including removal of unused
symbols) can be done under the `__INTEL_PREVIEW_BREAKING_CHANGES` macro. This
helps maintainers make sure that the ABI-breaking changes makes it in during the
ABI-breaking window, as they will be considered for promotion out of the
`__INTEL_PREVIEW_BREAKING_CHANGES` guards during that time.
Copy link
Contributor

Choose a reason for hiding this comment

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

We have inline namespace with API/ABI version. Why do we ignore this framework for making breaking changes and use a macro instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think they are necessarily mutually exclusive, though historically we have used the preview macro more. I personally prefer the breaking changes macro, as it makes it easier to make it clear what is going away and what is coming in with the breaking changes. It also lets us test that we can indeed remove the symbols that we intend on removing, rather than finding out during the limited ABI-break window that we still need to remove some uses of old symbols and/or someone added more without realizing.

Copy link
Contributor

Choose a reason for hiding this comment

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

I highly recommend using symbol versioning. It enables "preview" features in non-breaking manner.

If SYCL runtime team against using versioning for some reasons, please, remove it from the library sources with the next API/ABI breaking release.

I don't think they are necessarily mutually exclusive, though historically we have used the preview macro more.

Symbol versioning has never been used.

I personally prefer the breaking changes macro, as it makes it easier to make it clear what is going away and what is coming in with the breaking changes.

I'm not sure I understand what makes macro to be clearer than a separate namespace.

The main advantage of preview macro solution is small code to compile. As adding a new namespace is non-breaking change, old symbols versions must be compiled as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If SYCL runtime team against using versioning for some reasons, please, remove it from the library sources with the next API/ABI breaking release.

I don't think they are necessarily mutually exclusive, though historically we have used the preview macro more.

Symbol versioning has never been used.

I am personally ambivalent towards it, but every time we've wanted to use it, it was insufficient or brought on other issues. It has been a while though, so I don't remember the details.

I'm not sure I understand what makes macro to be clearer than a separate namespace.

When using the preview macro, it is much easier to place the breaks close to the old changes. For example, if you want to make an API break inside a function, you can just make the changes inside the function, while with the versioning you have to place a copy of the function (or split out the functionality) into a different namespace somewhere potentially far from the current function.

For ABI breaking changes, the preview macro guard has two benefits:

  1. You can place the changes close to (or even inside) the old symbols.
  2. You can mask out the old function, ensuring that we can test the library without it. I mentioned that in the previous comment, but it helps us avoid using the symbols we intend to remove and also makes it clear which ones they are.


**Note**: Features clearly marked as experimental are considered as an exception
to this guideline.