-
Notifications
You must be signed in to change notification settings - Fork 22
[RootSignature] Add frontend RootSignature testing #45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
this commit adds testing demonstrating the various behaviours of specifying a root signature through a function attribute in HLSL for any non-sampler descriptor type - Defaults.test: all default parameter values - Flags.test: various root/descriptor flags for M3 - NumberParameters.test: integer parameter edge-cases - ManualDescriptors.test: shows manual user specified `offset` and `numDescriptors` parameters and the edge-cases - ParameterInsensitivity.test: shows that parameters have no specified order, as well as, the case-insensitivity of keywords and enums
@inbelic the new M3/RootSignatures/Flags.test seems to be crashing the test runner. If you have time to debug please do, otherwise I'll put it on my queue. |
- the UAV buffer was incorrectly described as static but was being written to - this commit makes the constant input take the DATA_STATIC flag in hopes of resolved the test runner failures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
test/M3/RootSignatures/Defaults.test
Outdated
RWStructuredBuffer<Output> Out1 : register(u1); | ||
RWStructuredBuffer<Output> Out2 : register(u2); | ||
|
||
// Root signature to sanity test default values optional values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment looks like it lost a word somewhere. Can you clarify it please?
|
||
// Root signature to test edge-cases of specify number arguments: | ||
// - Maximum valid register value (0xfffffffe = 4294967294) | ||
// - Maximum valid register space value (0x7fffffff = 4294967279) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0x7fffffff == 2147483647
0xffffffef == 4294967279
Which is this supposed to be?
Correct clarifying typos
"RootConstants(num32BitConstants = +61, b0), " \ | ||
"DescriptorTable( " \ | ||
" SRV(t4294967294), " \ | ||
" UAV(u1, space = 4294967279) " \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be concerned that the space specified here is much larger than the "maximum valid register space value" mentioned above? In any case, the comment and the test don't match about what we're doing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh sorry. It should have been 0xffffffef
as the max value. I did the great math of 0xfffffff0 - 1 = 0x7fffffff
and didn't put together that is what your comment was pointing out... So it is functionally correct
adds testing demonstrating the various behaviours of specifying a root signature through a function attribute in HLSL for non-sampler descriptor types
Defaults.test
: all default parameter valuesFlags.test
: various root/descriptor flags for M3NumberParameters.test
: integer parameter edge-casesManualDescriptors.test
: shows manual user specifiedoffset
andnumDescriptors
parameters and the edge-casesParameterInsensitivity.test
: shows that parameters have no specified order, as well as, the case-insensitivity of keywords and enumsNoted discrepancies with the root signature specification are tracked to be resolved here: llvm/wg-hlsl#192.
Resolves: llvm/llvm-project#130826