-
Notifications
You must be signed in to change notification settings - Fork 797
[Matrix][SYCL] Fix the bug of comparison between two bf16's wi_element #5493
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@intel/llvm-reviewers-runtime ping |
@@ -477,8 +477,8 @@ class wi_element<uint16_t, NumRows, NumCols, Layout, Group> { | |||
|
|||
explicit operator bool() { | |||
#ifdef __SYCL_DEVICE_ONLY__ | |||
return __spirv_VectorExtractDynamic(M.spvm, idx) != | |||
static_cast<uint16_t>(0); | |||
return std::fabs(make_fp32(__spirv_VectorExtractDynamic(M.spvm, idx))) < |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it seems element_wise_all_ops_bf16_yubing_20220128.cpp failed on this line.
when I change to
make_fp32(__spirv_VectorExtractDynamic(M.spvm, idx)) != 0.0f;
the testcase can pass
@AlexeySotkin , what should we do for operator bool()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we use ">=" instead of "<"
ping? @dkhaldi |
@intel/llvm-reviewers-runtime ping |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@vladimirlaz , hi, could you help us merge it ? |
Previously, as for bf16's comparison, we directly compare two unsigned short and it is wrong.
Now, we extend b16 to float and do the correct comparsion.