[SYCL][XPTI] Enable PI calls notifications with arguments #3973

alexbatashev · 2021-06-22T10:37:44Z

This PR adds capability to capture PI calls' arguments and pass this data to XPTI subscribers. The sample pi-trace library demonstrates how one can parse this data on the subscriber side. An example usage of this utility would be:

XPTI_TRACE_ENABLE=1 XPTI_FRAMEWORK_DISPATCHER=lib/libxptifw.so XPTI_SUBSCRIBERS=lib/libpi_trace.so ./my_app

smaslov-intel · 2021-06-22T21:52:58Z

Thanks @alexbatashev ! This should help debugging issues at PI level.

XPTI_TRACE_ENABLE=1 XPTI_FRAMEWORK_DISPATCHER=lib/libxptifw.so XPTI_SUBSCRIBERS=lib/libpi_trace.so ./my_app

That's a lot!
Can we build/ship a tool that will make it easy to use this functionality out-of-box?

sycl/source/detail/plugin.hpp

sycl/include/CL/sycl/detail/pi_api_id.hpp

tovinkere · 2021-06-22T22:54:13Z

Thanks @alexbatashev ! This should help debugging issues at PI level.

XPTI_TRACE_ENABLE=1 XPTI_FRAMEWORK_DISPATCHER=lib/libxptifw.so XPTI_SUBSCRIBERS=lib/libpi_trace.so ./my_app

That's a lot!
Can we build/ship a tool that will make it easy to use this functionality out-of-box?

That's a possibility - or the construction of the tool could be hidden in a script.

alexbatashev · 2021-06-23T06:52:17Z

That's a lot!
Can we build/ship a tool that will make it easy to use this functionality out-of-box?

I hardly find the existing pi_trace anything but an API usage example. It is less functional than the SYCL_PI_TRACE=-1. However, if it evolves to something useful, it's not a big problem to wrap it into some python script.

sycl/include/CL/sycl/detail/pi_args_helper.hpp

* upstream/sycl: (489 commits) [SYCL][NFC] Lower overhead of making plugin calls (intel#3982) [SYCL][NFC] Use default macro initialization where applicable (intel#3979) [SYCL] Enable SPV_INTEL_fpga_invocation_pipelining_attributes extension (intel#3864) [SYCL] Disable reassociate pass to reduce register pressure (intel#3615) [Driver][SYCL][FPGA] Restrict -O0 for FPGA with hardware (intel#3966) [SYCL][NFC] Fix warnings coming out of SYCL headers. (intel#3978) [SYCL] Fix bugs with recursion in SYCL kernel (intel#3958) [SYCL][LevelZero] Add support to detect host->device and device->host transfers for USM (intel#3975) [SYCL] Enable native FP atomics by default (intel#3869) [sycl-post-link] Avoid copying from nullptr (intel#3963) [SYCL-PTX] Add warp-reduce path in sub-group reduce (intel#3949) [BuildBot] Uplift CPU/FPGAEMU RT version for CI Process (intel#3946) Fix handling of complex constant expressions Handle OpSpecConstantOp with CompositeExtract and CompositeInsert Handle OpSpecConstantOp with VectorShuffle [FuncSpec] Don't specialise functions with NoDuplicate instructions. [mlir][linalg] Support low padding in subtensor(pad_tensor) lowering [gn build] Port 208332d [AMDGPU] Add Optimize VGPR LiveRange Pass. [mlir][Linalg] NFC - Drop unused variable definition. ...

romanovvlad

LGTM, but still suggest adding a test.

smaslov-intel · 2021-07-01T15:17:21Z

That's a lot!
Can we build/ship a tool that will make it easy to use this functionality out-of-box?

I hardly find the existing pi_trace anything but an API usage example. It is less functional than the SYCL_PI_TRACE=-1. However, if it evolves to something useful, it's not a big problem to wrap it into some python script.

Why are you evolving it then, if you don't think it is useful?

sycl/include/CL/sycl/detail/pi.def

alexbatashev · 2021-07-01T18:08:02Z

That's a lot!
Can we build/ship a tool that will make it easy to use this functionality out-of-box?

I hardly find the existing pi_trace anything but an API usage example. It is less functional than the SYCL_PI_TRACE=-1. However, if it evolves to something useful, it's not a big problem to wrap it into some python script.

Why are you evolving it then, if you don't think it is useful?

I'm not saying it's useless. It's just not mature to compete with SYCL_PI_TRACE (yet). So, I don't expect many users for now. In future pi_trace can very well grow more functionality (and replace the environment variable, if that makes sense).

* upstream/sycl: (649 commits) [SYCL][Driver][NFC] Update integration footer test for 32-bit host (intel#4039) [SYCL][L0] Initialize descriptor .stype and .pNext (intel#4032) [SYCL] Add sycl::kernel::get_kernel_bundle method (intel#3855) [SYCL] Add support for device UUID as a SYCL extension. (intel#3696) [SYCL][Matrix] Add spec document for the matrix extension interface and its first implementation for AMX (intel#3551) Fix debug build mangler test after PR#3992 (8f38045). (intel#4033) [Driver][SYCL] Restrict user -include file in final integration footer step (intel#4036) [SYCL] [Tests] Do not copy device binary image mocks (intel#4023) [SYCL][Doc] Update docs to reflect new compiler features (intel#4030) [SYCL][CUDA] cl_khr_fp16 extension connected to cuda PI. (intel#4029) [SYCL][NFC] Refactor RT unit tests (intel#4021) [SYCL] Switch to using integration footer by default (intel#3777) [SYCL][CUDA] Add the Use Default Stream property (intel#4004) Uplift GPU RT version for Linux to 21.24.20098 (intel#4003) [SYCL][CUDA] atomic_ref.fetch_add used for fp64 reduction if device.has(atomic64) (intel#3950) [Driver][SYCL] Differentiate host dependency link from regular host link (intel#4002) [SYCL][ESIMD] Support device half type in intrinsics. (intel#4024) [SYCL] Allow fpga_reg only for PODs and Trivially-copyable structs (intel#3643) [SYCL][FPGA] Restore legacy debug info version for the hardware (intel#3991) [SYCL][PI][L0] Force reset of memcpy command-list. (intel#4001) ...

smaslov-intel

LGTM, thanks

alexbatashev · 2021-07-06T18:41:32Z

@tovinkere could you please take a look? Your approval is required to merge this patch.

tovinkere

LGTM, thanks.

sycl/include/CL/sycl/detail/common.hpp

sycl/include/CL/sycl/detail/pi_api_id.hpp

sycl/source/detail/plugin.hpp

tovinkere · 2021-07-06T20:10:57Z

xpti/include/xpti_data_types.h

@@ -255,6 +269,10 @@ enum class trace_point_type_t : uint16_t {
  function_end = XPTI_TRACE_POINT_END(12),
  /// Use to notify that a new metadata entry is available for a given event
  metadata = XPTI_TRACE_POINT_BEGIN(13),
+  /// Used to trace function call begin and its arguments.
+  function_with_args_begin = XPTI_TRACE_POINT_BEGIN(14),


Just FYI - In the future, you don't need to extend this enum. You can extend it as a "user_defined" trace point.

alexbatashev · 2021-07-15T12:21:42Z

@tovinkere friendly ping

tovinkere

LGTM

alexbatashev · 2021-07-16T06:40:17Z

The pre-commit failure is not related to this patch.
@bader could you please review/merge?

)" This reverts commit e239fdf.

)" (#4144) This reverts commit e239fdf. The patch introduces build failures like this: llvm/sycl/tools/pi-trace/pi_trace.cpp:13:10: fatal error: xpti_trace_framework.h: No such file or directory 13 | #include "xpti_trace_framework.h" | ^~~~~~~~~~~~~~~~~~~~~~~~ compilation terminated.

This patch re-lands #3973 with fixes to #4137 ---- This PR adds capability to capture PI calls' arguments and pass this data to XPTI subscribers. The sample pi-trace library demonstrates how one can parse this data on the subscriber side. An example usage of this utility would be: ```bash XPTI_TRACE_ENABLE=1 XPTI_FRAMEWORK_DISPATCHER=lib/libxptifw.so XPTI_SUBSCRIBERS=lib/libpi_trace.so ./my_app ```

alexbatashev added 10 commits June 19, 2021 19:06

Initial argument handling implementation

ad32ada

A bit refined implementation

81a134e

improved comment

9b6d5a1

Minor cleanup

ae4c35d

more cleanup

0d3a580

more stylistic changes

a5792a2

more file headers

a283c03

clang-format

08702b5

remove unused include

df846f9

fix build

3603bda

alexbatashev requested review from andykaylor, smaslov-intel, tovinkere and a team as code owners June 22, 2021 10:37

alexbatashev added 2 commits June 22, 2021 13:44

More clang-format

ab7fba6

Add arguments for function end as well

cb248c1

smaslov-intel reviewed Jun 22, 2021

View reviewed changes

sycl/source/detail/plugin.hpp Show resolved Hide resolved

smaslov-intel reviewed Jun 22, 2021

View reviewed changes

sycl/include/CL/sycl/detail/pi_api_id.hpp Outdated Show resolved Hide resolved

alexbatashev added 3 commits June 23, 2021 10:08

get rid of piapiid

3118d0c

clang-format

571b6c4

more clang-format

71805c1

romanovvlad reviewed Jun 23, 2021

View reviewed changes

sycl/include/CL/sycl/detail/pi_args_helper.hpp Outdated Show resolved Hide resolved

alexbatashev added 3 commits June 24, 2021 10:16

address more feedback

3151a63

make CI a bit happier

9775db1

alexbatashev requested review from romanovvlad and smaslov-intel June 24, 2021 09:07

romanovvlad reviewed Jul 1, 2021

View reviewed changes

smaslov-intel reviewed Jul 1, 2021

View reviewed changes

sycl/include/CL/sycl/detail/pi.def Outdated Show resolved Hide resolved

alexbatashev added 2 commits July 2, 2021 11:09

more feedback

47ee4c4

alexbatashev requested review from romanovvlad and smaslov-intel July 5, 2021 13:29

use template magic instead of changing pi.def

57e287f

smaslov-intel previously approved these changes Jul 6, 2021

View reviewed changes

alexbatashev dismissed smaslov-intel’s stale review via 2e3321b July 6, 2021 18:40

clang-format

2e3321b

tovinkere previously approved these changes Jul 6, 2021

View reviewed changes

alexbatashev dismissed tovinkere’s stale review via 0b08a09 July 8, 2021 17:47

slight changes to xpti interfaces

0b08a09

alexbatashev requested review from tovinkere and smaslov-intel July 8, 2021 17:48

smaslov-intel approved these changes Jul 8, 2021

View reviewed changes

tovinkere approved these changes Jul 15, 2021

View reviewed changes

alexbatashev mentioned this pull request Jul 16, 2021

[SYCL][XPTI] Pass plugin information to subscribers #4121

Merged

bader merged commit e239fdf into intel:sycl Jul 16, 2021

alexbatashev deleted the xpti_pi_args branch July 16, 2021 10:57

bader added a commit that referenced this pull request Jul 19, 2021

Revert "[SYCL][XPTI] Enable PI calls notifications with arguments (#3973

03c3035

)" This reverts commit e239fdf.

