Skip to content

[SYCL] Repurpose SYCL_CACHE_TRACE to enable fine-grained tracing of SYCL caches #15822

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

Merged
merged 8 commits into from
Oct 28, 2024

Conversation

uditagarwal97
Copy link
Contributor

@uditagarwal97 uditagarwal97 commented Oct 23, 2024

Currently, we use SYCL_CACHE_TRACE for events in persistent cache only. This PR repurposes SYCL_CACHE_TRACE to also enable tracing of in-memory cache and kernel_compiler.

After this change, SYCL_CACHE_TRACE will accept the following bit-masks:

Bit-mask Corresponding cache tracing
0x01 Enable tracing of persistent cache
0x02 Enable tracing of in-memory cache
0x04 Enable tracing of kernel_compiler cache

Any valid combination of the above bit-masks can be used to enable/disable tracing of the corresponding caches.

@@ -26,7 +26,7 @@ CONFIG(SYCL_PROGRAM_APPEND_COMPILE_OPTIONS, 64, __SYCL_PROGRAM_APPEND_COMPILE_OP
CONFIG(SYCL_HOST_UNIFIED_MEMORY, 1, __SYCL_HOST_UNIFIED_MEMORY)
// 260 (Windows limit) - 12 (filename) - 84 (cache directory structure)
CONFIG(SYCL_CACHE_DIR, 164, __SYCL_CACHE_DIR)
CONFIG(SYCL_CACHE_TRACE, 1, __SYCL_CACHE_TRACE)
CONFIG(SYCL_CACHE_TRACE, 16, __SYCL_CACHE_TRACE)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this, anyway? Doesn't it set the default values for these environment vars? But, if so, why was it set to 1 before?

Copy link
Contributor Author

@uditagarwal97 uditagarwal97 Oct 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good question. From what I understand, the syntax is CONFIG(CONFIG_NAME, VALUE_MAX_SIZE_IN_BYTES, COMPILE_TIME_CONFIG_NAME) and VALUE_MAX_SIZE_IN_BYTES is the maximum amount of bytes to read when we read this environment variable from the configuration file.

I do not think there's a default - if I remove this line, I get the following error:

llvm/sycl/source/detail/config.hpp:711:30: error: ‘SYCL_CACHE_TRACE’ was not declared in this scope; did you mean ‘SYCL_UR_TRACE’?
  711 | template <> class SYCLConfig<SYCL_CACHE_TRACE> {

Now, regarding the size, I think it was 1 earlier because this flag was just accepting a boolean - to disable/enable tracing of disk cache. Now, it is accepting an integer so we need to updated the size accordingly. I used 16 bytes previously because I was copying what SYCL_UR_TRACE did. But, now that I think more, I guess 4 bytes should be sufficient.

Co-authored-by: Steffen Larsen <steffen.larsen@intel.com>
Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs LGTM! Tag @jopperm & @sommerlukas for awareness.

@uditagarwal97
Copy link
Contributor Author

@intel/llvm-gatekeepers the PR is ready to be merged.

@sarnex sarnex merged commit eed2d03 into intel:sycl Oct 28, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants