Repository navigation
Convert matrix out/inout arguments with a matrix cast - #8988
Open
Joel Kiptoo (Kiptoo-Deus) wants to merge 2 commits into
Open
Joel Kiptoo (Kiptoo-Deus) wants to merge 2 commits into
Joel Kiptoo (Kiptoo-Deus) wants to merge 2 commits into
Conversation
When the argument of an out or inout matrix parameter has a different element type (e.g. a half3x3 argument for an out float3x3 parameter), the copy back after the call went through ConvertScalarOrVector, which asserts on matrices, and the copy in for inout bitcast the matrix value, which is not a valid cast either. Use the same HLCast matrix conversion as an explicit matrix cast in both directions. Fixes microsoft#8809
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Copilot started reviewing on behalf of
Joel Kiptoo (Kiptoo-Deus)
September 30, 2026 02:32
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The signedness-sensitive conversion paths need an observable integer-to-floating-point regression test.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Fixes DXC crashes when converting matrix out/inout arguments between element types.
Changes:
- Adds explicit HLCast-based matrix conversions for copy-in and copy-back.
- Adds regression coverage for half, float, int, and uint matrices.
- Documents the fix in release notes.
| File | Description |
|---|---|
tools/clang/lib/CodeGen/CGHLSLMS.cpp |
Implements matrix argument conversions. |
tools/clang/test/HLSLFileCheck/hlsl/functions/arguments/matrix_out_param_conversion.hlsl |
Adds regression tests. |
docs/ReleaseNotes.md |
Records the crash fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Member
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
An int to uint matrix copy doesn't show which cast opcode was used, so also cover an inout uint2x2 argument for a float2x2 parameter, which needs uitofp on the way in and fptoui on the way out.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #8809.
When the argument of an
outorinoutmatrix parameter has a different element type (for example ahalf3x3argument for anout float3x3parameter with-enable-16bit-types), DXC crashed while generating the copies around the call:EmitHLSLOutParamConversionCopyBack) passed the matrix toConvertScalarOrVector, which asserts:EmitNumericConversion can only be used with int/float scalars/vectors.inout, the copy in (EmitHLSLOutParamConversionInit) used aBitCastbetween the two matrix types, which asserts withInvalid cast!.Both directions now emit the same
HLCastmatrix conversion as an explicit matrix cast (DefaultCast, or the unsigned variants depending on the element types), through a smallEmitHLSLMatrixConversionhelper. When the types already match, nothing changes.Testing (macOS arm64, Release build with assertions):
HLSLFileCheck/hlsl/functions/arguments/matrix_out_param_conversion.hlsl, coveringout float2x2tohalf2x2(fptruncon the copy back),inout float2x2with ahalf2x2argument (fpextin,fptruncout),out int2x2touint2x2, andinout float2x2with auint2x2argument (uitofpin,fptouiout). It uses-Odso the conversions stay visible, and theuint2x2/float2x2case checksuitofp/fptoui, so it fails if the signedness of the conversion is ignored.inoutcase and the int to uint case all crash (exit 133). With it they compile, and with constant inputs the results fold to the expected values (for example theinoutdoubling ofhalf2x2(1, 2, 3, 4)stores2, 4, 6, 8).