Skip to content

Conversation

@no-materials
Copy link
Contributor

This PR implements the previously TODO-marked SIMD boolean operations for WASM targets.

Changes Made

  • The reinterpretation logic for these operations was already implemented in the codebase. This PR removes the special conditions that were preventing these operations from being generated through the normal code flow.

  • Implemented tests for each boolean operation, currently covering the i8x16 vector type to verify correctness of the implementation.

Operations Implemented

The following SIMD boolean operations are now available for WASM:

  • and, or, xor, not

Testing

Added test coverage for each operation using the i8x16 vector type.

Should I extend the test suite to cover additional vector types, or is the current i8x16 coverage sufficient?

@LaurenzV
Copy link
Collaborator

LaurenzV commented Jun 26, 2025

Thanks! Will let @ajakubowicz-canva do the review because he is more familiar with WASM, but it looks good.

Should I extend the test suite to cover additional vector types, or is the current i8x16 coverage sufficient?

I think that's up to you, in the future we will obviously want to have tests for "everything", but we don't really have a proper test suite in place. This will happen eventually, but right now our focus is just to add all operations we need to vello.

@ajakubowicz-canva ajakubowicz-canva self-requested a review June 26, 2025 12:12
Copy link
Collaborator

@ajakubowicz-canva ajakubowicz-canva left a comment

Choose a reason for hiding this comment

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

Thank you so much. With the inclusion of those tests I'm very happy with this. I echo @LaurenzV that adding tests across all operators would be ideal, but it can be optional.

Thank you!

Edit: I realized that I am not aware if you're able to merge. Let me know if you need myself or Laurenz to merge this PR.

@no-materials
Copy link
Contributor Author

no-materials commented Jun 26, 2025

Thank you so much. With the inclusion of those tests I'm very happy with this. I echo @LaurenzV that adding tests across all operators would be ideal, but it can be optional.

Thank you!

Edit: I realized that I am not aware if you're able to merge. Let me know if you need myself or Laurenz to merge this PR.

Thank you for the positive feedback! I've added additional tests covering unsigned and mask vector types to provide broader coverage of the boolean operations.
I decided not to extend tests to all vector sizes at this time since, as @LaurenzV mentioned, a proper comprehensive test suite will be implemented in the future.

I don't have merge permissions, so I'd appreciate if you or @LaurenzV could merge this PR when you're ready.

Thanks again for the review and guidance!

Copy link
Collaborator

@ajakubowicz-canva ajakubowicz-canva left a comment

Choose a reason for hiding this comment

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

Thank you so much! Changes look great. I'll merge once CI passes.

@ajakubowicz-canva ajakubowicz-canva merged commit 62bd72c into linebender:main Jun 26, 2025
6 checks passed
@no-materials no-materials deleted the wasm-boolean-simd-ops branch June 27, 2025 05:39
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