-
Notifications
You must be signed in to change notification settings - Fork 790
[SYCL] Implement device libraries: C assert #922
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 all commits
9c58843
6a8b3e7
bc335d7
b03b3e2
08a37e3
7b57b6b
462af28
52e077c
75c05b5
269d6dc
3d80fe0
7497a23
82e0e84
265aaf6
4ce3a57
278206f
a5d65db
6b67601
d585921
e882460
f1ae75f
a1aeda7
f3a4e07
27bcf97
45b9be8
73fdcda
9f1aecc
8e826ef
cf819f1
8b41e4a
423a054
42d52b8
2f61342
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,162 @@ | ||
C and C++ Standard libraries support | ||
=================================== | ||
|
||
This extension enables a set of functions from the C and C++ standard | ||
libraries, and allows to use them in SYCL device code. | ||
|
||
Function declarations are taken from the standard headers (e.g. from | ||
<assert.h> or <cassert>), and the corresponding header has to be | ||
explicitly included in user code. | ||
|
||
Implementation requires a special device library to be linked with a | ||
SYCL program. The library should match the C or C++ standard library | ||
used to compile the program: | ||
|
||
For example, on Linux with GNU glibc: | ||
.. code: | ||
clang++ -fsycl -c main.cpp -o main.o | ||
clang++ -fsycl main.o $(SYCL_INSTALL)/lib/libsycl-glibc.o -o a.out | ||
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. I think device library should be linked by default i.e. the same way OpenMP/CUDA compiler links device side library + driver option to disable linking device code. 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. Right. This requires changes in clang driver. |
||
|
||
or, in case of Windows: | ||
.. code: | ||
clang++ -fsycl -c main.cpp -o main.obj | ||
clang++ -fsycl main.obj %SYCL_INSTALL%/lib/libsycl-msvc.o -o a.exe | ||
|
||
List of supported functions from C standard library: | ||
- assert macro (from <assert.h> or <cassert>) | ||
|
||
NOTE: only the GNU glibc and Microsoft C libraries are currently | ||
supported. | ||
|
||
Example of usage | ||
================ | ||
|
||
.. code: c++ | ||
#include <assert.h> | ||
#include <CL/sycl.hpp> | ||
|
||
template <typename T, size_t N> | ||
void simple_vadd(const std::array<T, N>& VA, const std::array<T, N>& VB, | ||
std::array<T, N>& VC) { | ||
// ... | ||
cl::sycl::range<1> numOfItems{N}; | ||
cl::sycl::buffer<T, 1> bufferA(VA.data(), numOfItems); | ||
cl::sycl::buffer<T, 1> bufferB(VB.data(), numOfItems); | ||
cl::sycl::buffer<T, 1> bufferC(VC.data(), numOfItems); | ||
|
||
deviceQueue.submit([&](cl::sycl::handler& cgh) { | ||
auto accessorA = bufferA.template get_access<sycl_read>(cgh); | ||
auto accessorB = bufferB.template get_access<sycl_read>(cgh); | ||
auto accessorC = bufferC.template get_access<sycl_write>(cgh); | ||
|
||
cgh.parallel_for<class SimpleVadd<T>>(numOfItems, | ||
[=](cl::sycl::id<1> wiID) { | ||
accessorC[wiID] = accessorA[wiID] + accessorB[wiID]; | ||
assert(accessorC[wiID] > 0 && "Invalid value"); | ||
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. Is assert a good example? It relies on some kernel features not available in OpenCL... Are there real use cases?
? :-) 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.
Right, assert is an edge case and it is probably a bad example.
For assert? It is widely used as a sanity check in host code, so I don't see a reason why it cannot be used for device code as well.
That should work if you define the macro before
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. I am unsure that The macro 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.
There are two problems:
So we avoid this dance around
Agree. |
||
}); | ||
}); | ||
deviceQueue.wait_and_throw(); | ||
} | ||
|
||
Frontend | ||
======== | ||
|
||
Once the system header is included, the corresponding functions can be | ||
used in SYCL device code. This results in a handful of unresolved | ||
functions in LLVM IR after clang: | ||
|
||
.. code: | ||
; Function Attrs: noreturn nounwind | ||
declare dso_local spir_func void @__assert_fail(i8 addrspace(4)*, i8 addrspace(4)*, i32, i8 addrspace(4)*) | ||
|
||
[...] | ||
cond.false: | ||
call spir_func void @__assert_fail([...]) | ||
unreachable | ||
|
||
The C and C++ specifications do not define names and signatures of the | ||
functions from libc implementation that are used for a particular | ||
function. For example, the `assert` macro: | ||
|
||
- in Glibc and musl libraries it expands to `__assert_fail` | ||
- in MSVC library it expands to `_wassert` | ||
- in newlib library it expands to `__assert_func` | ||
|
||
This makes it difficult to handle all possible cases in device | ||
compilers. In order to facilitate porting to new platforms, and to | ||
avoid imposing a lot of boilerplate code in *every* device compiler, | ||
wrapper libraries are provided with the SYCL compiler that "lower" | ||
libc implementation-specific functions into a stable set of functions, | ||
that can be later handled by a device compiler. | ||
|
||
.. code: | ||
clang++ -fsycl -c main.cpp -o main.o | ||
clang++ -fsycl main.o $(SYCL_INSTALL)/lib/libsycl-glibc.o -o a.out | ||
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. Two points:
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. (1) should be explained on line 77. Let me know if I should elaborate. 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. (1) I understand that we should have definitions for 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.
This doesn't work for functions that have the same name, but different arguments (e.g. __assert_fail in glibc vs musl).
There are no shared symbols. If there is a function with identical name and signature, it will be duplicated in every wrapper library. In any case, I anticipate the wrapper libraries to go away soon. @s-kanaev is working on a header-based approach to provide the wrappers. |
||
|
||
This `libsycl-glibc.o` is one of these wrapper libraries: it provides | ||
definitions for glibc specific library function, and these definitions | ||
call the corresponding functions from `__devicelib_*` set of | ||
functions. | ||
|
||
For example, `__assert_fail` from IR above gets transformed into: | ||
.. code: | ||
; Function Attrs: noreturn nounwind | ||
declare dso_local spir_func void @__devicelib_assert_fail(i8 addrspace(4)*, i8 addrspace(4)*, i32, i8 addrspace(4)*) | ||
|
||
; Function Attrs: noreturn nounwind | ||
define dso_local spir_func void @__assert_fail(i8 addrspace(4)*, i8 addrspace(4)*, i32, i8 addrspace(4)*) { | ||
call spir_func void @__devicelib_assert_fail([...]) | ||
} | ||
|
||
[...] | ||
cond.false: | ||
call spir_func void @__assert_fail([...]) | ||
unreachable | ||
|
||
A single wrapper object provides function wrappers for *all* supported | ||
library functions. Every supported C library implementation (MSVC or | ||
glibc) has its own wrapper library object: | ||
|
||
- libsycl-glibc.o | ||
- libsycl-msvc.o | ||
|
||
SPIR-V | ||
====== | ||
|
||
Standard library functions are represented as external (import) | ||
functions in SPIR-V: | ||
|
||
.. code: | ||
8 Decorate 67 LinkageAttributes "__devicelib_assert_fail" Import | ||
... | ||
2 Label 846 | ||
8 FunctionCall 63 864 67 855 857 863 859 | ||
1 Unreachable | ||
|
||
Device compiler | ||
=============== | ||
|
||
Device compiler is free to implement these `__devicelib_*` functions. | ||
In order to indicate support for a particular set of functions, | ||
underlying runtime have to support the corresponding OpenCL (PI) | ||
extension. See ``DeviceLibExtensions.rst`` for a list of supported | ||
functions and corresponding extensions. | ||
|
||
Fallback implementation | ||
======================= | ||
|
||
If a device compiler does not indicate "native" support for a | ||
particular function, a fallback library is linked at JIT time by the | ||
SYCL Runtime. This library is distributed with the SYCL Runtime and | ||
resides in the same directory as the `libsycl.so` or `sycl.dll`. | ||
|
||
A fallback library is implemented as a device-agnostic SPIR-V program, | ||
and it is supposed to work for any device that supports SPIR-V. | ||
|
||
Every set of functions is implemented in a separate fallback | ||
library. For example, a fallback for `cl_intel_devicelib_cassert` | ||
extension is provided as `libsycl-fallback-cassert.spv` | ||
bader marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
NOTE that AOT compilation is not yet supported. Driver will have to | ||
check for extension support and link the corresponding SPIR-V fallback | ||
implementation, but this is not implemented yet. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
Device library extensions | ||
=================================== | ||
|
||
Device compiler that indicates support for a particular extension is | ||
supposed to support *all* the corresponding functions. | ||
|
||
cl_intel_devicelib_cassert | ||
========================== | ||
|
||
.. code: | ||
void __devicelib_assert_fail(__generic const char *expr, | ||
__generic const char *file, | ||
int32_t line, | ||
__generic const char *func, | ||
size_t gid0, size_t gid1, size_t gid2, | ||
size_t lid0, size_t lid1, size_t lid2); | ||
Semantic: | ||
the function is called when an assertion expression `expr` is false, | ||
and it indicates that a program does not execute as expected. | ||
The function should print a message containing the information | ||
provided in the arguments. In addition to that, the function is free | ||
to terminate the current kernel invocation. | ||
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. And if not, what happens next? 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. It returns to the caller, and the kernel continues. 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. Even if it launch a nuclear missile that should have been avoided? I would prefer the program not to compile at the first place!!! 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. I'd advise against using SYCL for anything that involves radioactive materials or weapons of mass destruction. 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.
Wait, have you guarded this just by an assertion? Not the best idea, IMO |
||
|
||
Arguments: | ||
|
||
- `expr` is a string representation of the assertion condition | ||
- `file` and `line` are the source code location of the assertion | ||
- `func` (optional, may be NULL) name of the function containing the assertion | ||
- `gidX` current work-item global id | ||
- `lidX` current work-item local id | ||
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. What happens if there is no such things (single_task and specific FPGA mode where the concepts are not even implemented in hardware)? 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. Global and local IDs should always be available, if we talk about standard OpenCL or SPIR-V. |
||
|
||
Example of a message: | ||
.. code: | ||
foo.cpp:42: void foo(int): global id: [0,0,0], local id: [0,0,0] Assertion `buf[wiID] == 0 && "Invalid value"` failed. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,66 @@ | ||
# Place device libraries near the libsycl.so library in a build | ||
# directory | ||
if (WIN32) | ||
set(binary_dir "${CMAKE_RUNTIME_OUTPUT_DIRECTORY}") | ||
else() | ||
set(binary_dir "${CMAKE_LIBRARY_OUTPUT_DIRECTORY}") | ||
endif() | ||
|
||
set(clang $<TARGET_FILE:clang>) | ||
|
||
set(compile_opts | ||
# suppress an error about SYCL_EXTERNAL | ||
-Wno-sycl-strict | ||
# for CL/__spirv/spirv_vars.hpp | ||
-I${sycl_inc_dir}) | ||
|
||
if (WIN32) | ||
set(devicelib-obj-file ${binary_dir}/libsycl-msvc.o) | ||
add_custom_command(OUTPUT ${devicelib-obj-file} | ||
COMMAND ${clang} -fsycl -c | ||
${compile_opts} | ||
${CMAKE_CURRENT_SOURCE_DIR}/msvc_wrapper.cpp | ||
-o ${devicelib-obj-file} | ||
MAIN_DEPENDENCY msvc_wrapper.cpp | ||
DEPENDS wrapper.h clang | ||
VERBATIM) | ||
else() | ||
set(devicelib-obj-file ${binary_dir}/libsycl-glibc.o) | ||
add_custom_command(OUTPUT ${devicelib-obj-file} | ||
COMMAND ${clang} -fsycl -c | ||
${compile_opts} | ||
${CMAKE_CURRENT_SOURCE_DIR}/glibc_wrapper.cpp | ||
-o ${devicelib-obj-file} | ||
MAIN_DEPENDENCY glibc_wrapper.cpp | ||
DEPENDS wrapper.h clang | ||
VERBATIM) | ||
endif() | ||
|
||
add_custom_command(OUTPUT ${binary_dir}/libsycl-fallback-cassert.spv | ||
COMMAND ${clang} -S -fsycl-device-only -fno-sycl-use-bitcode | ||
${compile_opts} | ||
${CMAKE_CURRENT_SOURCE_DIR}/fallback-cassert.cpp | ||
-o ${binary_dir}/libsycl-fallback-cassert.spv | ||
MAIN_DEPENDENCY fallback-cassert.cpp | ||
DEPENDS wrapper.h clang llvm-spirv | ||
VERBATIM) | ||
|
||
add_custom_target(devicelib-obj DEPENDS ${devicelib-obj-file}) | ||
add_custom_target(devicelib-spv DEPENDS ${binary_dir}/libsycl-fallback-cassert.spv) | ||
add_dependencies(sycl devicelib-obj devicelib-spv) | ||
if (MSVC) | ||
add_dependencies(sycld devicelib-obj devicelib-spv) | ||
endif() | ||
|
||
# Place device libraries near the libsycl.so library in an install | ||
# directory as well | ||
if (WIN32) | ||
set(install_dest bin) | ||
else() | ||
set(install_dest lib) | ||
endif() | ||
|
||
install(FILES ${devicelib-obj-file} | ||
${binary_dir}/libsycl-fallback-cassert.spv | ||
DESTINATION ${install_dest} | ||
COMPONENT sycl) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a single device library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, see the line 77 in this document.
Basically, different libraries may use the same function name (e.g. __assert_fail), but with different arguments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's the case for existing C/C++ standard library implementations.
Assuming that we want avoid potential issues with conflicting names, why do you propose "a special device library" instead of "a special header" as it's done for CUDA/OpenMP?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These little inconsistencies definitely exist.
For example, in musl C library,
__assert_fail
has an line argument of typeint
, and Glibc has this argument asunsigned int
.Even if we consider only one library (e.g. Glibc), I'm not sure that ABI between headers and the library will always remain stable, so future/past versions may bring similar problems.
Can you describe this "special header" approach using the assert function as an example?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, take a look here: https://github.com/intel/llvm/blob/sycl/clang/lib/Headers/__clang_cuda_runtime_wrapper.h#L335.
I suggest reading comments in https://github.com/intel/llvm/blob/sycl/clang/lib/Headers/__clang_cuda_* and take a look at https://github.com/intel/llvm/tree/sycl/clang/lib/Headers/cuda_wrappers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is hard to tell how this approach is going to co-exist with the rest of the standard library, and scale beyond a handful of functions. The code you refer to doesn't seem to support anything other than glibc and libc++/libstdc++.
In general, I think the most difficult part here is (1) to establish an interface with a device compiler (DevicelibExtensions.rst), and (2) wire standard library functions through that interface, instead of using library specific routines.
(2) can probably be done in a header file, but I don't see a huge benefit in comparison with the library approach. If you (or anyone reading this) see a benefit and know to to implement this with portability in mind - feel free to send a patch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this at least should be discussed with the clang community.
It should be safe to develop solution aligned with the existing approaches in the community code w/o constantly checking with the community, but in this case we went in slightly different direction.