Conversation
This commits adds multiple tests around user & system semantics. This commit also enables the only graphic tests for Clang+Vulkan as it is now supported.
s-perron
left a comment
s-perron
left a comment
There was a problem hiding this comment.
Generally looks good. I think one test could be improved.
s-perron
left a comment
s-perron
left a comment
There was a problem hiding this comment.
I think the testing is good enough, but a test could be
s-perron
left a comment
s-perron
left a comment
There was a problem hiding this comment.
I think we should try to simplify the compute shader test. I also don't understand the non-determinism that you are seeing.
I made a few other comments that can be applied in other places too. For example, all of the tests should be in the semantics folder.
There was a problem hiding this comment.
Sorry, I don't understand what is going on with this test.
- What exactly are you trying to test?
I'm guessing that you are trying to test that the 4 thread-related systems semantics are giving the correct values. You do this by trying to compare them to each other. Comments describing what is being output, and why you expect them to be that value.
See test/Feature/ResourceArrays/array-global.test
There was a problem hiding this comment.
Here is a sample that I wrote. I wonder why something like this cannot work?
#--- system_values.hlsl
RWStructuredBuffer<uint> Out[] : register(u0);
[numthreads(2, 2, 2)]
void main(uint3 DTID : SV_DispatchThreadID, uint3 GID : SV_GroupID, uint3 GTID : SV_GroupThreadID, uint GI : SV_GroupIndex) {
// Computing the flat global index.
const uint FID = GID.z * 32 + GID.y * 16 + GID.x * 8 + GI;
Out[FID][0] = GI;
Out[FID][1] = GTID.x;
Out[FID][2] = GTID.y;
Out[FID][3] = GTID.z;
Out[FID][4] = GID.x;
Out[FID][5] = GID.y;
Out[FID][6] = GID.z;
Out[FID][7] = DTID.x;
Out[FID][8] = DTID.y;
Out[FID][9] = DTID.z;
}
//--- system_values.yaml
---
Shaders:
- Stage: Compute
Entry: main
DispatchSize: [2, 2, 2]
Buffers:
- Name: Out
Format: Hex32
ArraySize: 64
# dispatch: 2 * 2 * 2 = 8
# numthreads: 2 * 2 * 2 = 8
# thread count: 8 * 8 = 64
# each writes 10 uint: 64 * 10 = 640
# each uint is 4 bytes: 640 * 4 = 2560
FillSize: 40
FillValue: 0xFF
DescriptorSets:
- Resources:
- Name: Out
Kind: RWStructuredBuffer
DirectXBinding:
Register: 0
Space: 0
VulkanBinding:
Binding: 0
...
There was a problem hiding this comment.
Then you can do 64 checks to make sure you have the correct values, or add the ExpectedValues as I mentioned in another comment, and make sure that each line has consistent values.
| @@ -0,0 +1,1567 @@ | |||
| #--- system_values.hlsl | |||
| RWStructuredBuffer<uint4> Out : register(u0); | |||
There was a problem hiding this comment.
Could we make this an array of resources? You could have a separate resource for each thread. I could make the test easier to check.
| // Values will le randomly located in the buffer due to the dispatch | ||
| // order. And due to yaml formatting, we cannot rely on a per-tuple | ||
| // key to to the unordered check. |
There was a problem hiding this comment.
I don't get this comment. The location of the value in the buffer is determined by the FID, which should be unique for each thread. How does the dispatch order affect the output?
| # Due to the way the YAML file is formatted, and the way work groups | ||
| # are scheduled, we have to do a per-item CHECK-DAG. Each value is composed | ||
| # as followed: | ||
| # +-------------+----------------+ | ||
| # | high 16-bit | low 16-bit | | ||
| # | UNIQUE KEY | VALUE TO CHECK | | ||
| # +-------------+----------------+ | ||
| # The unique key starts at 0 and increments for each entry in the buffer. | ||
| # Order might change, but not the unique-key <> value association. |
There was a problem hiding this comment.
There is another way to test the contents of the buffers.
offload-test-suite/test/Feature/ResourceArrays/array-global.test
Lines 79 to 86 in c7c9470
offload-test-suite/test/Feature/ResourceArrays/array-global.test
Lines 98 to 102 in c7c9470
| # Due to the way the YAML file is formatted, and the way work groups | ||
| # are scheduled, we have to do a per-item CHECK-DAG. Each value is composed | ||
| # as followed: | ||
| # +-------------+----------------+ | ||
| # | high 16-bit | low 16-bit | | ||
| # | UNIQUE KEY | VALUE TO CHECK | | ||
| # +-------------+----------------+ | ||
| # The unique key starts at 0 and increments for each entry in the buffer. | ||
| # Order might change, but not the unique-key <> value association. |
There was a problem hiding this comment.
I don't understand how the order might change. Can you explain that?
There was a problem hiding this comment.
I know this is a vertex and pixel shader, but could this be in the "Semantics" folder? That is what you are testing.
| # CHECK: Data: | ||
| # CHECK: [ 0.5, 1, 0, 1 ] |
There was a problem hiding this comment.
How do you know you're getting the right data? You should be looking for output.
# CHECK: Name: Output
# CHECK: Data:
# CHECK-SAME: [ 0.5, 1, 0, 1 ]
There was a problem hiding this comment.
What is a shadowed semantic? A comment might be nice.
2 participants
This commits adds multiple tests around user & system semantics. This commit also enables the only graphic tests for Clang+Vulkan as it is now supported.