[Matrix] Add single subscript test #503

farzonl · 2025-11-13T05:41:07Z

fixes #502
https://godbolt.org/z/xaEE44cbE
Test 3 cases (loads and stores):

single subscript as an l-value swizzle
single subscript where the store/load takes a vector r-value swizzle
single subscript where we store/load the vector as is.

https://godbolt.org/z/xaEE44cbE Test 3 cases (loads and stores): 1. single subscript as an l-value swizzle 2. single subscript where the store/load takes a vector r-value swizzle 3. single subscript where we store/load the vector as is.

alsepkow · 2025-11-14T17:21:41Z

test/Basic/matrix_single_subscript_load.test

+                      In[12], In[13], In[14],
+                      In[15]);
+
+    for (int i = 0; i < 4; i++) {


nit: Why don't we follow coding standards in test? I've noticed this in lots of tests.

Even in llvm-project we don't run clang-format on tests, but the capital I is a c++ coding standard for c/c++ in llvm-project. I wasn't aware we defined a coding standard for HLSL but I suppose we could adopt the c/c++ one?

Yeah. I was referring to the c/c++ coding standard. I didn't consider that it was c/c++ specific. It's just stood out to me for the variable names in tests we add. So, my nit really was more of a question. Just stands out to me.

There is a slight nuance here too, which is that Clang tests often deliberately do not conform to LLVM's coding standards. It would be pretty awful if Clang only worked on C/C++ code that conformed to LLVM's standards (one could imagine a parser bug that didn't properly handle tabs because LLVM disallows them, or CRLF line endings because LLVM biases toward LF).

I don't have a strong opinion on these specific cases. I do find code easier to read if it conforms to coding standards, but there are sometimes benefits to not conform.

alsepkow · 2025-11-14T17:30:48Z

test/Basic/matrix_single_subscript_load.test

+                      In[15]);
+
+    for (int i = 0; i < 4; i++) {
+        int4 B;


very minor nit, but a for loop for 4 assignments with two of them be 'special' made me look at this for a moment longer.
4 explicit assignment statements might be cleaner.

alsepkow

LGTM. Minor nit on the first for loop in both tests. Take it or leave it.

Icohedron

LGTM! Just a few formatting/spacing nits

Icohedron · 2025-11-14T19:48:35Z

test/Basic/matrix_single_subscript_load.test

+    int4x4 A = int4x4(In[0],  In[1],  In[2],
+                      In[3],  In[4],  In[5],
+                      In[6],  In[7],  In[8],
+                      In[9],  In[10], In[11],
+                      In[12], In[13], In[14],
+                      In[15]);


Formatting nit:

Suggested change

int4x4 A = int4x4(In[0], In[1], In[2],

In[3], In[4], In[5],

In[6], In[7], In[8],

In[9], In[10], In[11],

In[12], In[13], In[14],

In[15]);

int4x4 A = int4x4(In[0], In[1], In[2], In[3],

In[4], In[5], In[6], In[7],

In[8], In[9], In[10], In[11],

In[12], In[13], In[14], In[15]);

Icohedron · 2025-11-14T19:53:11Z

test/Basic/matrix_single_subscript_store.test

+    int4 vec = int4(In[0], In[1], In[2], 
+                      In[3]);


Formatting nit

Suggested change

int4 vec = int4(In[0], In[1], In[2],

In[3]);

int4 vec = int4(In[0], In[1], In[2], In[3]);

Icohedron · 2025-11-14T19:57:21Z

test/Basic/matrix_single_subscript_store.test

+Buffers:
+  - Name: In
+    Format: Int32
+    Data: [ 1, 2, 3, 4]


nit: There is an extra space here for some reason

Suggested change

Data: [ 1, 2, 3, 4]

Data: [1, 2, 3, 4]

tex3d

Minor comments/suggestions, nothing necessarily requiring changes regarding:

dynamic/static subscript indexing and potential optimizations
symmetrical swizzles (equivalent when used in source or dest)

tex3d · 2025-11-14T21:22:29Z

test/Basic/matrix_single_subscript_load.test

+    for (int i = 0; i < 4; i++) {
+        int4 B;
+        if (i % 2 == 0)
+            B = A[i];


Just a comment, not a request for change:

These subscript accesses look dynamic, but these loops are likely to be unrolled, either before IR (in DXC, Clang, etc.) or after (in a driver compiler). That unrolling would make all accesses static, even eliminating the intermediate local variables to copy values directly from static indices of In to static indices of Out. That's a fine path to test, but this might not test other important dynamic indexing code paths, if desired.

I guess this gets at the fuzzy purpose issue with the offload-test-suite. This seems fine to test this scenario end-to-end, whatever path it may take through optimizations, and maybe that's all that's desired for this test.

tex3d · 2025-11-14T22:04:29Z

test/Basic/matrix_single_subscript_load.test

+        else
+            B.grab = A[i];


Assignment swizzling is always harder to follow, but this one happens to be identical in mapping to the same swizzle if used on source instead. That seems a little too convenient, so maybe the target swizzle should be different to avoid that symmetry and make sure it's not treated the same somehow.

grab is symmetric (equivalent to grab on source):
B.grab = {5,6,7,8} -> {6,5,8,7}
B = {5,6,7,8}.grab -> {6,5,8,7}

agrb is not symmetric (equivalent to bgar on source):
B.agrb = {5,6,7,8} -> {7,6,8,5}
B = {5,6,7,8}.agrb -> {8,6,5,7}
B = {5,6,7,8}.bgar -> {7,6,8,5}

Perhaps a comment with the equivalent in source swizzle form would help too:

Suggested change

else

B.grab = A[i];

else // i == 1

B.agrb = A[i]; // equivalent: B = A[1].bgar

Corresonding update to expected values would be:

- Data: [ 1, 2, 3, 4, 6, 5, 8, 7, 9, 10, 11, 12, 16, 15, 14, 13 ] + Data: [ 1, 2, 3, 4, 7, 6, 8, 5, 9, 10, 11, 12, 16, 15, 14, 13 ]

tex3d · 2025-11-14T22:04:48Z

test/Basic/matrix_single_subscript_load.test

+        if (i % 2 == 0)
+            B = A[i];
+        else if (i % 3 == 0)
+            B = A[i].abgr;


Note: abgr is also a symmetric swizzle. bagr is another asymmetric swizzle you could use.

tex3d · 2025-11-14T22:05:10Z

test/Basic/matrix_single_subscript_store.test

+            A[i] = vec;
+        // A[1] is reverse In Buffer
+        else
+            A[i] = vec.abgr;


Note: another symmetric swizzle.

[Matrix] Add single subscript test

ca001e1

https://godbolt.org/z/xaEE44cbE Test 3 cases (loads and stores): 1. single subscript as an l-value swizzle 2. single subscript where the store/load takes a vector r-value swizzle 3. single subscript where we store/load the vector as is.

alsepkow reviewed Nov 14, 2025

View reviewed changes

Icohedron approved these changes Nov 14, 2025

View reviewed changes

tex3d approved these changes Nov 14, 2025

View reviewed changes

	int4 vec = int4(In[0], In[1], In[2],
	In[3]);
	int4 vec = int4(In[0], In[1], In[2], In[3]);

-        else
-            B.grab = A[i];
+        else // i == 1
+            B.agrb = A[i]; // equivalent: B = A[1].bgar

[Matrix] Add single subscript test #503

Are you sure you want to change the base?

[Matrix] Add single subscript test #503

Uh oh!

Conversation

farzonl commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alsepkow left a comment

Choose a reason for hiding this comment

Uh oh!

Icohedron left a comment

Choose a reason for hiding this comment

Uh oh!

Icohedron Nov 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tex3d left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

farzonl commented Nov 13, 2025 •

edited

Loading

Icohedron Nov 14, 2025 •

edited

Loading