Skip to content

Comments

Cbuffer tests for MatrixSingleSubscriptExpr#753

Open
farzonl wants to merge 3 commits intollvm:mainfrom
farzonl:cbuffer_tests
Open

Cbuffer tests for MatrixSingleSubscriptExpr#753
farzonl wants to merge 3 commits intollvm:mainfrom
farzonl:cbuffer_tests

Conversation

@farzonl
Copy link
Member

@farzonl farzonl commented Feb 18, 2026

fixes #747

This change tests Matrix padding is correct and applies to SingleSubscripting.

farzonl and others added 3 commits February 18, 2026 01:05
- Add Vulkan Clang Xfails (Vulkan doesn't know how to legalize arrays of
  vectors)
@farzonl farzonl added the test-all When applied to a PR this will opt-in to additional pre-merge test configurations.. label Feb 18, 2026
@farzonl farzonl changed the title Cbuffer tests Cbuffer tests for MatrixSingleSubscriptExpr Feb 18, 2026
@farzonl
Copy link
Member Author

farzonl commented Feb 18, 2026

This test does not include Swizzle of MatrixSingleSubscriptExpr that is coming in a later pr.

RWStructuredBuffer<float4> Out : register(u0);

cbuffer example : register(b1) {
float2x4 M1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this meant to be float1x4?

Copy link
Member Author

@farzonl farzonl Feb 18, 2026

Choose a reason for hiding this comment

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

Yep. Copy paste error and looks like an unstaged change.

Buffers:
- Name: cbuffer
Format: Float32
Data: [ 1.1, 0xFFFF, 0xFFFF, 0xFFFF,
Copy link
Collaborator

Choose a reason for hiding this comment

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

0xFFFF isn't a great sigil value for float IMO (It's like 9.18e-41?). I guess you were going for an all 1s NaN and missed a few digits. That's fine, alternatively a weird bit pattern like 0x5a5a5a5a is just a number but it's usually fairly easy to spot.

Comment on lines +55 to +56
# Note: Metal and Vulkan KosmicKrisp and MoltenVK
# Are always empty
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume you'll put bug numbers along with all of these unsupported/xfail before you merge this.

# UNSUPPORTED: Darwin

# NOTE: AMD on Clang and DXC have result:
# [ 1.1, 65535, 65535, 65535]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting - row vs column major confusion?

Comment on lines +14 to +17
Out0[0] = M1[0];
Out1[0] = M1[1];
Out2[0] = M1[2];
Out3[0] = M1[3];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not that I'm opposed to this test, but does this catch something that the simple ones don't? Nothing special about this test seems to be related to matrix at all, really.

[numthreads(1,1,1)]
void main() {
Out[0] = M1[0];
Out[1] = M1[1];
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Out[1] = M1[1];

This read and write goes out of bounds.

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not currently but will when this becomes a 1x4.

Copy link
Contributor

@Icohedron Icohedron Feb 18, 2026

Choose a reason for hiding this comment

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

It does currently read out of bounds because the input data ends with 4.4] when it should end with 4.4, 0xFFFF] for a 2x4 matrix. It writes out of bounds of course because the output buffer is only 16 bytes.

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

Labels

test-all When applied to a PR this will opt-in to additional pre-merge test configurations..

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[HLSL][Cbuffer][Matrix] Test EmitMatrixSingleSubscriptExpr padding

3 participants