Skip to content

Conversation

@aelovikov-intel
Copy link
Contributor

@aelovikov-intel aelovikov-intel commented Feb 26, 2025

Seems to have been removed accidentally in #130.
KhronosGroup/SYCL-Docs#675 might be considered somewhat related.

@aelovikov-intel aelovikov-intel requested a review from a team as a code owner February 26, 2025 19:49
aelovikov-intel added a commit to aelovikov-intel/SYCL-CTS that referenced this pull request Feb 26, 2025
Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this something not implemented in SimSYCL?

@aelovikov-intel
Copy link
Contributor Author

Is this something not implemented in SimSYCL?

Based on the error, it's implemented incorrectly:

/__w/SYCL-CTS/SYCL-CTS/build/tests/vector_operators/vector_operators_bool.cpp:569:26: error: ambiguous overload for ‘operator=’ (operand types are ‘simsycl::sycl::vec<signed char, 1>’ and ‘simsycl::sycl::vec<bool, 1>’)
  569 |     resVec = testVec1 && testVec2;
      |                          ^~~~~~~~

Result of relational/logical operations should have fixed-width integer type of the same size as operand's element type, not bool. That's true in present version of the spec, but is clarified further in the proposed KhronosGroup/SYCL-Docs#675.

Copy link
Member

@keryell keryell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
Is the SimSYCL team watching this?

Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, disable failing tests for SimSYCL implementation. PR must not regress CI checks.

https://github.com/KhronosGroup/SYCL-CTS/tree/main/docs#compile-time-disabled-test-cases

@aelovikov-intel aelovikov-intel requested a review from bader March 19, 2025 18:26
Copy link
Contributor

@bader bader left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@bader bader merged commit 1d48dfe into KhronosGroup:main Mar 22, 2025
9 checks passed
@aelovikov-intel aelovikov-intel deleted the restore-rel-logical branch March 26, 2025 16:55
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.

3 participants