[SYCL][Doc] Update ITT instrumentation docs #4503

alexbatashev · 2021-09-07T11:28:38Z

Add more details about ITT annotations implementation in SYCL runtime and update feature controls documentation.

sycl/doc/extensions/ITTAnnotations/ITTAnnotations.rst

Co-authored-by: Alexey Bader <[email protected]>

sycl/doc/extensions/ITTAnnotations/ITTAnnotations.rst

gmlueck

Is this document really a DPC++ extension specification? It doesn't describe any API that the user can call from a DPC++ application. Is it supposed to be a design document instead? If so, maybe it should be moved outside of the "extensions" folder.

sycl/doc/extensions/ITTAnnotations/ITTAnnotations.rst

alexbatashev · 2021-09-08T18:20:49Z

I moved the docs out of extension directory, converted it to markdown to match other design documents, added it to index page, and mentioned the env var on a special page.

pvchupin · 2021-09-08T21:02:12Z

sycl/doc/EnvironmentVariables.md

@@ -47,6 +47,7 @@ subject to change. Do not rely on these variables in production code.
 | `SYCL_CACHE_THRESHOLD` | Positive integer | Cache eviction threshold in days (default value is 7 for 1 week). Set to 0 for disabling time-based cache eviction. |
 | `SYCL_CACHE_MIN_DEVICE_IMAGE_SIZE` | Positive integer | Minimum size of device code image in bytes which is reasonable to cache on disk because disk access operation may take more time than do JIT compilation for it. Default value is 0 to cache all images. |
 | `SYCL_CACHE_MAX_DEVICE_IMAGE_SIZE` | Positive integer | Maximum size of device image in bytes which is cached. Too big kernels may overload disk too fast. Default value is 1 GB. |
+| `INTEL_ENABLE_OFFLOAD_ANNOTATIONS` | Any(\*) | Enables ITT Annotations support for SYCL runtime. This variable should only be used by tools, that support ITT Annotations. |


Let's start splitting USER vars and special/experimental use vars and state clearly that these must not be used by end users.

I have created a separate file for env variables, that are not meant to be accessed by SYCL users. A separate file is needed so that we can put two distinct entries in https://intel.github.io/llvm-docs/. When this patch is merged, I plan to move rest of the debug variables from this file to the new one and move Environment Variables item to Using oneAPI DPC++ for Application Development section.

@pvchupin, I'm not sure why and how DPC++ should differentiate environment variables based on whom they are intended for "human" users or "tools". Could you clarify how do we define "end users"?

@alexbatashev, I think adding another table for environment variables intended for "tools" should resolve this comment and IMHO is better than adding a separate document.

@bader,
My comment was mostly about "experimental/internal-use-only" vs. "production", rather than "tools" vs. "human".
I believe disclaimer we currently have is not working well anymore... some of these are used widely and some of these must never be used.
Separating these to different files sounds like a good idea. Would be good to split and categorize them clearly and draw the line more explicitly.
I would suggest to keep EnvironmentVariables.md (as it was referred many times in the past) and move the most of the variables to InternalEnvironmentVariables.md.

My comment was mostly about "experimental/internal-use-only" vs. "production", rather than "tools" vs. "human".
I believe disclaimer we currently have is not working well anymore... some of these are used widely and some of these must never be used.

I believe variables that "must never be used" must be removed.
I also suggest disabling by default (what I think you mean by "production") features which are "experimental/internal-use-only". There should be an option to enable those if needed, but this must be conscious action.

I don't see how moving around documentation can solve the usage problem. If feature is available, it's going to be used by someone eventually.

Somebody added them, so I assume there was/is "experimental" reason at least. It's probably fine to disable by default or remove some, on the other hand there could be cases where experiment can be done randomly on existing build, e.g. during triaging.
I believe there are many vars now and categorizing them properly should help to at least some extent.

@pvchupin @bader I propose the following: I will move this variable back to EnvironmentVariables.md, and we will keep this discussion offline. I will prepare a followup patch once we have a decision.

Ok. Thanks.

romanovvlad · 2021-09-09T11:42:57Z

sycl/doc/InternalEnvironmentVariables.md

+# Internal Environment Variables
+
+**Warning:** the environment variables described in this document are used for
+development and debugging of DPC++ compiler and runtime. Their semantics are


Suggested change

