Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

[SYCL] Fix CUDA tests using bfloat16 #1421

Merged
merged 2 commits into from
Nov 30, 2022

Conversation

steffenlarsen
Copy link

@steffenlarsen steffenlarsen commented Nov 29, 2022

SYCL/BFloat16/bfloat16_builtins.cpp, SYCL/Matrix/element_wise_all_ops_bf16.cpp, SYCL/Matrix/element_wise_wi_marray_legacy.cpp are made to use the correct extension namespaces to have both bfloat16 and the corresponding experimental builtins without namespaces.

SYCL/Matrix/joint_matrix_tensorcores_legacy.cpp is changed to case convert bfloat to float directly - using the existing else-branch - instead of using the removed bfloat16::raw() method and custom conversion.

These test changes require intel/llvm#7567.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen
Copy link
Author

@JackAKirk - Would you be able to help verify that these fix the problems seen in pre-commit CI? For example in intel/llvm#7563.

@JackAKirk
Copy link

@JackAKirk - Would you be able to help verify that these fix the problems seen in pre-commit CI? For example in intel/llvm#7563.

Yep sure, no problem.

@JackAKirk
Copy link

@JackAKirk - Would you be able to help verify that these fix the problems seen in pre-commit CI? For example in intel/llvm#7563.

Yes with your two PRs the joint_matrix_tensorcores_legacy.cpp and bfloat16_builtins.cpp are passing. Note that cuda doesn't run on element_wise_all_ops_bf16.cpp. Changes in the tests here look good.

bfloat16_type.cpp is failing locall for me too, as well as Matrix/element_wise_wi_marray_legacy.cpp: Matrix/element_wise_wi_marray_legacy.cpp can probably be removed anyway since I think we are going to scrap this functionality, at least for the time being anyway, because it is not very important and we would have to create a whole new joint_matrix class in an experimental::cuda name to be consistent with scope conventions.

@steffenlarsen
Copy link
Author

Thanks a ton for checking! 😄

bfloat16_type.cpp is failing locall for me too, as well as Matrix/element_wise_wi_marray_legacy.cpp: Matrix/element_wise_wi_marray_legacy.cpp can probably be removed anyway since I think we are going to scrap this functionality, at least for the time being anyway, because it is not very important and we would have to create a whole new joint_matrix class in an experimental::cuda name to be consistent with scope conventions.

bfloat16_type.cpp seems to have been split up into a CUDA variant at some point too as the CUDA backend needs SM80 on the corresponding device, so I've disabled CUDA in that test again and changed the CUDA variant of it slightly to check for SM version. See #1423.

For SYCL/Matrix/element_wise_wi_marray_legacy.cpp I suspect there's just a missing using namespace sycl::ext::oneapi;. I will add that to this PR, then we can remove it later if need be.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
@steffenlarsen steffenlarsen marked this pull request as ready for review November 29, 2022 10:38
@steffenlarsen steffenlarsen requested review from a team as code owners November 29, 2022 10:38
@pvchupin
Copy link

/verify with intel/llvm#7567

@pvchupin pvchupin merged commit 4c2dc33 into intel:intel Nov 30, 2022
myler pushed a commit to myler/llvm-test-suite that referenced this pull request Mar 22, 2023
* [SYCL] Fix CUDA tests using bfloat16
* Add missing using in element_wise_wi_marray_legacy

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
* [SYCL] Fix CUDA tests using bfloat16
* Add missing using in element_wise_wi_marray_legacy

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants