Add matrix tests for pure float->float intrinsics - #1471
danbrown-amd wants to merge 2 commits into
Conversation
kmpeng
left a comment
kmpeng
left a comment
There was a problem hiding this comment.
All your tests are failing - you need to XFAIL: Clang them since the matrix implementations aren't in.
danbrown-amd
changed the title
4fde249 to
3f1828a
Compare
2dbfbc2 to
1bfe8c2
Compare
| float3x2 R32 = acos(float3x2(In[0].xy, In[0].zw, In[1].xy)); | ||
| float2x4 R24 = acos(float2x4(In[1], In[2])); | ||
| float3x1 R31 = acos(float3x1(In[4].xyz)); | ||
| float4x4 R44 = acos(float4x4(In[5], In[6], In[7], In[8])); | ||
| float2x2 R22 = acos(float2x2(-1.0, -0.5, 0.5, 1.0)); |
There was a problem hiding this comment.
(For all files)
The In indexing is confusing/hard to follow. For example, In[3] is completely skipped. In[3] and In[4].w are also populated with real values but never used.
I would rather we follow the same format as already merged matrix tests.
| -1.0, -0.86603, -0.70711, -0.5, | ||
| 0.0, 0.5, 0.70711, 0.86603, | ||
| 1.0, -0.5, 0.70711, -0.86603, | ||
| -0.86603, 0.86603, -0.5, -0.70711 ] |
There was a problem hiding this comment.
(For all files)
Seems your In[5]-In[8] values are just exact duplicates of In[0]-In[3]. Can we test some new values? Or if you don't think that's necessary, let's at least reuse In[0]-In[3] so we don't have to allocate space for repeat values.
| # 3x2: -1, -sqrt(3)/2, -1/sqrt(2), -0.5, 0, 0.5, _, _ | ||
| # 2x4: 0, 0.5, 1/sqrt(2), sqrt(3)/2, 1, -0.5, 1/sqrt(2), -sqrt(3)/2 | ||
| # 3x1: -1, -0.5, 0.5, _ | ||
| # 4x4: -1, -sqrt(3)/2, -1/sqrt(2), -0.5, 0, 0.5, 1/sqrt(2), sqrt(3)/2, | ||
| # 1, -0.5, 1/sqrt(2), -sqrt(3)/2, -sqrt(3)/2, sqrt(3)/2, -0.5, -1/sqrt(2) |
There was a problem hiding this comment.
(For all files that comment on buffer values)
This comment format doesn't feel very useful for In. I would rather we use the format other matrix tests have used for the In buffers.
| 0.0, 2.0944, 0.7854, 2.61799, | ||
| 2.61799, 0.5236, 2.0944, 2.35619, | ||
| 3.14159, 2.0944, 1.0472, 0.0 ] | ||
| Results: |
There was a problem hiding this comment.
(For all files that comment on buffer values)
No comment on output buffer values?
| - Name: In | ||
| Format: Float32 | ||
| Stride: 16 | ||
| Data: [ -89.0, -2.0, -1.0, 0.0, |
There was a problem hiding this comment.
Doesn't exp(-89) = a denormal?
| Results: | ||
| - Result: Test1 | ||
| Rule: BufferFloatEpsilon | ||
| Epsilon: 0.0008 |
There was a problem hiding this comment.
This uses Epsilon: 0.0008 but its vector/scalar test + all sqrt tests use ULP: 1. It would be good to be consistent.
| 0.0, 0.5, 0.75, 1.0 ] | ||
| Results: | ||
| - Result: Test1 | ||
| Rule: BufferExact |
There was a problem hiding this comment.
Sorry I said this could be changed to BufferExact, but I think we should leave it as Epsilon: 0 to keep it consistent with similar tests. BufferExact also handles some values differently from Epsilon: 0, which shouldn't matter here currently but may in the future.
| # 3x2: 0, 1/16, 1/4, 1, 2, 4, _, _ | ||
| # 2x4: 2, 4, 9, 16, 100, 1, 4, 1/16 | ||
| # 3x1: 1, 4, 9, _ | ||
| # 4x4: 0, 1/16, 1/4, 1, 2, 4, 9, 16, | ||
| # 100, 1, 4, 1/16, 1/16, 1/4, 9, 16 |
|
llvm/llvm-project#220723 was fixed so you can remove those XFAILs |
3 participants
Resolves #791, #797, #800, #801, #802, #804, #805, #806, #807, #808, #809, #812, #813, #814, #819, #820, #821, #822, #823, #824, #825, #826
Assisted-by: Claude Opus 4