Skip to content

[SM6.10][NFC] Combine LinAlg ops stage tests with errors and move under LitDXILValidation#8232

Closed
hekota wants to merge 2 commits into
microsoft:mainfrom
hekota:merge-error-stage-tests
Closed

[SM6.10][NFC] Combine LinAlg ops stage tests with errors and move under LitDXILValidation#8232
hekota wants to merge 2 commits into
microsoft:mainfrom
hekota:merge-error-stage-tests

Conversation

@hekota

@hekota hekota commented Mar 6, 2026

Copy link
Copy Markdown
Member

All of the test files are very similar and can be combined into a single file with multiple //RUN: lines.
These are validation tests, not Sema, so the new file is created in LitDXILValidation directory.

Part of #8211

@alsepkow alsepkow left a comment

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.

lgtm

@hekota hekota changed the title [SM6.10][NFC] Combine LinAlg ops stage tests that expect errors into one file [SM6.10][NFC] Combine LinAlg ops stage tests with errors and move under LitDXILValidation Mar 6, 2026

@V-FEXrt V-FEXrt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not for now, but there is talk about adding -D from upstream into FileCheck. If that actually happens then it would be good to come back later and make the CHECK lines more strict

@tex3d tex3d left a comment

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.

Normally, our validation tests should be IR tests instead of from HLSL source. Especially since we need to add checks in Sema to catch these earlier, with better diagnostics.

Unfortunately, this makes it more difficult to use a bunch of RUN lines to test each scenario. It might be possible to run a preprocess step on the IR with defines selecting the core DXIL operation in the IR still though.

{
__builtin_LinAlgMatrix [[__LinAlgMatrix_Attributes(1, 5, 4, 2, 2)]] mat1;
__builtin_LinAlgMatrix [[__LinAlgMatrix_Attributes(1, 5, 4, 2, 2)]] mat2;
__builtin_LinAlgMatrix [[__LinAlgMatrix_Attributes(1, 5, 4, 2, 2)]] mat3;

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 imagine we're going to also want validation for legal and consistent matrix attributes for each DXIL op. These attributes selected here definitely create invalid calls below, regardless of shader stage. We could probably create A, B, and Accumulator matrix args, which have consistent dimensions/component types so that these calls are only illegal due to their shader stage.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll try to update these to use valid types when I convert to IR tests. I believe we are going to handle parameter validation after we ship the preview, so if I miss any we can fix it up then.

@hekota

hekota commented Mar 6, 2026

Copy link
Copy Markdown
Member Author

Normally, our validation tests should be IR tests instead of from HLSL source. Especially since we need to add checks in Sema to catch these earlier, with better diagnostics.

Unfortunately, this makes it more difficult to use a bunch of RUN lines to test each scenario. It might be possible to run a preprocess step on the IR with defines selecting the core DXIL operation in the IR still though.

Got it. I did not realize these should have been IR tests from the start.

@hekota hekota closed this Mar 7, 2026
@github-project-automation github-project-automation Bot moved this from New to Done in HLSL Roadmap Mar 7, 2026
@hekota

hekota commented Mar 7, 2026

Copy link
Copy Markdown
Member Author

These should be IR validation tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants