Skip to content

[SYCL] Fix operators for bool swizzled vec #12001

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 7 commits into from
Nov 29, 2023

Conversation

0x12CC
Copy link
Contributor

@0x12CC 0x12CC commented Nov 23, 2023

c3a9615 added CommonDataT to fix an issue where results were truncated if an operand had a larger type than the result. As CommonDataT uses std::common_type_t of the left and right operands, it ignores DataT. This causes an issue for cases where CommonDataT = bool and DataT != bool since the operation result will be implicitly converted from DataT to bool.

This change adds DataT to set of types used to define CommonDataT. Cases where the operands were bool type will now have the correct result rather than 1. Fixes #11995.

c3a9615 added `CommonDataT` to fix an
issue where results were truncated if an operand had a larger type than
the result. As `CommonDataT` uses `std::common_type_t` of the left and
right operands, it ignores `DataT`. This causes an issue for cases where
`CommonDataT = bool`  and `DataT != bool` since the operation result
will be implicitly converted from `DataT` to `bool`.

This change adds `DataT` to set of types used to define `CommonDataT`.
Cases where the operands were `bool` type will now have the correct
result rather than `1`. Fixes intel#11995.

Signed-off-by: Michael Aziz <[email protected]>
Signed-off-by: Michael Aziz <[email protected]>
@0x12CC
Copy link
Contributor Author

0x12CC commented Nov 29, 2023

The failing test is the following:

Timed Out Tests (1):
  SYCL :: Basic/vector/int-convert.cpp

It's unrelated to this PR and was reported in #12011. @intel/llvm-gatekeepers, can this be merged?

@steffenlarsen steffenlarsen merged commit 462d6d4 into intel:sycl Nov 29, 2023
@0x12CC 0x12CC deleted the vec_bool_swizzle branch November 29, 2023 17:04
@sarnex
Copy link
Contributor

sarnex commented Nov 29, 2023

@0x12CC It looks like vec_rel_swizzle_ops.cpp is failing, I see it failing in my PR here, I don't see it failing anywhere else but I can't imagine I broke this

@0x12CC
Copy link
Contributor Author

0x12CC commented Nov 29, 2023

I'm not sure why it's failing but I'll try to reproduce the error.

@sarnex
Copy link
Contributor

sarnex commented Nov 29, 2023

Cool thx

@steffenlarsen
Copy link
Contributor

Failure should be fixed with #12040.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

operator&& result differs for certain swizzle operands
4 participants