Skip to content

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