Skip to content

Implement HLSL Diagnostics for LinAlg operations#7430

Merged
tex3d merged 51 commits into
microsoft:mainfrom
anupamachandra:anupamac/coopvec-hlsl-validation-main
May 16, 2025
Merged

Implement HLSL Diagnostics for LinAlg operations#7430
tex3d merged 51 commits into
microsoft:mainfrom
anupamachandra:anupamac/coopvec-hlsl-validation-main

Conversation

@anupamachandra

@anupamachandra anupamachandra commented May 5, 2025

Copy link
Copy Markdown
Collaborator

This PR implements checks to validate the linalg builtin functions: __builtin_MatVecMul, __builtin_MatVecMulAdd, __builtin_OuterProductAccumulate and __builtin_VectorAccumulate. This includes:

  • verify valid types for input and output vectors
  • const checks for compile-time const parameters
  • value checks for interpretation and layout (enum) parameters
  • min/max checks for matrix dimensions
  • verify input-output vector are the right dimensions for the given matrix (dimM and dimK) : packed and unpacked cases
  • verify matrix layout, transpose and stride rules
  • incorrect shader model warning

Adds tests for all the above error checks.

Implements #7336

Comment thread tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_add_invalid.hlsl Outdated
Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Outdated
@anupamachandra

Copy link
Copy Markdown
Collaborator Author

Couple of items left to do here:

  1. Update the DXIL tests with IR, since the HLSL diagnostics prevent the invalid IR generation we were relying on.
  2. Packed Format input vector validations. Will discuss this offline about the integer to float cast issue I am running into.

@anupamachandra anupamachandra requested review from damyanp and tex3d May 5, 2025 16:38
@tex3d tex3d self-assigned this May 5, 2025
@tex3d tex3d moved this to Active in HLSL Support May 5, 2025
@damyanp

damyanp commented May 5, 2025

Copy link
Copy Markdown
Member

@anupamachandra - given that there's build errors and you have some more work planned, is it worth reviewing at this point?

@anupamachandra

Copy link
Copy Markdown
Collaborator Author

@anupamachandra - given that there's build errors and you have some more work planned, is it worth reviewing at this point?

Yes please. I'm working on the build errors and missed item is marked with a //FIXME. It think it'll be good to get some eyes on it to check if the PR meets expectations in terms of scope and if any big issues/gaps stand out.

Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
@damyanp damyanp assigned bob80905 and unassigned tex3d May 12, 2025

@bob80905 bob80905 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.

A few nits / questions

Comment thread tools/clang/lib/Headers/hlsl/dx/linalg.h Outdated
Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Outdated
Comment thread tools/clang/lib/Sema/SemaHLSL.cpp Outdated
Comment thread tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_add_invalid.hlsl
Comment thread tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_add_invalid.hlsl Outdated
Comment thread tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_invalid.hlsl
Comment thread tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_invalid.hlsl
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated
Comment thread tools/clang/lib/Sema/SemaHLSL.cpp

@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.

A couple comments on linalg.h API changes.

Comment thread tools/clang/lib/Headers/hlsl/dx/linalg.h Outdated
Comment thread tools/clang/lib/Headers/hlsl/dx/linalg.h Outdated
@github-actions

github-actions Bot commented May 15, 2025

Copy link
Copy Markdown
Contributor

✅ With the latest revision this PR passed the Python code formatter.

@tex3d tex3d merged commit adffd31 into microsoft:main May 16, 2025
13 checks passed
@github-project-automation github-project-automation Bot moved this from Active to Closed in HLSL Support May 16, 2025
@github-project-automation github-project-automation Bot moved this from New to Done in HLSL Roadmap May 16, 2025
@damyanp damyanp removed this from HLSL Support Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants