[SYCL] Introduce XPTI-based tooling for SYCL applications by alexbatashev · Pull Request #5389 · intel/llvm

alexbatashev · 2022-01-25T13:53:24Z

sycl-trace

Tracing tool that prints SYCL PI calls, analog to SYCL_PI_TRACE env variable.

Limitations:

No support for plugin load diagnostics
No support for device selector diagnostics
No support for graph tracing

sycl-prof

Dumps profiling information from SYCL runtime and saves it to Chrome-compatible JSON file

Limitations:

No support for "summary" mode.

sycl-sanitize

A CLI tool to facilitate development of DPC++ applications. Currently sycl-sanitizer is capable of the following diagnostics:

USM pointer memory leaks. Report is generated at the end of program execution and contains location of leaked pointer malloc call.
Construction of SYCL buffers from device or shared USM pointer. The tool will abort execution of application if it attempts to create a buffer from device or shared USM pointer.
Construction of SYCL buffer from host USM pointer of smaller size, than buffer range. The application will be terminated.

Common limitations to all tools:

All tools are only available on Linux

sycl-trace - tracing tool that prints SYCL PI calls, analog to SYCL_PI_TRACE env variable sycl-prof - dumps profiling information from SYCL runtime and saves it to Chrome-compatible JSON file sycl-sanitize - provides some diagnostics based on SYCL PI calls. Currently this tool can diagnose USM memory leaks and abuse of USM pointers in SYCL buffers

alexbatashev · 2022-01-25T14:11:28Z

Pending on #5391

sycl/tools/CMakeLists.txt

sycl/tools/pi-trace/CMakeLists.txt

sycl/tools/sycl-prof/CMakeLists.txt

keryell

I cannot wait using all these new tools!

sycl/tools/sycl-prof/collector.cpp

keryell · 2022-02-22T07:49:57Z

sycl/tools/sycl-prof/collector.cpp

+#include <thread>
+#include <unistd.h>
+
+unsigned long process_id() { return static_cast<unsigned long>(getpid()); }


Why unsigned long?

This part is very Linux-specific. I'll have to come up with a Windows variant, though.

keryell · 2022-02-22T07:51:14Z

sycl/tools/sycl-prof/collector.cpp

+                                          xpti::trace_event_data_t *,
+                                          uint64_t /*Instance*/,
+                                          const void *UserData) {
+  unsigned long TID = std::hash<std::thread::id>{}(std::this_thread::get_id());


Why not what is standard https://en.cppreference.com/w/cpp/utility/hash instead of unsigned long?

Standard type is size_t, made a change

sycl/tools/sycl-prof/collector.cpp

keryell · 2022-02-22T07:58:23Z

sycl/tools/sycl-prof/writer.hpp

+public:
+  explicit JSONWriter(const std::string &OutPath) : MOutFile(OutPath) {}
+
+  void init() final {


Are you worried by your neighbors with these final? :-)

Choosing between override and final I figured final is more suitable here...

keryell · 2022-02-22T08:03:07Z

sycl/tools/sycl-sanitize/collector.cpp

+                                     const char * /*version_str*/,
+                                     const char *StreamName) {
+  if (std::string_view(StreamName) == "sycl.pi.debug") {
+    GS = new GlobalState;


Do you really need some raw memory?

Yes. Some SYCL users use SYCL APIs in global scope, and to work correctly, GlobalState has to outlive everything else in the application. Pointer is trivially destructible, so it'll be alive as long as process is alive. And it'll allow us to capture all the calls.

keryell · 2022-02-22T08:05:40Z

sycl/tools/sycl-sanitize/main.cpp

+
+  Args.push_back(nullptr);
+
+  int Err = launch(TargetExecutable.c_str(), Args, NewEnv);


Perhaps launch should have an STL friendly interface instead of having all these raw pointers?

Does it look better now?

PS Perhaps STL should have a process launch function?

keryell · 2022-02-22T08:07:01Z

sycl/tools/sycl-trace/main.cpp

+
+  Args.push_back(nullptr);
+
+  int Err = launch(TargetExecutable.c_str(), Args, NewEnv);


Perhaps LLVM library comes with such a function?

It does not, unfortunately... Would be great to have one, though

@Ralender you told me about something in this area.

sycl/tools/xpti_helpers/launch.hpp

* upstream/sycl: (2757 commits) [SYCL][Doc] Fixing incorrect merge of community Readme.md with our version (intel#5636) [SYCL] Change USM pooling parameters. (intel#5457) [CI] Fix cache location on Windows (intel#5603) [SYCL][NFC] Fix a warning about uninitialized struct members (intel#5610) [Buildbot] Update Windows GPU version to 101.1340 (intel#5620) Fix SPIRV -> OCL barrier call argument attributes Move SPV_INTEL_memory_access_aliasing tokens from spirv_internal [SYCL][ESIMD] Add support for named barrier APIs (intel#5583) [SYCL][L0] Remove ZeModule when program build failed (intel#5541) [SYCL] Silence "unknown attribute" warning for `device_indirectly_callable` (intel#5591) [SYCL][DOC] Introductory material for extensions (intel#5605) [SYCL][DOC] Change extension names to lower case (intel#5607) [SYCL] Improve get_kernel_bundle performance (intel#5496) [SYCL] Do not build device code for sub-devices (intel#5240) [sycl-post-link] Fix a crash during spec-constant properties generation (intel#5538) [SYCL][DOC] Move SPIR-V and OpenCL extensions (intel#5578) [SYCL][ESIMD][EMU] Update memory intrinsics for ESIMD_EMU plugin (intel#4748) [CI] Allow stale issue bot to analyze more issues (intel#5602) [SYCL][L0] Honor property::queue::enable_profiling (intel#5543) [OpenMP] Properly save strings when doing LTO ...

bader · 2022-03-01T13:45:52Z

@keryell, any objections to merge this PR?

bader · 2022-03-05T07:53:29Z

@keryell, any objections to merge this PR?

Any post-commit comments are still welcome.

bader · 2022-03-05T09:22:39Z

@alexbatashev, could you fix post-commit, please?
https://github.com/intel/llvm/runs/5431759815?check_suite_focus=true

keryell · 2022-03-05T17:42:08Z

Any post-commit comments are still welcome.

Sure! :-)
But I am fine with this PR. Sorry for the feedback delay.

alexbatashev requested a review from vladimirlaz January 25, 2022 14:18

vladimirlaz reviewed Jan 26, 2022

View reviewed changes

Alexander Batashev added 2 commits February 7, 2022 18:56

some fixes

9da103d

Merge remote-tracking branch 'upstream/sycl' into sycl_prof_tool

dd58467

alexbatashev marked this pull request as ready for review February 7, 2022 15:59

alexbatashev requested a review from a team as a code owner February 7, 2022 15:59

More fixes

ac51c0a

alexbatashev requested a review from vladimirlaz February 18, 2022 09:44

vladimirlaz previously approved these changes Feb 21, 2022

View reviewed changes

keryell reviewed Feb 22, 2022

View reviewed changes

alexbatashev added 2 commits February 23, 2022 09:48

address comments

b6a5997

alexbatashev dismissed vladimirlaz’s stale review via b6a5997 February 23, 2022 07:14

alexbatashev added 2 commits February 23, 2022 10:32

more comments

8956b5c

clang-format

6dcafe5

alexbatashev requested a review from vladimirlaz February 23, 2022 07:44

This was referenced Feb 23, 2022

[SYCL] WA for RT hang when exit(1) unexpectedly called in backend & SYCL_PI_TRACE enabled #5630

Closed

[SYCL] Introduce sycl-sanitizer tool #4174

Closed

vladimirlaz approved these changes Feb 28, 2022

View reviewed changes

bader merged commit 789d138 into intel:sycl Mar 5, 2022

alexbatashev deleted the sycl_prof_tool branch April 1, 2022 12:09

bader mentioned this pull request Jul 26, 2022

[SYCL][ABI-Break] Re-design the handling of optional code location argument for USM calls and honor memory allocation properties #6474

Merged


		Args.push_back(nullptr);

		int Err = launch(TargetExecutable.c_str(), Args, NewEnv);

Comments

Conversation

alexbatashev commented Jan 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

sycl-trace

sycl-prof

sycl-sanitize

Uh oh!

alexbatashev commented Jan 25, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

keryell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bader commented Mar 1, 2022

Uh oh!

bader commented Mar 5, 2022

Uh oh!

bader commented Mar 5, 2022

Uh oh!

keryell commented Mar 5, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexbatashev commented Jan 25, 2022 •

edited

Loading