Conversation
| void main(uint3 tid : SV_GroupThreadID) | ||
| { |
There was a problem hiding this comment.
We should generally try to stick to LLVM's C++ style for the HLSL in these tests unless we have a good reason not to.
| void main(uint3 tid : SV_GroupThreadID) | |
| { | |
| void main(uint3 TID : SV_GroupThreadID) { |
| Shaders: | ||
| - Stage: Compute | ||
| Entry: main | ||
| DispatchSize: [1,1,1] |
There was a problem hiding this comment.
Whitespace between list elements helps readability
| DispatchSize: [1,1,1] | |
| DispatchSize: [1, 1, 1] |
| Data: [ | ||
| 3,0,0,0, | ||
| 5,0,0,0, | ||
| 6,0,0,0, | ||
| 9,0,0,0 | ||
| ] |
There was a problem hiding this comment.
Similarly
| Data: [ | |
| 3,0,0,0, | |
| 5,0,0,0, | |
| 6,0,0,0, | |
| 9,0,0,0 | |
| ] | |
| Data: [ | |
| 3, 0, 0, 0, | |
| 5, 0, 0, 0, | |
| 6, 0, 0, 0, | |
| 9, 0, 0, 0 | |
| ] |
|
|
||
| - Name: In | ||
| Format: UInt32 | ||
| Stride: 16 |
There was a problem hiding this comment.
Is there a reason that In is StructuredBuffer<uint4> rather than StructuredBuffer<uint>? We only ever access the x element.
| - Name: ExpectedOut2 | ||
| Format: UInt32 | ||
| Stride: 4 | ||
| Data: [0, 0, 15, 15] |
There was a problem hiding this comment.
This is checking that we're getting the result of just 9 | 6, which is 0xf, but the values we're using are kind of unfortunate, since if we did or that with 3 | 5 we wouldn't set any extra bits. Should we adjust the input values a bit so it's more obvious exactly which ones are being or'd together?
| - Name: ExpectedOut3 | ||
| Format: UInt32 | ||
| Stride: 4 | ||
| Data: [15, 15, 15, 15] |
There was a problem hiding this comment.
Case in point, this is an or of all four values, but it's indistinguishable from an or of the last two threads only.
| for (uint i = 0; i < 2; i++) | ||
| r = WaveActiveBitOr(r); |
There was a problem hiding this comment.
All lanes are active on every iteration, right? What is this testing?
There was a problem hiding this comment.
The important part is the loop construct, which introduces some different control flow.
You'll notice the XFAIL here: llvm/llvm-project#188323
This loop created a situation where spirv-val complains. The semantics of the code aren't really important, the fact that a loop exists tests whether the compiler handles different control flow scenarios well.
There was a problem hiding this comment.
Is there an argument for a loop case that has some divergence as well?
| Data: [ | ||
| 17,2,4,8, | ||
| 16,32,64,128, | ||
| 256,512,1024,2048, | ||
| 4096,8192,16384,32768 | ||
| ] |
There was a problem hiding this comment.
Since we're using or here, I think specifying the inputs and outputs in hex will be clearer.
bogner
left a comment
bogner
left a comment
There was a problem hiding this comment.
A lot of the variables in the tests aren’t following the llvm capitalization conventions (TID, V, R rather than tid, v, r), so it would be nice to make that consistent. Otherwise, LGTM!
3 participants
This PR adds tests for WaveActiveBitOr.
WaveActiveBitOr only accepts uint and uint64 types.
The PR also adds a control flow test, since this operation is convergent.
Fixes #92
Assisted by: Claude Opus 4.6