Currently we are not properly handling register(cX) reservations for SM1, this is one of the things required for the SNK shaders (CW Bug Bug 18092).
register(cX) reservations also change the offset in the $Globals buffer in SM4, so support for this is also included.
---
Patch 1/4 is required to specify: ``` [require] shader model < 4.0 ``` so that the tests that follow do not get run with the vulkan backend on SM4. I think nobody disagreed with that patch.
From: Francisco Casas fcasas@codeweavers.com
We need these checks to properly handle tests that require target profiles 3.0 and lower, or tests that require target profile 6.0 and higher.
Since all backeds require an ops->check_requirements now, expect it to always be defined. --- tests/shader_runner.c | 3 ++- tests/shader_runner.h | 4 ++-- tests/shader_runner_d3d11.c | 13 +++++++++++++ tests/shader_runner_d3d12.c | 14 ++++++++++++++ tests/shader_runner_vulkan.c | 13 +++++++++++++ 5 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/tests/shader_runner.c b/tests/shader_runner.c index dadfe3338..43949c01a 100644 --- a/tests/shader_runner.c +++ b/tests/shader_runner.c @@ -1089,8 +1089,9 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o break;
case STATE_REQUIRE: + assert(runner->ops->check_requirements); if (runner->maximum_shader_model < runner->minimum_shader_model - || (runner->ops->check_requirements && !runner->ops->check_requirements(runner))) + || !runner->ops->check_requirements(runner)) { skip_tests = true; } diff --git a/tests/shader_runner.h b/tests/shader_runner.h index bda44a429..ee0e9094d 100644 --- a/tests/shader_runner.h +++ b/tests/shader_runner.h @@ -142,9 +142,9 @@ struct shader_runner
struct shader_runner_ops { - /* Returns false if unable to run the given tests. If NULL, all tests are - * run. */ + /* Returns false if unable to run the given tests. */ bool (*check_requirements)(struct shader_runner *runner); + struct resource *(*create_resource)(struct shader_runner *runner, const struct resource_params *params); void (*destroy_resource)(struct shader_runner *runner, struct resource *resource); bool (*draw)(struct shader_runner *runner, D3D_PRIMITIVE_TOPOLOGY primitive_topology, unsigned int vertex_count); diff --git a/tests/shader_runner_d3d11.c b/tests/shader_runner_d3d11.c index bd9d363ce..7a8f8dac0 100644 --- a/tests/shader_runner_d3d11.c +++ b/tests/shader_runner_d3d11.c @@ -100,6 +100,18 @@ static ID3D10Blob *compile_shader(const struct d3d11_shader_runner *runner, cons return blob; }
+static bool d3d11_runner_check_requirements(struct shader_runner *r) +{ + struct d3d11_shader_runner *runner = d3d11_shader_runner(r); + + if (runner->r.maximum_shader_model < SHADER_MODEL_4_0) + return false; + if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0) + return false; + + return true; +} + static IDXGIAdapter *create_adapter(void) { IDXGIFactory4 *factory4; @@ -724,6 +736,7 @@ static void d3d11_runner_release_readback(struct shader_runner *r, struct resour
static const struct shader_runner_ops d3d11_runner_ops = { + .check_requirements = d3d11_runner_check_requirements, .create_resource = d3d11_runner_create_resource, .destroy_resource = d3d11_runner_destroy_resource, .dispatch = d3d11_runner_dispatch, diff --git a/tests/shader_runner_d3d12.c b/tests/shader_runner_d3d12.c index a05dfd059..bb50564fa 100644 --- a/tests/shader_runner_d3d12.c +++ b/tests/shader_runner_d3d12.c @@ -96,6 +96,19 @@ static ID3D10Blob *compile_shader(const struct d3d12_shader_runner *runner, cons return blob; }
+static bool d3d12_runner_check_requirements(struct shader_runner *r) +{ + struct d3d12_shader_runner *runner = d3d12_shader_runner(r); + + if (runner->r.maximum_shader_model < SHADER_MODEL_4_0) + return false; + + if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0 && !runner->dxc_compiler) + return false; + + return true; +} + #define MAX_RESOURCE_DESCRIPTORS (MAX_RESOURCES * 2)
static struct resource *d3d12_runner_create_resource(struct shader_runner *r, const struct resource_params *params) @@ -561,6 +574,7 @@ static void d3d12_runner_release_readback(struct shader_runner *r, struct resour
static const struct shader_runner_ops d3d12_runner_ops = { + .check_requirements = d3d12_runner_check_requirements, .create_resource = d3d12_runner_create_resource, .destroy_resource = d3d12_runner_destroy_resource, .dispatch = d3d12_runner_dispatch, diff --git a/tests/shader_runner_vulkan.c b/tests/shader_runner_vulkan.c index f89b4d624..ec2d1fc9e 100644 --- a/tests/shader_runner_vulkan.c +++ b/tests/shader_runner_vulkan.c @@ -83,6 +83,18 @@ static struct vulkan_shader_runner *vulkan_shader_runner(struct shader_runner *r
#define VK_CALL(f) (runner->f)
+static bool vulkan_runner_check_requirements(struct shader_runner *r) +{ + struct vulkan_shader_runner *runner = vulkan_shader_runner(r); + + if (runner->r.maximum_shader_model < SHADER_MODEL_4_0) + return false; + if (runner->r.minimum_shader_model >= SHADER_MODEL_6_0) + return false; + + return true; +} + static void begin_command_buffer(struct vulkan_shader_runner *runner) { VkCommandBufferBeginInfo buffer_begin_desc = {.sType = VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO}; @@ -1157,6 +1169,7 @@ static void vulkan_runner_release_readback(struct shader_runner *r, struct resou
static const struct shader_runner_ops vulkan_runner_ops = { + .check_requirements = vulkan_runner_check_requirements, .create_resource = vulkan_runner_create_resource, .destroy_resource = vulkan_runner_destroy_resource, .dispatch = vulkan_runner_dispatch,
From: Francisco Casas fcasas@codeweavers.com
--- tests/hlsl/register-reservations.shader_test | 88 ++++++++++++++++++++ 1 file changed, 88 insertions(+)
diff --git a/tests/hlsl/register-reservations.shader_test b/tests/hlsl/register-reservations.shader_test index 9539b05ff..5664b0d24 100644 --- a/tests/hlsl/register-reservations.shader_test +++ b/tests/hlsl/register-reservations.shader_test @@ -1,6 +1,94 @@ +[pixel shader fail(sm<6) todo] +// Overlapping register(cX) reservations should only be allowed on SM6. +float a : register(c0); +float b : register(c0); + +float4 main() : sv_target +{ + return a + b; +} + + +[require] +shader model < 4.0 + +[pixel shader] +struct +{ + float2 a; + float b; +} apple : register(c2); + +float4 main() : sv_target +{ + return float4(apple.a, apple.b, 0); +} + +[test] +uniform 0 float4 0.1 0.2 0.3 0.4 +uniform 4 float4 1.1 1.2 1.3 1.4 +uniform 8 float4 2.1 2.2 2.3 2.4 +uniform 12 float4 3.1 3.2 3.3 3.4 +draw quad +todo probe all rgba (2.1, 2.2, 3.1, 0.0) + + [require] shader model >= 4.0
+[pixel shader] +struct +{ + float2 a; + float b; +} apple : register(c2); + +float4 main() : sv_target +{ + return float4(apple.a, apple.b, 0); +} + +[test] +uniform 0 float4 0.1 0.2 0.3 0.4 +uniform 4 float4 1.1 1.2 1.3 1.4 +uniform 8 float4 2.1 2.2 2.3 2.4 +uniform 12 float4 3.1 3.2 3.3 3.4 +draw quad +todo(sm<6) probe all rgba (2.1, 2.2, 2.3, 0.0) + + +[pixel shader] +// On SM4 register(cX) has no effect unless in the $Globals buffer. +cbuffer extra +{ + float a : register(c1); +}; + +float4 main() : sv_target +{ + return a; +} + +[test] +uniform 0 float 100 +uniform 4 float 101 +draw quad +probe all rgba (100, 100, 100, 100) + + +[pixel shader fail(sm>=6)] +// On SM4 register(cX) has no effect unless in the $Globals buffer. +float4 main(uniform float a : register(c1)) : sv_target +{ + return a; +} + +[test] +uniform 0 float 100 +uniform 4 float 101 +draw quad +probe all rgba (100, 100, 100, 100) +
[texture 0] size (1, 1)
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/hlsl_codegen.c | 28 ++++++++++++++++++-- tests/hlsl/register-reservations.shader_test | 2 +- 2 files changed, 27 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 76a010fb3..56ef0aa5b 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -3954,8 +3954,32 @@ static void allocate_const_registers(struct hlsl_ctx *ctx, struct hlsl_ir_functi if (reg_size == 0) continue;
- var->regs[HLSL_REGSET_NUMERIC] = allocate_numeric_registers_for_type(ctx, &allocator, - 1, UINT_MAX, var->data_type); + if (var->reg_reservation.reg_type == 'c') + { + unsigned int reg_idx = var->reg_reservation.reg_index; + unsigned int i; + + assert(reg_size % 4 == 0); + for (i = 0; i < reg_size / 4; ++i) + { + if (get_available_writemask(&allocator, 1, UINT_MAX, reg_idx + i) != VKD3DSP_WRITEMASK_ALL) + { + hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Overlapping register() reservations on 'c%u'.", reg_idx + i); + } + + record_allocation(ctx, &allocator, reg_idx + i, VKD3DSP_WRITEMASK_ALL, 1, UINT_MAX); + } + + var->regs[HLSL_REGSET_NUMERIC].id = reg_idx; + var->regs[HLSL_REGSET_NUMERIC].allocation_size = reg_size / 4; + var->regs[HLSL_REGSET_NUMERIC].allocated = true; + } + else + { + var->regs[HLSL_REGSET_NUMERIC] = allocate_numeric_registers_for_type(ctx, &allocator, + 1, UINT_MAX, var->data_type); + } TRACE("Allocated %s to %s.\n", var->name, debug_register('c', var->regs[HLSL_REGSET_NUMERIC], var->data_type)); } diff --git a/tests/hlsl/register-reservations.shader_test b/tests/hlsl/register-reservations.shader_test index 5664b0d24..a2ade367c 100644 --- a/tests/hlsl/register-reservations.shader_test +++ b/tests/hlsl/register-reservations.shader_test @@ -30,7 +30,7 @@ uniform 4 float4 1.1 1.2 1.3 1.4 uniform 8 float4 2.1 2.2 2.3 2.4 uniform 12 float4 3.1 3.2 3.3 3.4 draw quad -todo probe all rgba (2.1, 2.2, 3.1, 0.0) +probe all rgba (2.1, 2.2, 3.1, 0.0)
[require]
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/hlsl_codegen.c | 7 ++++++- tests/hlsl/register-reservations.shader_test | 4 ++-- 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 56ef0aa5b..caf766f43 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -4121,7 +4121,12 @@ static void calculate_buffer_offset(struct hlsl_ctx *ctx, struct hlsl_ir_var *va enum hlsl_type_class var_class = var->data_type->class; struct hlsl_buffer *buffer = var->buffer;
- if (var->reg_reservation.offset_type == 'c') + if (var->reg_reservation.reg_type == 'c' && var->buffer == ctx->globals_buffer) + { + /* On SM4, register(cX) reservations are translated to buffer offsets, but only for the $Globals buffer. */ + var->buffer_offset = 4 * var->reg_reservation.reg_index; + } + else if (var->reg_reservation.offset_type == 'c') { if (var->reg_reservation.offset_index % 4) { diff --git a/tests/hlsl/register-reservations.shader_test b/tests/hlsl/register-reservations.shader_test index a2ade367c..74674dfc7 100644 --- a/tests/hlsl/register-reservations.shader_test +++ b/tests/hlsl/register-reservations.shader_test @@ -1,4 +1,4 @@ -[pixel shader fail(sm<6) todo] +[pixel shader fail(sm<6)] // Overlapping register(cX) reservations should only be allowed on SM6. float a : register(c0); float b : register(c0); @@ -54,7 +54,7 @@ uniform 4 float4 1.1 1.2 1.3 1.4 uniform 8 float4 2.1 2.2 2.3 2.4 uniform 12 float4 3.1 3.2 3.3 3.4 draw quad -todo(sm<6) probe all rgba (2.1, 2.2, 2.3, 0.0) +probe all rgba (2.1, 2.2, 2.3, 0.0)
[pixel shader]
Zebediah Figura (@zfigura) commented about tests/hlsl/register-reservations.shader_test:
+[pixel shader fail(sm<6) todo] +// Overlapping register(cX) reservations should only be allowed on SM6.
Why, what happens on sm6?
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl_codegen.c:
hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION,"Overlapping register() reservations on 'c%u'.", reg_idx + i);}record_allocation(ctx, &allocator, reg_idx + i, VKD3DSP_WRITEMASK_ALL, 1, UINT_MAX);}var->regs[HLSL_REGSET_NUMERIC].id = reg_idx;var->regs[HLSL_REGSET_NUMERIC].allocation_size = reg_size / 4;var->regs[HLSL_REGSET_NUMERIC].allocated = true;}else{var->regs[HLSL_REGSET_NUMERIC] = allocate_numeric_registers_for_type(ctx, &allocator,1, UINT_MAX, var->data_type);}
What about types other than 'c'? I know 'i' and 'b' do something that we don't implement [and should maybe have a hlsl_fixme() for]; what about invalid types?
Zebediah Figura (@zfigura) commented about libs/vkd3d-shader/hlsl_codegen.c:
enum hlsl_type_class var_class = var->data_type->class; struct hlsl_buffer *buffer = var->buffer;
- if (var->reg_reservation.offset_type == 'c')
- if (var->reg_reservation.reg_type == 'c' && var->buffer == ctx->globals_buffer)
- {
/* On SM4, register(cX) reservations are translated to buffer offsets, but only for the $Globals buffer. */var->buffer_offset = 4 * var->reg_reservation.reg_index;- }
- else if (var->reg_reservation.offset_type == 'c')
This doesn't include any of the validity checks for packoffset(). Should it? We don't have tests for this.
It'd be nice to have tests for mixing register and non-register allocations, e.g. to make sure that precedence and overlapping works as expected.
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Zebediah Figura (＠zfigura)