Implement HLSL Diagnostics for LinAlg operations #7430

anupamachandra · 2025-05-05T12:11:52Z

Implements #7336

tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_add_invalid.hlsl

tools/clang/lib/Sema/SemaHLSL.cpp

anupamachandra · 2025-05-05T16:13:11Z

Couple of items left to do here:

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

damyanp · 2025-05-05T21:28:03Z

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

anupamachandra · 2025-05-05T21:50:03Z

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

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

Co-authored-by: Damyan Pepper <[email protected]>

tools/clang/lib/Sema/SemaHLSL.cpp

bob80905

A few nits / questions

tools/clang/lib/Headers/hlsl/dx/linalg.h

tools/clang/lib/Sema/SemaHLSL.cpp

tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_add_invalid.hlsl

tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_invalid.hlsl

tools/clang/test/SemaHLSL/hlsl/linalg/builtins/outer_product_accumulate_invalid.hlsl

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

llvm-beanz · 2025-05-12T19:15:49Z

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

+def err_hlsl_linalg_isunsigned_incorrect_for_given_type : Error<
+  "%0 must be %select{false|true}1 for a %2 vector type">;
+def err_hlsl_linalg_interpretation_value_incorrect : Error<
+  "%0 is an invalid %select{Memory|Register}1 Interpretation value">;


Suggested change

"%0 is an invalid %select{Memory|Register}1 Interpretation value">;

"%0 is an invalid %select{memory|register}1 interpretation value">;

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

llvm-beanz · 2025-05-12T19:19:23Z

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

+def err_hlsl_linalg_optimal_matrix_layout_matrix_stride_must_be_zero : Error<
+  "for optimal matrix layout, matrix stride must be zero">;
+def err_hlsl_linalg_matrix_dim_must_not_be_zero: Error<
+  "matrix dimension must not be zero">;


Suggested change

"matrix dimension must not be zero">;

"matrix dimension must be greater than 0">;

yeah, much better.

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

llvm-beanz · 2025-05-12T19:20:41Z

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

+  "matrix dimension must not be zero">;
+def err_hlsl_linalg_matrix_layout_invalid : Error<
+  "matrix layout %0 is not valid, must be in the range %1 - %2">;
+def err_hlsl_linalg_function_requires_shader_model_6_9_or_above : Error<


This should use warn_hlsl_intrinsic_in_wrong_shader_model (which is a default-error warning)

warnings can be overriden?

yes, and for all the availability warnings we allow them to be overridden because they might be valid if the compiler fully optimizes them away.

I remembered as I was modifying this per your feedback that I had originally tried to use warn_hlsl_intrinsic_in_wrong_shader_model but that error prints out the entry function which is not available here. In my earlier PR I had added check to the DiagnoseReachableHLSLCall in SemaHLSL.cpp, but that gets called after these intrinsics are processed. Is there a better place to move this diagnostic?

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td

tools/clang/lib/Sema/SemaHLSL.cpp

tex3d

A couple comments on linalg.h API changes.

tex3d · 2025-05-14T18:15:09Z

tools/clang/lib/Headers/hlsl/dx/linalg.h

