Skip to content

Add abs matrix tests - #1492

Merged
kmpeng merged 3 commits into
llvm:mainfrom
kmpeng:abs-matrix-tests
Sep 11, 2026
Merged

kmpeng merged 3 commits into
llvm:mainfrom
kmpeng:abs-matrix-tests

Conversation

@kmpeng

@kmpeng kmpeng commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Closes #796.

Adds matrix tests for abs testing float, int, and uint. Will remove the XFAILs once the Clang implementation is in.
Only testing 32-bit types because float/int/uint exercise all of the different abs paths on the DirectX and SPIR-V backends, and those paths behave the same at every bit size.

Assisted-by: Claude Opus 4.8

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.

One question here, do we generally need abs on uint? I assume these always get optimized away to nothing right?

@kmpeng kmpeng Sep 3, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was actually debating whether to add this test for a while. I ended up adding it because uint goes through its own path (I believe a separate lowering in DXC and a different overload in Clang), so I thought it might be worth testing that it really behaves as an identity across targets. But definitely open to removing it.

Comment thread test/Feature/HLSLLib/abs_mat.fp.test Outdated
@kmpeng
kmpeng merged commit b66e016 into llvm:main Sep 11, 2026
28 checks passed
Alexander-Johnston pushed a commit to Alexander-Johnston/offload-test-suite that referenced this pull request Sep 14, 2026
Closes llvm#796.

Adds matrix tests for `abs` testing `float`, `int`, and `uint`. Will
remove the XFAILs once the Clang implementation is in.
Only testing 32-bit types because `float`/`int`/`uint` exercise all of
the different `abs` paths on the DirectX and SPIR-V backends, and those
paths behave the same at every bit size.

Assisted-by: Claude Opus 4.8
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 matrix test for abs

4 participants