Skip to content

[SYCL][CUDA] Add support of CUDA XPTI tracing #6373

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 21 commits into from
Sep 15, 2022

Conversation

pgorlani
Copy link
Contributor

@pgorlani pgorlani commented Jun 29, 2022

Fork of #5797

This patch rebases and finalizes (similarly to #6023) the draft in #5797, which already contained the most important commits thanks to @alexbatashev.

The most relevant additions of this patch were done in CMake files, in particular

  • switch from FindCUDA CMake (deprecated) module to the FindCUDAToolkit one in order to find cupti library by means of CUDA_cupti_LIBRARY. This is advisable because on some systems FindCUDA fails to find CUDA_cupti_LIBRARY. This is also the case of the CI, see the log in case of FindCUDA is used.
  • find generated_cuda_meta.h for generating the CUDA printer definitions, since the location of this header file seems to vary depending on the system, in case of this file is not found a warning is printed and no errors are thrown.

@pgorlani pgorlani requested review from a team as code owners June 29, 2022 14:01
@pgorlani pgorlani requested a review from smaslov-intel June 29, 2022 14:01
@pgorlani
Copy link
Contributor Author

pgorlani commented Jun 29, 2022

Unfortunately, I realized that FindCUDAToolkit module was introduced in CMake 3.17, whereas the minimum version required by SYCL is 3.14.

The CI seems to fail for this reason, see logs.

@pgorlani pgorlani force-pushed the cuda_tracing_rebased_PR branch 2 times, most recently from b3bc957 to be113ab Compare June 29, 2022 16:39
@pgorlani
Copy link
Contributor Author

pgorlani commented Jul 5, 2022

Problem: FindCUDA CMake module (deprecated since 3.10) fails to find the CUPTI library on some systems.

There are two possible solutions.

S1: Use FindCUDAToolkit module since it seems to have a more consistent behavior on the systems I tested so far. Unfortunately, there is a problem. This module was introduced in CMake 3.17, whereas the minimum CMake version required by SYCL is 3.14. This solution implies the increasing of the minimum required CMake version to 3.17. This could be tricky since the default CMake version in Ubuntu 20.04 repositories is 3.16.

S2: Accept the fact that in some systems the CUPTI library cannot be found by FindCUDA. In this case, it would be possible

  1. print a warning explaining to the users that they can set manually the path to the library,
  2. continue the configuration/compilation without errors at the cost of disabling the tracing for CUDA.

This solution implies modifying/guarding different parts of the code in order to deal with the lack of the CUPTI library symbols.

@bader
Copy link
Contributor

bader commented Jul 10, 2022

S3: we can replicate the part of FindCUDAToolkit that is missing in FindCUDA. Usually Find* are doing quite simple actions like checking if files with predefined names exist in expected locations, set CMake and/or environment variables. Is difficult to duplicate FindCUDAToolkit CMake 3.17 in our own CMake files while we use older version.

BTW, what is the recommended solution for versions between CMake 3.10 (FindCUDA is deprecated) and CMake 3.17 (FindCUDAToolkit is introduced). There must be some other way to configure CUDA libraries for the version we use. Right?

@pgorlani
Copy link
Contributor Author

Thanks @bader for S3, I implemented it in the last commit and it is working.

I had a look but it seems that between CMake 3.10 and 3.17 there is not a direct replacement of FindCUDA module. I can only assume that it was deprecated in 3.10 in order to direct users to the enable_language(CUDA) added in 3.8. This doesn't fit our needs nor solve our problems with CUPTI, see this.

Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

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

LGTM with one refactoring suggestion.

@bader bader changed the title [SYCL] [CUDA] CUDA XPTI tracing [SYCL][CUDA] Add support of CUDA XPTI tracing Jul 12, 2022
bader
bader previously approved these changes Jul 12, 2022
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.

Looks good. Only a small question.

@bader bader merged commit 0cd0414 into intel:sycl Sep 15, 2022
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