Skip to content

[SYCL] Initial ABI checks implementation #1528

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom
Merged
merged 24 commits into from
Apr 23, 2020
Merged

[SYCL] Initial ABI checks implementation #1528

merged 24 commits into from
Apr 23, 2020

Conversation

alexbatashev
Copy link
Contributor

  • Add check for symbol changes on Linux
  • Check sizes for classes
  • Check internal layout for handler class (via AST)
  • Add initial ABI Policy Guide, that describes the goals and use cases for those checks

The set of tests is incomplete and will be further extended.

Signed-off-by: Alexander Batashev alexander.batashev@intel.com

Alexander Batashev added 15 commits January 29, 2020 10:40
[SYCL] Add basic ABI checks
3deffec 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Basic layout test
189b639 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Make tests more universal
ad5e3b6 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
A more robust symbols checker
407341e 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Add vtable checks
a5f4978 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Merge remote-tracking branch 'origin/sycl' into private/abatashe/basi…
3f9f6d8 
…c_abi_checks

* origin/sycl: (6966 commits)
  [NFC][SYCL] Do not add `sycl_device` attribute to OpenCL kernel (intel#1439)
  [SYCL][NFC] Move SYCL pipe metadata call to be inside the null check for D (intel#1436)
  [SYCL][NFC] Move template function definition to .h file (intel#1433)
  [SYCL] Don't expose vector of booleans as storage format (intel#1419)
  [SYCL] Don't throw exceptions from destructors (intel#1378)
  [BuildBot] Add support for multiple CMake options (intel#1434)
  [SYCL][NFC] Fix warning inline namespace reopened as no-inline (intel#1435)
  [SYCL] Check if loadPlugin returns a nullptr (intel#1411)
  [SYCL] Release notes for February'20 SYCL implementation update (intel#1400)
  [SYCL][Doc] Improve contribution guidelines (intel#1422)
  [BuildBot] Add --cmake-opts option to configure.py script (intel#1430)
  [SYCL] Enable non-read-write memory object mapping in scheduler (intel#1390)
  [SYCL][Driver] Do not store AOT-specific options in the image descriptor (intel#1428)
  [Driver][NFC] Fix string problem used for tracking duplicate triples (intel#1424)
  [SYCL][NFC] Use the non-deprecated setAlignment() in LowerWGScope (intel#1420)
  [SYCL][NFC] Fix formatting in GetStartedGuide (intel#1417)
  [NFC] Move CODEOWNERS file to enable GitHub automation (intel#1418)
  [SYCL] Add test for private array init by zeroes (intel#1402)
  [Driver][SYCL][FPGA] Adjust device and AOCX link order for FPGA (intel#1389)
  [SYCL] Change runtime check to assert in program_manager.cpp (intel#1413)
  ...
React to some comments
c748c91 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Merge remote-tracking branch 'origin/sycl' into private/abatashe/basi…
961afa4 
…c_abi_checks

* origin/sycl: (625 commits)
  [SYCL][Test] Disable spec_const_redefine.cpp on all devices but HOST (intel#1488)
  [SYCL] Only export public API (intel#1456)
  [SYCL][CUDA] Fix selected_binary argument in piextDeviceSelectBinary (intel#1475)
  [SYCL] Enable LIT testing with CUDA BE (intel#1458)
  [SYCL] Fix float to half-type conversion (intel#1395)
  [NFC] Cleanup unneded macro from builtins implementation (intel#1445)
  Enable cfg-printer LLVM lit tests only if LLVM linked statically (intel#1479)
  [SYCL][NFC] Reflect the "allowlist" renaming in the code (intel#1480)
  [SYCL][Doc] Update prerequisites in GetStartedGuide (intel#1466)
  [SYCL][USM] Remove vestigial dead code (intel#1474)
  [SYCL-PTX] Fix __spirv_GroupAsyncCopy stride computation (intel#1451)
  [Driver][SYCL] Emit an error if c compilation is forced (intel#1438)
  [SYCL] Fix sycl-post-link when no split and symbols are requested. (intel#1454)
  [SYCL] Change priority of devices in default_selector (intel#1264)
  [CI] Update CODEOWNERS matching rules order (intel#1468)
  [SYCL] Share PFWG lambda object through shared memory (intel#1455)
  [CI] Fix CODEOWNERS file syntax (intel#1464)
  [SYCL][CUDA] Fix active context when creating base event (intel#1447)
  [SYCL] Diagnose implicit declaration of kernel function type (intel#1450)
  [BuildBot] Modify configure script (intel#1421)
  ...
React to comments
23ca0ec 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Add some Windows support
3ccb3aa 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
React to comments
a9a8f0f 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Fix more comments
c3838e1 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Merge remote-tracking branch 'origin/sycl' into private/abatashe/basi…
07e1b93 
…c_abi_checks

* origin/sycl: (32 commits)
  [SYCL] Do not force LLVM_INCLUDE_TESTS variable (intel#1505)
  [SYCL][NFC] Align nd_item members with constructor initialization list (intel#1521)
  [SYCL] Move get_info_host implementation to header (intel#1514)
  [SYCL] Always use dynamic CRT for Unit tests (intel#1515)
  [SYCL][NFC] Temporarily disable sporadically failing test (intel#1526)
  [SYCL] Fix inline namespaces (intel#1525)
  [SYCL] Release notes for March'20 DPCPP implementation update (intel#1511)
  [SYCL][PI][CUDA] Implements get_native interoperability (intel#1332)
  [SYCL] Fix check-sycl test suite on systems w/o OpenCL (intel#1503)
  [SYCL][Doc] Update ExtendedAtomics documentation (intel#1487)
  [SYCL][CUDA] Expose context extended deleters on PI API (intel#1483)
  [SYCL][NFC] Remove a dropped environment variable from a test (intel#1506)
  [SYCL] Add opencl-aot to sycl-toolchain target (intel#1504)
  [SYCL] Allow to run deploy LIT tests from particular directory
  [SYCL][CUDA] Fix LIT testing with CUDA devices (intel#1300)
  [SYCL] Remove operator name keywords (intel#1501)
  [Driver][SYCL] Consider .lo files as static archives (intel#1500)
  [SYCL-PTX] Update the compiler design to describe the CUDA target (intel#1408)
  [SYCL] Fix library build on Windows (intel#1499)
  [SYCL][NFC] Refactor lit.cfg.py (intel#1452)
  ...
Update tests
eb108bb 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
React to comments
3e2c8e3 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
@alexbatashev alexbatashev requested a review from kbobrovs as a code owner April 15, 2020 14:42
Apply clang-format
371a715 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
bader
bader requested changes Apr 16, 2020
View reviewed changes
@alexbatashev alexbatashev requested a review from a team as a code owner April 16, 2020 13:39
React to comments
4005b2b 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
@alexbatashev alexbatashev requested a review from bader April 16, 2020 13:42
@alexbatashev @s-kanaev
Update sycl/test/abi/symbol_size.cpp
ecc3771 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>

Co-Authored-By: s-kanaev <57672082+s-kanaev@users.noreply.github.com>
bader
bader previously approved these changes Apr 17, 2020
View reviewed changes
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

Any ideas how to reduce # of exported symbols?

@bader bader requested a review from pvchupin April 17, 2020 17:50
@alexbatashev alexbatashev dismissed stale reviews from bader and sergey-semenov via bf41e70 April 17, 2020 18:26
React to comments
bf41e70 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
@alexbatashev
Copy link
Contributor Author

Any ideas how to reduce # of exported symbols?

@bader most of these symbols are actually builtins. Can we move those to headers?

@bader
Copy link
Contributor

bader commented Apr 17, 2020

Any ideas how to reduce # of exported symbols?

@bader most of these symbols are actually builtins. Can we move those to headers?

Might be a viable option, considering almost all them are either wrappers around standard C++ functions or has trivial implementation.

kbobrovs
kbobrovs reviewed Apr 21, 2020
View reviewed changes
Alexander Batashev added 3 commits April 21, 2020 15:50
React to comments
e93a28e 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
Merge remote-tracking branch 'origin/sycl' into private/abatashe/basi…
7a24da3 
…c_abi_checks

* origin/sycl:
  [SYCL][Driver] Enforce unique filenames when -save-temps is used (intel#1545)
  [SYCL] [xmethods] Allow replacing xmethod script (intel#1532)
  [SYCL] Add tests for inline asm feature (intel#1444)
  [SYCL][Doc] Add device_specific_kernel_queries extension. (intel#1540)
  [SYCL][USM] Remove unused header and unnecessary includes (intel#1537)
  Fix check-llvm dependencies (intel#1547)
  [SYCL] Add __SYCL_EXPORT to declaration of contextSetExtendedDeleter (intel#1531)
  [SYCL][Doc] Add static local memory query extension. (intel#1539)
  [SYCL][Doc] Add sycl_bitcast extension (intel#1541)
  [SYCL][NFC] Temporarily disable sporadically failing test (intel#1533)
  [SYCL][NFC] Adjust codeowners for sycl directory (intel#1529)
  [SYCL] Fix processing of spec consts referenced twice (intel#1524)
  [SYCL] Use correct macro name in export.hpp (intel#1527)
  [Driver][NFC] Fix -help information for -Xs options (intel#1530)
  [SYCL][Doc] Add Graph Scheduler design documentation (intel#1457)
  [SYCL] Add diagnostics for long double in device code (intel#1512)
  [SYCL] Add a mutex to state-modifying program functions (intel#1204)
  [SYCL][Test] Add Devicelib tests (intel#1256)
  [SYCL] Refactor semantic checks for variable types (intel#1513)
React to comments
ad7489b 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
romanovvlad
romanovvlad previously approved these changes Apr 21, 2020
View reviewed changes
@alexbatashev alexbatashev requested a review from kbobrovs April 21, 2020 17:11
keryell
keryell reviewed Apr 22, 2020
View reviewed changes
Copy link
Contributor

@keryell keryell left a comment

Choose a reason for hiding this comment

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

Interesting.
Have you chosen a simple ABI numbering easy to look at which is bumped at each ABI change?

Add test for abi_check.py
cfb822d 
Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
@alexbatashev
Copy link
Contributor Author

Interesting.
Have you chosen a simple ABI numbering easy to look at which is bumped at each ABI change?

This PR only introduces ABI checks, without versioning scheme. The part of the documentation, that says the version must be updated, should be ignored for now.

@alexbatashev alexbatashev requested a review from kbobrovs April 22, 2020 10:59
kbobrovs
kbobrovs approved these changes Apr 23, 2020
View reviewed changes
Copy link
Contributor

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

LGTM

@bader
Copy link
Contributor

bader commented Apr 23, 2020 •
edited
Loading

@alexbatashev, could you merge your branch with the tip of the sycl branch, branch, please?
We just merged 937fec1 and I'd like to make sure it doesn't conflict with this patch.

@bader bader merged commit d8eb099 into intel:sycl Apr 23, 2020
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 26, 2020
Merge remote-tracking branch 'origin/sycl' into private/abatashe/user…
d120b47 
…_docs

* origin/sycl:
  [XPTI][Framework] Reference implementation of the Xpti framework to be used with instrumentation in SYCL (intel#1557)
  [SYCL] Initial ABI checks implementation (intel#1528)
  [SYCL] Support connection with multiple plugins (intel#1490)
  [SYCL] Add a new header file with the reduction class definition (intel#1558)
  [SYCL] Add test for SYCL kernels with accessor and spec constant (intel#1536)
vladimirlaz pushed a commit to vladimirlaz/llvm that referenced this pull request Apr 27, 2020
[SYCL] Initial ABI checks implementation (intel#1528)
464af48 
Includes basic layout test and symbols checker

Signed-off-by: Alexander Batashev <alexander.batashev@intel.com>
alexbatashev pushed a commit to alexbatashev/llvm that referenced this pull request Apr 29, 2020
Merge remote-tracking branch 'origin/sycl' into private/abatashe/lib_…
8092b9c 
…versioning

* origin/sycl:
  [XPTI][Framework] Reference implementation of the Xpti framework to be used with instrumentation in SYCL (intel#1557)
  [SYCL] Initial ABI checks implementation (intel#1528)
  [SYCL] Support connection with multiple plugins (intel#1490)
  [SYCL] Add a new header file with the reduction class definition (intel#1558)
  [SYCL] Add test for SYCL kernels with accessor and spec constant (intel#1536)
  [SYCL][CUDA] Move interop tests (intel#1570)
  [Driver][SYCL] Remove COFF object format designator for Windows device compiles (intel#1574)
  [SYCL] Fix conflicting visibility attributes (intel#1571)
  [SYCL][DOC] Update the SYCL Runtime Interface document with design details (intel#680)
  [SYCL] Improve image accessors support on a host device (intel#1502)
  [SYCL] Make queue's non-USM event ownership temporary (intel#1561)
  [SYCL] Added support of rounding modes for non-host devices (intel#1463)
  [SYCL] SemaSYCL significant refactoring (intel#1517)
  [SYCL] Support 0-dim accessor in handler::copy(accessor, accessor) (intel#1551)
@alexbatashev alexbatashev deleted the private/abatashe/basic_abi_checks branch July 28, 2021 06:44
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@keryell keryell keryell left review comments

@s-kanaev s-kanaev s-kanaev left review comments

@romanovvlad romanovvlad romanovvlad approved these changes

@kbobrovs kbobrovs kbobrovs approved these changes

@pvchupin pvchupin pvchupin approved these changes

@bader bader bader left review comments

@sergey-semenov sergey-semenov sergey-semenov left review comments

@vladimirlaz vladimirlaz Awaiting requested review from vladimirlaz

@andykaylor andykaylor Awaiting requested review from andykaylor

Assignees

@romanovvlad romanovvlad

@sergey-semenov sergey-semenov

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@alexbatashev @bader @keryell @romanovvlad @kbobrovs @sergey-semenov @pvchupin @s-kanaev