[HLSL] Add matrix tests for degrees(), radians() - #1475
Conversation
kmpeng
left a comment
kmpeng
left a comment
There was a problem hiding this comment.
Mostly have the same comments as I've had in other PRs.
|
|
||
| [numthreads(1,1,1)] | ||
| void main() { | ||
| float3x2 r32 = degrees(float3x2(In[0].xy, In[0].zw, In[1].xy)); |
There was a problem hiding this comment.
(For all files)
Same as the other PRs. Capitalize variable names
| float3x2 r32 = degrees(float3x2(In[0].xy, In[0].zw, In[1].xy)); | ||
| float2x4 r24 = degrees(float2x4(In[1], In[2])); | ||
| float3x1 r31 = degrees(float3x1(In[0].x, In[2].x, In[2].y)); | ||
| float4x4 r44 = degrees(float4x4(In[0], In[1], In[2], In[3])); |
There was a problem hiding this comment.
(For all files)
Same as the other PRs. The indexing here makes it kind of hard to follow/verify values. Can we just add more values and index one row per element like other matrix tests do?
| float3x2 r32 = degrees(float3x2(In[0].xy, In[0].zw, In[1].xy)); | ||
| float2x4 r24 = degrees(float2x4(In[1], In[2])); | ||
| float3x1 r31 = degrees(float3x1(In[0].x, In[2].x, In[2].y)); | ||
| float4x4 r44 = degrees(float4x4(In[0], In[1], In[2], In[3])); |
There was a problem hiding this comment.
(For all files)
Same as the other PRs, there should be a constant folding case.
| Out[0] = float4(r32[0], 0.0, 0.0); | ||
| Out[1] = float4(r32[1], 0.0, 0.0); | ||
| Out[2] = float4(r32[2], 0.0, 0.0); | ||
| Out[3] = r24[0]; | ||
| Out[4] = r24[1]; | ||
| Out[5] = float4(r31[0], 0.0, 0.0, 0.0); | ||
| Out[6] = float4(r31[1], 0.0, 0.0, 0.0); | ||
| Out[7] = float4(r31[2], 0.0, 0.0, 0.0); |
There was a problem hiding this comment.
(For all files)
Same as the other PRs. You have a lot of dead space in Out the way you're packing it.
| Binding: 1 | ||
| #--- end | ||
|
|
||
| # Unimplemented: https://github.com/llvm/llvm-project/issues/184480 |
There was a problem hiding this comment.
(For all files)
Same as the other PRs.
| # Unimplemented: https://github.com/llvm/llvm-project/issues/184480 | |
| # Unimplemented https://github.com/llvm/llvm-project/issues/184480 |
| @@ -0,0 +1,96 @@ | |||
| #--- source.hlsl | |||
There was a problem hiding this comment.
Same as other PRs. I don't think we need fp16 matrix tests for these intrinsics.
| # In[0]: 0, pi/6, pi/4, pi/3 | ||
| # In[1]: pi/2, pi, 3pi/2, 2pi | ||
| # In[2]: -pi/4, -pi/2, -pi, pi/12 | ||
| # In[3]: pi/6, pi/2, pi/12, pi/4 |
There was a problem hiding this comment.
nit: I feel labeling In[0], In[1], etc. doesn't really add anything
| # Out[0]: r32[0]: 0, pi/6 (+ pad) | ||
| # Out[1]: r32[1]: pi/4, pi/3 (+ pad) | ||
| # Out[2]: r32[2]: pi/2, pi (+ pad) | ||
| # Out[3]: r24[0]: pi/2, pi, 3pi/2, 2pi | ||
| # Out[4]: r24[1]: -pi/4, -pi/2, -pi, pi/12 | ||
| # Out[5]: r31[0]: 0 (+ pad) | ||
| # Out[6]: r31[1]: -pi/4 (+ pad) | ||
| # Out[7]: r31[2]: -pi/2 (+ pad) | ||
| # Out[8]: r44[0]: 0, pi/6, pi/4, pi/3 | ||
| # Out[9]: r44[1]: pi/2, pi, 3pi/2, 2pi | ||
| # Out[10]: r44[2]: -pi/4, -pi/2, -pi, pi/12 | ||
| # Out[11]: r44[3]: pi/6, pi/2, pi/12, pi/4 |
There was a problem hiding this comment.
nit: Same as degrees, I feel labeling Out[0] etc. doesn't really add anything. Also it's confusing to have with the r__ labels.
I think a better way to annotate these are to group by matrix shape and use _ as padding like other matrix tests do. For example:
offload-test-suite/test/Feature/HLSLLib/pow_mat.16.test
Lines 89 to 94 in fa2e933
3 participants
Resolves #787, #790
Assisted-by: Claude Opus 4