-template <> bool IsUnsigned<uint16_t>() { return true; }
-#endif
+template <typename T> struct IsUnsigned {
+  static const bool Value = false;


I feel like the default template shouldn't have any definition for Value, if possible. That way, unsupported types trigger an error here instead of defaulting to signed. That would require specializations for float types and all supported signed int types, of course, but it would also prevent use of unexpected types.

Do we still want this, given the builtins are now updated to use only the allowed types?

tex3d · 2025-05-14T18:15:57Z

tools/clang/lib/Headers/hlsl/dx/linalg.h

+template <> struct IsUnsigned<uint64_t> {
+  static const bool value = true;
+};
+
+#ifdef __HLSL_ENABLE_16_BIT
+template <> struct IsUnsigned<uint16_t> {
+  static const bool value = true;
+};


These specializations use a different casing for Value.

Suggested change

template <> struct IsUnsigned<uint64_t> {

static const bool value = true;

};

#ifdef __HLSL_ENABLE_16_BIT

template <> struct IsUnsigned<uint16_t> {

static const bool value = true;

};

template <> struct IsUnsigned<uint64_t> {

static const bool Value = true;

};

#ifdef __HLSL_ENABLE_16_BIT

template <> struct IsUnsigned<uint16_t> {

static const bool Value = true;

};

Good catch - but this makes me concerned we have a test gap?

Yeah, this is the issue Joshua mentioned in his review that I mentioned at today's morning meeting was a bug. I am adding high-level API tests that would have caught this.

…pretation

…ocalization issues

…g component class and update tests

github-actions · 2025-05-15T14:12:43Z

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

anupamachandra added 13 commits May 5, 2025 05:02

Add HLSL validation to coopvec builtins

3dd5f58

Add more HLSL validation

70119f8

Add IsTransposable check for matrix layouts

737e65b

Check validation of Bias Interpretation parameters

e4eab81

Add diagnostics for OuterProoductAccumulate

392322d

Updated error message

1831811

Minor Fixes

d1fe869

Clang Format

4d751a7

Fix error location and add tests

02cb085

More validation tests

969d7e6

Check Matrix dimensions are not zero

f132c60

More tests

3c19297

Remove unnecessary space

3da3872

github-project-automation bot added this to HLSL Roadmap May 5, 2025

github-project-automation bot moved this to New in HLSL Roadmap May 5, 2025

Fix run command

16a5ab8

anupamachandra commented May 5, 2025

View reviewed changes

tools/clang/test/SemaHLSL/hlsl/linalg/builtins/mul_add_invalid.hlsl Outdated Show resolved Hide resolved

anupamachandra commented May 5, 2025

View reviewed changes

tools/clang/lib/Sema/SemaHLSL.cpp Outdated Show resolved Hide resolved

anupamachandra added 2 commits May 5, 2025 08:52

Updated Comments

2132e01

Update invalid checks for input interpretation

182baae

anupamachandra requested review from tex3d and damyanp May 5, 2025 16:38

tex3d self-assigned this May 5, 2025

tex3d added this to HLSL Support May 5, 2025

tex3d moved this to Active in HLSL Support May 5, 2025

anupamachandra commented May 5, 2025

View reviewed changes

tools/clang/include/clang/Basic/DiagnosticSemaKinds.td Show resolved Hide resolved

Fix Build errors

974f2d4

anupamachandra and others added 6 commits May 7, 2025 15:11

Ugh..line number change from updating the linalg headers

be4c02f

Update tools/clang/lib/Headers/hlsl/dx/linalg.h

c4233f0

Co-authored-by: Damyan Pepper <[email protected]>

Update variable name missed

4295c84

Use ComponentType and MatrixLayout from DxilConstants.h

17b810e

Fix unsafe inmplicit cast

3cce131

Add tests to mul_add_invalid for shared parameters

c9c1389

damyanp approved these changes May 9, 2025

View reviewed changes

tools/clang/lib/Sema/SemaHLSL.cpp Outdated Show resolved Hide resolved

anupamachandra and others added 2 commits May 9, 2025 09:52

Add validation and tests for packed input types

e870b99

chore: autopublish 2025-05-09T16:56:52Z

46c2caf

damyanp approved these changes May 9, 2025

View reviewed changes

anupamachandra added 3 commits May 9, 2025 10:18

review feedback

99a9566

alignment

2ab5c86

Add Max Matrix Dim checks

90fac97

damyanp assigned bob80905 and unassigned tex3d May 12, 2025

bob80905 reviewed May 12, 2025

View reviewed changes

llvm-beanz reviewed May 12, 2025

View reviewed changes

tex3d reviewed May 14, 2025

View reviewed changes

anupamachandra added 6 commits May 14, 2025 17:19

Update diagnostics per review feedback

62c31bc

Per review feedback, change upper to lower case register/memory inter…

a8676ea

…pretation

Review feedback: Prefer 0 over zero in diagnostic messages to avoid l…

9045c82

…ocalization issues

Updates based on review feedback: fix range specification

4fc747e

Change input/output vector allowed types from numeric to custom linal…

5650cf1

…g component class and update tests

Update per feeback: refactor Matrix dimension check diagnostics

b845582

anupamachandra added 2 commits May 15, 2025 07:28

Fix darker error

baa4bbc

Merge from main

40a2311

anupamachandra mentioned this pull request May 15, 2025

[0031] Update LinAlg HLSL diagnostics to provide all errors in a call, than one at a time #7456

Open

anupamachandra added 2 commits May 15, 2025 10:08

Fix typo

5751f2b

Remove unused variable

c60bcba

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement HLSL Diagnostics for LinAlg operations #7430

Implement HLSL Diagnostics for LinAlg operations #7430

anupamachandra commented May 5, 2025

anupamachandra commented May 5, 2025

damyanp commented May 5, 2025

anupamachandra commented May 5, 2025

bob80905 left a comment

llvm-beanz May 12, 2025

llvm-beanz May 12, 2025

anupamachandra May 12, 2025

llvm-beanz May 12, 2025

anupamachandra May 12, 2025

llvm-beanz May 13, 2025

anupamachandra May 15, 2025

tex3d left a comment

tex3d May 14, 2025 •

edited

Loading

anupamachandra May 15, 2025

tex3d May 14, 2025

damyanp May 14, 2025

anupamachandra May 14, 2025

github-actions bot commented May 15, 2025 •

edited

Loading

	"%0 is an invalid %select{Memory\|Register}1 Interpretation value">;
	"%0 is an invalid %select{memory\|register}1 interpretation value">;

	"matrix dimension must not be zero">;
	"matrix dimension must be greater than 0">;

Implement HLSL Diagnostics for LinAlg operations #7430

Are you sure you want to change the base?

Implement HLSL Diagnostics for LinAlg operations #7430

Conversation

anupamachandra commented May 5, 2025

anupamachandra commented May 5, 2025

damyanp commented May 5, 2025

anupamachandra commented May 5, 2025

bob80905 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tex3d left a comment

Choose a reason for hiding this comment

tex3d May 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 15, 2025 • edited Loading

tex3d May 14, 2025 •

edited

Loading

github-actions bot commented May 15, 2025 •

edited

Loading