[SYCL][CUDA][HIP AMD] Correct cuda/hip spirv libclc .bc and module triple.#7245
Merged
pvchupin merged 6 commits intointel:syclfrom Nov 2, 2022
Merged
[SYCL][CUDA][HIP AMD] Correct cuda/hip spirv libclc .bc and module triple.#7245pvchupin merged 6 commits intointel:syclfrom
pvchupin merged 6 commits intointel:syclfrom
Conversation
added 4 commits
October 31, 2022 10:32
Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
Contributor
Author
|
By the way, we would like to have this cherry-picked for the 2023.0 release. |
Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
Signed-off-by: JackAKirk <jack.kirk@codeplay.com>
romanovvlad
approved these changes
Nov 2, 2022
Contributor
Author
|
Thanks for reviewing. I've put it as draft since I may refactor it a bit before merging. Will get back to it shortly. |
Contributor
Author
cc @AerialMantis @romanovvlad @steffenlarsen @pvchupin I've moved it from draft so it can be merged for the 2023.0 cherry pick. We were thinking of refactoring to avoid retyping the lib name everywhere but since it now needs to be in ASAP for the cherry pick we will leave it as it is for this PR. Correcting the lib names and triples has removed the red herring warning. Test failure is known unrelated issue: |
codeplay-sycl
pushed a commit
to codeplaysoftware/intel-llvm-mirror
that referenced
this pull request
Nov 14, 2022
…iple. (intel#7245) People have been getting confused, see e.g. intel#6922, due to warnings of mismatching triples in the HIP AMD and CUDA backends during linking. This warning occurs because the bitcode compiled from spirv libclc does not use the same triples as those passed to clang to compile SYCL in the CUDA, "nvptx64-nvidia-cuda", and HIP "amdgcn-amd-amdhsa" backends. However these modules are created only to be linked with clang compiled SYCL code and the modules are always compatible. Therefore in this patch we update the triples of the final remangled spirv modules and update the names of the final remangled .bcs that are built/installed/linked with the clang built .bcs. Fixes intel#6922 Signed-off-by: JackAKirk <jack.kirk@codeplay.com> Co-authored-by: JackAKirk <chezjakirk@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
People have been getting confused, see e.g. #6922, due to warnings of mismatching triples in the HIP AMD and CUDA backends during linking. This warning occurs because the bitcode compiled from spirv libclc does not use the same triples as those passed to clang to compile SYCL in the CUDA, "nvptx64-nvidia-cuda", and HIP "amdgcn-amd-amdhsa" backends.
However these modules are created only to be linked with clang compiled SYCL code and the modules are always compatible.
Therefore in this patch we update the triples of the final remangled spirv modules and update the names of the final remangled .bcs that are built/installed/linked with the clang built .bcs.
Fixes #6922