bader mentioned this pull request Jul 19, 2021

Revert "[SYCL][XPTI] Enable PI calls notifications with arguments" #4144

Merged

alexbatashev mentioned this pull request Jul 20, 2021

[SYCL][XPTI] Enable PI calls notifications with arguments #4148

Merged

[SYCL][XPTI] Enable PI calls notifications with arguments #3973

[SYCL][XPTI] Enable PI calls notifications with arguments #3973

Uh oh!

Conversation

alexbatashev commented Jun 22, 2021

Uh oh!

smaslov-intel commented Jun 22, 2021

Uh oh!

Uh oh!

Uh oh!

tovinkere commented Jun 22, 2021

Uh oh!

alexbatashev commented Jun 23, 2021

Uh oh!

Uh oh!

romanovvlad left a comment

Choose a reason for hiding this comment

Uh oh!

smaslov-intel commented Jul 1, 2021

Uh oh!

Uh oh!

alexbatashev commented Jul 1, 2021

Uh oh!

smaslov-intel left a comment

Choose a reason for hiding this comment

Uh oh!

alexbatashev commented Jul 6, 2021

Uh oh!

tovinkere left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tovinkere Jul 6, 2021

Choose a reason for hiding this comment

Uh oh!

alexbatashev commented Jul 15, 2021

Uh oh!

tovinkere left a comment

Choose a reason for hiding this comment

Uh oh!

alexbatashev commented Jul 16, 2021

Uh oh!

Uh oh!