-
Notifications
You must be signed in to change notification settings - Fork 788
[SYCL] Initial ABI checks implementation #1528
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
Changes from 19 commits
3deffec
189b639
ad5e3b6
407341e
a5f4978
3f9f6d8
c748c91
961afa4
23ca0ec
3ccb3aa
a9a8f0f
c3838e1
07e1b93
eb108bb
3e2c8e3
371a715
4005b2b
ecc3771
82b9ee4
bf41e70
e93a28e
7a24da3
ad7489b
cfb822d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
# ABI Policy Guide | ||
|
||
## Intro | ||
|
||
Application Binary Interface is a contract between binary modules, that defines | ||
how structures and routines are accessed in machine code. Changing the ABI may | ||
break backwards compatibility with library for user-developed applications, | ||
resulting in need to rebuild such applications. The goal of this document is to | ||
provide guidelines for maintaining the current ABI and mechanisms of notifying | ||
alexbatashev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
users about ABI changes. | ||
|
||
All ABI changes can be divided into two large groups: breaking and non-breaking. | ||
A breaking change means that the new binary is incompatible with the previous | ||
version (i.e. it can not be used as a drop-in replacement). A non-breaking | ||
change means that the forward compatibility is broken (i.e. the old library | ||
can be replaced with newer version, but not vice versa). | ||
|
||
The following non-exhaustive list contains changes that are considered to be | ||
breaking: | ||
|
||
1. Changing the size of exported symbol (for example, adding new member field | ||
to the exported class). | ||
1. Removing the exported symbol (that includes both changing the signature of | ||
exported routine and removing it). | ||
1. Changing the alignment of exported symbol. | ||
1. Changing the layout of exported symbol (for example, reordering class field | ||
members). | ||
1. Adding or removing base classes. | ||
|
||
Adding a new exported symbol is considered to be non-breaking change. | ||
|
||
## ABI Versioning Policy | ||
|
||
TBD | ||
|
||
## `__SYCL_EXPORT` Macro | ||
|
||
The `__SYCL_EXPORT` provides facilities for fine-grained control over exported | ||
symbols. Mark symbols that are supposed to be accessible by the user and that | ||
are implemented in the SYCL Runtime library with this macro. Template | ||
specializations also must be explicitly marked with `__SYCL_EXPORT` macro. | ||
Symbols not marked `__SYCL_EXPORT` have internal linkage. | ||
|
||
A few examples of when it is necessary to mark symbols with the macro: | ||
|
||
* The `device` class: | ||
- It is defined as API by the SYCL spec. | ||
- It is implemented in `device.cpp` file. | ||
* The `SYCLMemObjT` class: | ||
- It is not defined in the SYCL spec, but it is an implementation detail that | ||
is accessible by the user (buffer and image inherit from this class). | ||
- It has symbols that are implemented in the Runtime library. | ||
|
||
When it is not necessary to mark symbols with `__SYCL_EXPORT`: | ||
* The `buffer` class: | ||
- It is defined by the SYCL spec, but it is fully implemented in the headers. | ||
* The `ProgramManager` class: | ||
- It is an implementation detail. | ||
- It is not accessed from the header files that are available to users. | ||
|
||
## Automated ABI Changes Testing | ||
|
||
> The automated tests deal with the most commonly occurring problems, but they | ||
kbobrovs marked this conversation as resolved.
Show resolved
Hide resolved
|
||
> may not catch some corner cases. If you believe your PR breaks ABI, but the | ||
> test does not indicate that, please, notify the reviewers. | ||
|
||
There is a set of tests to help identifying ABI changes: | ||
|
||
* `test/abi/sycl_symbols_*.dump` contains dump of publicly available symbols. | ||
If you add a new symbol, it is considered non-breaking change. When the test | ||
reports missing symbols, it means you have either changed or remove some of | ||
existing API methods. In both cases you need to adjust the dump file. You | ||
can do it either manually, or by invoking the following command: | ||
```shell | ||
python3 sycl/tools/abi_check.py --mode dump_symbols --output path/to/output.dump path/to/sycl.so(.dll) | ||
``` | ||
* `test/abi/layout*` and `test/abi/symbol_size*` are a group of tests to check | ||
the internal layout of some classes. The layout tests check Clang AST for | ||
changes, while symbol_size check `sizeof` for objects. Changing the class | ||
layout is a breaking change. | ||
|
||
## Breaking ABI | ||
|
||
Whenever you need to change the existing ABI, please, follow these steps: | ||
|
||
1. Get approval from project maintainers to make a breaking/non-breaking change. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably don't need this line. It's codeowners job to approve/don't approve. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @pvchupin the spirit of this line is "ABI stability is a major thing, make sure reviewers understand why you do it", and not the fact, that there're some formal steps needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. Can you rephrase accordingly then? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does it sound better now? |
||
2. Fix failing ABI tests in your Pull Request. Use aforementioned techniques to | ||
update test files. | ||
3. Update the library version according to the policies. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,4 +24,4 @@ Developing oneAPI DPC++ Compiler | |
CompilerAndRuntimeDesign | ||
EnvironmentVariables | ||
PluginInterface | ||
|
||
ABIPolicyGuide |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
// RUN: %clangxx -fsycl -c -fno-color-diagnostics -Xclang -ast-dump %s | FileCheck %s | ||
// REQUIRES: linux | ||
|
||
#include <CL/sycl/handler.hpp> | ||
|
||
// The order of field declarations and their types are important. | ||
|
||
// CHECK: CXXRecordDecl {{.*}} class handler definition | ||
// CHECK: FieldDecl {{.*}} MQueue 'shared_ptr_class<detail::queue_impl>':'std::shared_ptr<cl::sycl::detail::queue_impl>' | ||
// CHECK-NEXT: FieldDecl {{.*}} MArgsStorage 'vector_class<vector_class<char> >':'std::vector<std::vector<char, std::allocator<char> >, std::allocator<std::vector<char, std::allocator<char> > > >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MAccStorage 'vector_class<detail::AccessorImplPtr>':'std::vector<std::shared_ptr<cl::sycl::detail::AccessorImplHost>, std::allocator<std::shared_ptr<cl::sycl::detail::AccessorImplHost> > >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MLocalAccStorage 'vector_class<detail::LocalAccessorImplPtr>':'std::vector<std::shared_ptr<cl::sycl::detail::LocalAccessorImplHost>, std::allocator<std::shared_ptr<cl::sycl::detail::LocalAccessorImplHost> > >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MStreamStorage 'vector_class<shared_ptr_class<detail::stream_impl> >':'std::vector<std::shared_ptr<cl::sycl::detail::stream_impl>, std::allocator<std::shared_ptr<cl::sycl::detail::stream_impl> > >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MSharedPtrStorage 'vector_class<shared_ptr_class<const void> >':'std::vector<std::shared_ptr<const void>, std::allocator<std::shared_ptr<const void> > >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MArgs 'vector_class<detail::ArgDesc>':'std::vector<cl::sycl::detail::ArgDesc, std::allocator<cl::sycl::detail::ArgDesc> >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MAssociatedAccesors 'vector_class<detail::ArgDesc>':'std::vector<cl::sycl::detail::ArgDesc, std::allocator<cl::sycl::detail::ArgDesc> >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MRequirements 'vector_class<detail::Requirement *>':'std::vector<cl::sycl::detail::AccessorImplHost *, std::allocator<cl::sycl::detail::AccessorImplHost *> >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MNDRDesc 'detail::NDRDescT':'cl::sycl::detail::NDRDescT' | ||
// CHECK-NEXT: FieldDecl {{.*}} MKernelName 'cl::sycl::string_class':'std::__cxx11::basic_string<char>' | ||
// CHECK-NEXT: FieldDecl {{.*}} MKernel 'shared_ptr_class<detail::kernel_impl>':'std::shared_ptr<cl::sycl::detail::kernel_impl>' | ||
// CHECK-NEXT: FieldDecl {{.*}} MCGType 'detail::CG::CGTYPE':'cl::sycl::detail::CG::CGTYPE' | ||
// CHECK-NEXT: DeclRefExpr {{.*}} 'cl::sycl::detail::CG::CGTYPE' EnumConstant {{.*}} 'NONE' 'cl::sycl::detail::CG::CGTYPE' | ||
// CHECK-NEXT: FieldDecl {{.*}} MSrcPtr 'void *' | ||
// CHECK-NEXT: ImplicitCastExpr {{.*}} <NullToPointer> | ||
// CHECK-NEXT: CXXNullPtrLiteralExpr {{.*}} 'nullptr_t' | ||
// CHECK-NEXT: FieldDecl {{.*}} MDstPtr 'void *' | ||
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void *' <NullToPointer> | ||
// CHECK-NEXT: CXXNullPtrLiteralExpr {{.*}} 'nullptr_t' | ||
// CHECK-NEXT: FieldDecl {{.*}} MLength 'size_t':'unsigned long' | ||
// CHECK-NEXT: ImplicitCastExpr {{.*}} 'size_t':'unsigned long' <IntegralCast> | ||
// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 0 | ||
// CHECK-NEXT: FieldDecl {{.*}} MPattern 'vector_class<char>':'std::vector<char, std::allocator<char> >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MHostKernel 'unique_ptr_class<detail::HostKernelBase>':'std::unique_ptr<cl::sycl::detail::HostKernelBase, std::default_delete<cl::sycl::detail::HostKernelBase> >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MOSModuleHandle 'detail::OSModuleHandle':'long' | ||
// CHECK-NEXT: FieldDecl {{.*}} MInteropTask 'std::unique_ptr<detail::InteropTask>':'std::unique_ptr<cl::sycl::detail::InteropTask, std::default_delete<cl::sycl::detail::InteropTask> >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MEvents 'vector_class<detail::EventImplPtr>':'std::vector<std::shared_ptr<cl::sycl::detail::event_impl>, std::allocator<std::shared_ptr<cl::sycl::detail::event_impl> > >' | ||
// CHECK-NEXT: FieldDecl {{.*}} MIsHost 'bool' | ||
// CHECK-NEXT: CXXBoolLiteralExpr {{.*}} 'bool' false |
Uh oh!
There was an error while loading. Please reload this page.