development and debugging of DPC++ compiler and runtime. Their semantics are

development and debugging of DPC++ compiler and runtime or for interaction with other tools. Their semantics are

pvchupin · 2021-09-09T22:50:50Z

sycl/doc/EnvironmentVariables.md

@@ -47,6 +47,7 @@ subject to change. Do not rely on these variables in production code.
 | `SYCL_CACHE_THRESHOLD` | Positive integer | Cache eviction threshold in days (default value is 7 for 1 week). Set to 0 for disabling time-based cache eviction. |
 | `SYCL_CACHE_MIN_DEVICE_IMAGE_SIZE` | Positive integer | Minimum size of device code image in bytes which is reasonable to cache on disk because disk access operation may take more time than do JIT compilation for it. Default value is 0 to cache all images. |
 | `SYCL_CACHE_MAX_DEVICE_IMAGE_SIZE` | Positive integer | Maximum size of device image in bytes which is cached. Too big kernels may overload disk too fast. Default value is 1 GB. |
+| `INTEL_ENABLE_OFFLOAD_ANNOTATIONS` | Any(\*) | Enables ITT Annotations support for SYCL runtime. This variable should only be used by tools, that support ITT Annotations. |


@bader,
My comment was mostly about "experimental/internal-use-only" vs. "production", rather than "tools" vs. "human".
I believe disclaimer we currently have is not working well anymore... some of these are used widely and some of these must never be used.
Separating these to different files sounds like a good idea. Would be good to split and categorize them clearly and draw the line more explicitly.
I would suggest to keep EnvironmentVariables.md (as it was referred many times in the past) and move the most of the variables to InternalEnvironmentVariables.md.

…pdate_itt_docs

pvchupin · 2021-09-16T22:09:31Z

sycl/doc/EnvironmentVariables.md

@@ -47,6 +47,7 @@ subject to change. Do not rely on these variables in production code.
 | `SYCL_CACHE_THRESHOLD` | Positive integer | Cache eviction threshold in days (default value is 7 for 1 week). Set to 0 for disabling time-based cache eviction. |
 | `SYCL_CACHE_MIN_DEVICE_IMAGE_SIZE` | Positive integer | Minimum size of device code image in bytes which is reasonable to cache on disk because disk access operation may take more time than do JIT compilation for it. Default value is 0 to cache all images. |
 | `SYCL_CACHE_MAX_DEVICE_IMAGE_SIZE` | Positive integer | Maximum size of device image in bytes which is cached. Too big kernels may overload disk too fast. Default value is 1 GB. |
+| `INTEL_ENABLE_OFFLOAD_ANNOTATIONS` | Any(\*) | Enables ITT Annotations support for SYCL runtime. This variable should only be used by tools, that support ITT Annotations. |


Ok. Thanks.

pvchupin · 2021-09-16T22:11:26Z

@alexbatashev, please fill PR description with what's changed in this patch (to be used as commit message on merge).

