Skip to content

[Matrix] Add boolean operator tests#1054

Open
farzonl wants to merge 1 commit intollvm:mainfrom
farzonl:mat_bool_operator_tests
Open

[Matrix] Add boolean operator tests#1054
farzonl wants to merge 1 commit intollvm:mainfrom
farzonl:mat_bool_operator_tests

Conversation

fixes llvm#706

With the following PRs now merged
- llvm/llvm-project#172384
- llvm/llvm-project#175809
- llvm/llvm-project#175245
- llvm/llvm-project#175633

We have a working base to test booleans on both DXC and Clang.
@farzonl farzonl force-pushed the mat_bool_operator_tests branch from 30db7a7 to adb0358 Compare April 1, 2026 18:05
Copy link
Copy Markdown
Contributor

@bob80905 bob80905 left a comment

Choose a reason for hiding this comment

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

Looks mostly good. You might want to document your reasoning for why you chose not to add a thread test for the or operator.

Data: [ 1, 1, 1, 1, 1, 1 ]
- Name: AndTrueFalseOut
Format: Int32
FillSize: 24
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You might consider setting this data to be non-zero. A nit, but it ensures that the buffer is actually being set to 0, and that the program is actually running.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that we should ensure the buffers are not zero-initialized

Copy link
Copy Markdown
Contributor

@inbelic inbelic left a comment

Choose a reason for hiding this comment

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

LGTM once Josh's comments are addressed.

The tests do seem to overlap a lot, could we not have the same test coverage with a single 2x2 matrix that has a TT, TF, FT, FF combination?

Data: [ 1, 1, 1, 1, 1, 1 ]
- Name: AndTrueFalseOut
Format: Int32
FillSize: 24
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that we should ensure the buffers are not zero-initialized

# Bug https://github.com/llvm/offload-test-suite/issues/704
# XFAIL: Clang && Intel && Vulkan

Bug https://github.com/llvm/offload-test-suite/issues/705
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
Bug https://github.com/llvm/offload-test-suite/issues/705
# Bug https://github.com/llvm/offload-test-suite/issues/705

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.

[Matrix] Add bool tests for matrix logical operators

5 participants