Skip to content

[SYCL][CUDA][HIP] Update CUDA and HIP libspirv file diagnostic errors #4804

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 2 commits into from
Oct 26, 2021

Conversation

AidanBeltonS
Copy link
Contributor

@AidanBeltonS AidanBeltonS commented Oct 22, 2021

This patch updates CUDA and HIP's diagnostic errors for libspirv.
Currently, they do not reflect that the libspirv file becomes remangled and is looking for different variants depending upon the OS.
The HIP error also throws the same error as CUDA creating an incorrect message.

This patch resolves these two problems and creates adds HIP tests for the error messages.

This is proposed as a solution to fix #4370

@bader bader added cuda CUDA back-end hip Issues related to execution on HIP backend. libclc libclc project related issues and removed libclc libclc project related issues labels Oct 22, 2021
"cannot find 'libspirv-nvptx64--nvidiacl.bc'; provide path to libspirv "
"library via '-fsycl-libspirv-path', or pass '-fno-sycl-libspirv' to build "
"without linking with libspirv">;
def err_drv_no_sycl_cuda_libspirv : Error<
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you pass file names to the diagnostic and combine the diagnostics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that is a good idea. I have combined the diagnostics and passed the target name.

"libspirv library via '-fsycl-libspirv-path' or pass '-fno-sycl-libspirv' to "
"build without linking with libspirv. Provide path to "
"'remangled-l64-signed_char.libspirv-nvptx64--nvidiacl.bc' for Linux "
"or 'remangled-l32-signed_char.libspirv-nvptx64--nvidiacl.bc' for Windows">;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the 'remangled-... ' part supposed to be a part of this diagnostic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

remangled-... is part of the diagnostic as that is the file name. This is now being passed to the diagnostic and is not hardcoded any longer.

Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@smanna12 smanna12 left a comment

Choose a reason for hiding this comment

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

LGTM.

@bader bader requested a review from steffenlarsen October 26, 2021 11:38
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.

This is definitely a much better diagnostic message. LGTM!

@bader bader merged commit c54c605 into intel:sycl Oct 26, 2021
@AidanBeltonS AidanBeltonS deleted the remangled-variant-messages branch October 26, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cuda CUDA back-end hip Issues related to execution on HIP backend.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clearer error message for missing libspirv library in CUDA and ROCm targets
7 participants