* upstream/sycl: (36 commits) [SYCL] Add SYCL2020 target::device enumeration value (intel#4587) [SYCL][Doc] Update ITT instrumentation docs (intel#4503) [SYCL][L0] Make all L0 events have device-visibility (intel#4534) [SYCL] Updated Level-Zero backend spec according to SYCL 2020 standard (intel#4560) [SYCL] Add error_code support for SYCL 1.2.1 exception classes (intel#4574) [SYCL][CI] Provide --ci-defaults option for config script (intel#4583) [CI] Switch GitHub Actions to Ubuntu 20.04 (intel#4582) [SYCL][CUDA] Fix context clearing in PiCuda tests (intel#4483) [SYCL] Hide SYCL service kernels (intel#4519) [SYCL][L0] Fix mismatched ZE call count (intel#4559) [SYCL] Remove function pointers extension (intel#4459) [GitHub Actions] Uplift clang version in post-commit validation (intel#4581) [SYCL] Ignore usm prefetch dummy flag (intel#4568) [SYCL][Group algorithms] Add group sorting algorithms implementation (intel#4439) [SYCL] Resolve name clash with a user defined symbol (intel#4570) [clang-offload-wrapper] Do not create .tgtimg section with -emit-reg-funcs=0 (intel#4577) [SYCL][FPGA] Remove deprecated attribute functionality (intel#4532) [SYCL] Remove _class aliases (intel#4465) [SYCL][CUDA][HIP] Report every device in its own platform (intel#4571) [SYCL][L0] make_device shouldn't need platform as an input (intel#4561) ...

[SYCL][Doc] Update ITT instrumentation docs

ffcc2d2

alexbatashev requested review from romanovvlad and vzakhari September 7, 2021 11:28

alexbatashev requested a review from a team as a code owner September 7, 2021 11:28

bader reviewed Sep 7, 2021

View reviewed changes

sycl/doc/extensions/ITTAnnotations/ITTAnnotations.rst Outdated Show resolved Hide resolved

Update sycl/doc/extensions/ITTAnnotations/ITTAnnotations.rst

a3571c5

Co-authored-by: Alexey Bader <[email protected]>

MrSidims reviewed Sep 7, 2021

View reviewed changes

sycl/doc/extensions/ITTAnnotations/ITTAnnotations.rst Show resolved Hide resolved

Update docs

5ea7a2f

MrSidims previously approved these changes Sep 7, 2021

View reviewed changes

gmlueck reviewed Sep 7, 2021

View reviewed changes

sycl/doc/extensions/ITTAnnotations/ITTAnnotations.rst Show resolved Hide resolved

Address feedback

7ae8283

alexbatashev dismissed MrSidims’s stale review via 7ae8283 September 8, 2021 18:18

alexbatashev requested a review from pvchupin as a code owner September 8, 2021 18:18

bader requested a review from gmlueck September 8, 2021 18:30

bader previously approved these changes Sep 8, 2021

View reviewed changes

gmlueck previously approved these changes Sep 8, 2021

View reviewed changes

pvchupin suggested changes Sep 8, 2021

View reviewed changes

Split env vars

d4c1fef

alexbatashev dismissed stale reviews from gmlueck and bader via d4c1fef September 9, 2021 11:24

romanovvlad reviewed Sep 9, 2021

View reviewed changes

apply suggesion from review

ac36b4e

pvchupin previously approved these changes Sep 9, 2021

View reviewed changes

romanovvlad dismissed pvchupin’s stale review via ac36b4e September 10, 2021 07:28

Alexander Batashev added 2 commits September 10, 2021 11:46

Merge branch 'sycl' into update_itt_docs

d379db8

update env vars

e0e0750

alexbatashev requested review from romanovvlad, pvchupin and bader September 16, 2021 07:05

alexbatashev requested review from MrSidims and gmlueck September 16, 2021 07:05

MrSidims previously approved these changes Sep 16, 2021

View reviewed changes

Merge branch 'update_itt_docs' of github.com:alexbatashev/llvm into u…

8644af6

…pdate_itt_docs

alexbatashev dismissed MrSidims’s stale review via 8644af6 September 16, 2021 11:31

bader approved these changes Sep 16, 2021

View reviewed changes

gmlueck approved these changes Sep 16, 2021

View reviewed changes

pvchupin approved these changes Sep 16, 2021

View reviewed changes

bader merged commit 9c0508b into intel:sycl Sep 17, 2021

alexbatashev deleted the update_itt_docs branch September 17, 2021 10:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL][Doc] Update ITT instrumentation docs #4503

[SYCL][Doc] Update ITT instrumentation docs #4503

alexbatashev commented Sep 7, 2021 •

edited

Loading

gmlueck left a comment

alexbatashev commented Sep 8, 2021

pvchupin Sep 8, 2021

alexbatashev Sep 9, 2021

bader Sep 9, 2021

pvchupin Sep 9, 2021

bader Sep 10, 2021

pvchupin Sep 10, 2021

alexbatashev Sep 11, 2021

pvchupin Sep 16, 2021

romanovvlad Sep 9, 2021

pvchupin Sep 9, 2021

pvchupin Sep 16, 2021

pvchupin commented Sep 16, 2021

	development and debugging of DPC++ compiler and runtime. Their semantics are
	development and debugging of DPC++ compiler and runtime or for interaction with other tools. Their semantics are

[SYCL][Doc] Update ITT instrumentation docs #4503

[SYCL][Doc] Update ITT instrumentation docs #4503

Conversation

alexbatashev commented Sep 7, 2021 • edited Loading

gmlueck left a comment

Choose a reason for hiding this comment

alexbatashev commented Sep 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pvchupin commented Sep 16, 2021

alexbatashev commented Sep 7, 2021 •

edited

Loading