-
Notifications
You must be signed in to change notification settings - Fork 795
[SYCL][XPTI] Pass plugin information to subscribers #4121
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][XPTI] Pass plugin information to subscribers #4121
Conversation
1ca3f66
to
8060a4e
Compare
This creates new interface dependency between components, e.g. you can't reorder backend enum values now.
Is this normal for XPTI model to cross shared objects boundaries with some internal pointers? Shouldn't that come at least with some pointer lifetime management, such that it is not released before XPTI readers are done? Again, what is the motivation of this change? |
It’s part of SYCL ABI, you can’t reorder them anyway. As for strings, they can be expensive, if you need to do comparisons.
I don’t see why you’d need to save pointer to plugin somewhere. And it is guaranteed to be valid throughout callback lifetime, because they’re synchronous. Same thing is true about any other callback argument. My motivation is that memory transfers are asynchronous, and I need them to be synchronous in order to be able to store memory content. Thus, I put extra waits in callbacks. |
Why do you need backend ID at all in XPTI stream receiver?
Good point about other pointer arguments, so at least you aren't making things worse than they are already.
Can you point me to where you are doing so? I am curious how do you identify what to wait for, and avoid deadlocks (e.g. when some copies are dependent on some others, and waiting for them may cause no forward progress, I think). |
Seems like a useful information for debugging purpose. Applications may use more than one platform. Knowledge of particular backend, that executes PI call may help here.
Scheduler wouldn't enqueue a command, unless all dependencies are satisfied. Thus, I don't expect deadlocks. |
Why not defer adding it until we know how to use it? Also, if we wanted it, I think we should have this info in the "pi_plugin" structure, pointer to which you are passing already.
Really? Aren't we using events wait-lists for drivers to track/resolve dependencies? llvm/sycl/include/CL/sycl/detail/pi.h Line 1380 in a4165b3
|
I can remove it if nobody sees any use for it. Right now all I do with that argument is print it on the screen for debugging purpose. Maybe @tovinkere can suggest other usages here. What I'd like to avoid, though, is messing with pi_plugin structure. Since it's part of cross-library interface, it would be great to have some degree of stability here.
I don't see much use for those arguments. What if our dependency is a host task? Or an event from another plugin? So, we have to wait for particular events anyway. And even if we use this parameter somewhere, waiting on the output event will also wait on this list of events. So, it should be safe from that point of view. Besides, there're other usages for additional PI calls. For example, one may want to query pointer type to figure out data flow direction in memcpy. |
…ackend_plugin * upstream/sycl: (26 commits) [SPIR-V][NFC] Move non-upstreamed FuncParam decorations into internal:: (intel#4138) [SYCL] Move free function queries to experimental namespace (intel#4090) [SYCL][XPTI] Enable PI calls notifications with arguments (intel#4148) [SYCL] Revert queue::wait() to its old behaviour with Level Zero (intel#4153) [SYCL] Add missing <cstring> header to spirv.hpp (intel#4157) [SYCL] Adds info query for atomic_memory_order_capabilities on device and context (intel#4105) [SYCL] Improve performance of generic shuffles (intel#3815) [SYCL] Fix the error with namespaces caused during rebase of intel#4014 (intel#4151) [ESIMD] Fix 'ambiguous operator' error with length 1 simd operands (intel#4149) [libdevice][NFC] Fix libdevice dependencies list (intel#4130) [SPIR-V] Reland Encode debug info producer in SPIR-V (intel#4082) [SYCL][ROCm] Add ROCm support to get_device_count_by_type (intel#4113) [SYCL] Fix sRGB device info (intel#4145) [SYCL][ROCm] Fix kernel launch with multiple dimensions (intel#4063) [SYCL][ROCm] Fix compilation for AMD GPU with -fsycl-dead-args-optimization (intel#4126) [SYCL][Level Zero] Enable multi-CCS support. (intel#4038) [SYCL] Pass bound arch to unbundler (intel#4112) [ESIMD][doc] Added documentation for some ESIMD math APIs (intel#3995) [ESIMD] rename gather4/scatter4 to gather_rgba/scatter_rgba (intel#4120) [SYCL][NFC] Remove unused variable. (intel#4131) ...
@smaslov-intel what would be the final decision here? Should I remove |
Yes, please remove it and just pass the pi_plugin. |
…ackend_plugin * upstream/sycl: (755 commits) [SYCL] Add operator= to atomic_ref specializations (intel#4183) [SYCL] Make spelling of Debug value for CMAKE_BUILD_TYPE variable case insensitive (intel#4069) [SYCL][LIBCLC] Add atan and cbrt for amdgcn-amdhsa (intel#4180) [SYCL][CUDA] Correctly free managed memory (intel#4181) [SYCL] Revert barrier deprecation note (intel#4162) [SYCL][FPGA] Refactor of statement attributes (intel#4136) [Driver][SYCL] Enable way to emit int-footer source to a specific dir (intel#4167) [Driver] Fix default MSVC version setting for -fms-compatibilty-version (intel#4165) [BuildBot] Add llvm-enable-projects flag to configure.py (intel#4169) [Driver][SYCL][FPGA] Improve aocx archive processing for FPGA (intel#4160) [SYCL] Correct int-header emission with type aliases [SYCL] Fix name collisions in SYCL enums (intel#4154) [SYCL] Return the correct status info for host_task event (intel#4161) [ESIMD][NFC] Added tests for simd class type traits (intel#4146) [SYCL][ROCm] Fix missing parameter in ROCm plugin (intel#4166) [SYCL][L0] Add temporary option to allow user to use copy engine for device to device copy (intel#4127) Remove check for AMD HIP to fix Driver/cuda-arch-translation.cu Reapply after conflict resolution 418a6d6 "Fix nvptx_target_teams_distribute_parallel_for_simd_codegen failure" Revert "[SYCL] Removes redefinitions of macros in libclc (intel#3505)" [PGO] Change test-run line to check NewPM pass behavior ...
This patch makes the following additional information available to XPTI subscribers.
All streams:
sycl.pi.debug
stream:uint8_t
value ofsycl::backend
enum.