[PATCH 0/5] MR793: hlsl: Parse profile and register space reservations.
From: Zebediah Figura <zfigura(a)codeweavers.com> "opt" seems to imply "optional", but the optional aspect of these reservations is not (and never has been) encoded into these rules. --- libs/vkd3d-shader/hlsl.y | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 8f7155675..04b3cafeb 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -5683,8 +5683,8 @@ static bool state_block_add_entry(struct hlsl_state_block *state_block, struct h %type <parameters> param_list %type <parameters> parameters -%type <reg_reservation> register_opt -%type <reg_reservation> packoffset_opt +%type <reg_reservation> register_reservation +%type <reg_reservation> packoffset_reservation %type <sampler_dim> texture_type texture_ms_type uav_type rov_type @@ -6308,12 +6308,12 @@ colon_attribute: $$.reg_reservation.reg_type = 0; $$.reg_reservation.offset_type = 0; } - | register_opt + | register_reservation { $$.semantic = (struct hlsl_semantic){0}; $$.reg_reservation = $1; } - | packoffset_opt + | packoffset_reservation { $$.semantic = (struct hlsl_semantic){0}; $$.reg_reservation = $1; @@ -6335,7 +6335,7 @@ semantic: } /* FIXME: Writemasks */ -register_opt: +register_reservation: ':' KW_REGISTER '(' any_identifier ')' { $$ = parse_reg_reservation($4); @@ -6350,7 +6350,7 @@ register_opt: vkd3d_free($6); } -packoffset_opt: +packoffset_reservation: ':' KW_PACKOFFSET '(' any_identifier ')' { $$ = parse_packoffset(ctx, $4, NULL, &@$); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/793
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.h | 2 +- libs/vkd3d-shader/hlsl.y | 60 ++++++++++++++++++++++++++++++++-------- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 3cb98b765..3142a8b76 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -396,7 +396,7 @@ struct hlsl_attribute struct hlsl_reg_reservation { char reg_type; - unsigned int reg_index; + unsigned int reg_space, reg_index; char offset_type; unsigned int offset_index; diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 04b3cafeb..4f153bfd9 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1198,17 +1198,18 @@ static bool add_effect_group(struct hlsl_ctx *ctx, const char *name, struct hlsl return true; } -static struct hlsl_reg_reservation parse_reg_reservation(const char *reg_string) +static bool parse_reservation_index(const char *string, char *type, uint32_t *index) { - struct hlsl_reg_reservation reservation = {0}; + if (!sscanf(string + 1, "%u", index)) + return false; - if (!sscanf(reg_string + 1, "%u", &reservation.reg_index)) - { - FIXME("Unsupported register reservation syntax.\n"); - return reservation; - } - reservation.reg_type = ascii_tolower(reg_string[0]); - return reservation; + *type = ascii_tolower(string[0]); + return true; +} + +static bool parse_reservation_space(const char *string, uint32_t *space) +{ + return !ascii_strncasecmp(string, "space", 5) && sscanf(string + 5, "%u", space); } static struct hlsl_reg_reservation parse_packoffset(struct hlsl_ctx *ctx, const char *reg_string, @@ -6338,16 +6339,51 @@ semantic: register_reservation: ':' KW_REGISTER '(' any_identifier ')' { - $$ = parse_reg_reservation($4); + memset(&$$, 0, sizeof($$)); + if (!parse_reservation_index($4, &$$.reg_type, &$$.reg_index)) + hlsl_error(ctx, &@4, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid register reservation '%s'.", $4); + vkd3d_free($4); } | ':' KW_REGISTER '(' any_identifier ',' any_identifier ')' { - FIXME("Ignoring shader target %s in a register reservation.\n", debugstr_a($4)); + memset(&$$, 0, sizeof($$)); + if (parse_reservation_index($6, &$$.reg_type, &$$.reg_index)) + { + hlsl_fixme(ctx, &@4, "Reservation shader target %s.", $4); + } + else if (parse_reservation_space($6, &$$.reg_space)) + { + if (!parse_reservation_index($4, &$$.reg_type, &$$.reg_index)) + hlsl_error(ctx, &@4, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid register reservation '%s'.", $4); + } + else + { + hlsl_error(ctx, &@6, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid register or space reservation '%s'.", $6); + } + vkd3d_free($4); + vkd3d_free($6); + } + | ':' KW_REGISTER '(' any_identifier ',' any_identifier ',' any_identifier ')' + { + hlsl_fixme(ctx, &@4, "Reservation shader target %s.", $4); - $$ = parse_reg_reservation($6); + memset(&$$, 0, sizeof($$)); + if (!parse_reservation_index($6, &$$.reg_type, &$$.reg_index)) + hlsl_error(ctx, &@6, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid register reservation '%s'.", $6); + + if (!parse_reservation_space($8, &$$.reg_space)) + hlsl_error(ctx, &@8, VKD3D_SHADER_ERROR_HLSL_INVALID_RESERVATION, + "Invalid register space reservation '%s'.", $8); + + vkd3d_free($4); vkd3d_free($6); + vkd3d_free($8); } packoffset_reservation: -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/793
From: Zebediah Figura <zfigura(a)codeweavers.com> --- tests/shader_runner_d3d11.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/shader_runner_d3d11.c b/tests/shader_runner_d3d11.c index b26d6a36b..f0300ad39 100644 --- a/tests/shader_runner_d3d11.c +++ b/tests/shader_runner_d3d11.c @@ -272,7 +272,7 @@ static BOOL init_test_context(struct d3d11_shader_runner *runner) runner->caps.runner = "d3d11.dll"; runner->caps.minimum_shader_model = SHADER_MODEL_4_0; - runner->caps.maximum_shader_model = SHADER_MODEL_5_1; + runner->caps.maximum_shader_model = SHADER_MODEL_5_0; hr = ID3D11Device_CheckFeatureSupport(runner->device, D3D11_FEATURE_DOUBLES, &doubles, sizeof(doubles)); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/793
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 1 + .../register-reservations-profile.shader_test | 261 ++++++++++++++++++ 2 files changed, 262 insertions(+) create mode 100644 tests/hlsl/register-reservations-profile.shader_test diff --git a/Makefile.am b/Makefile.am index f823cbc85..936fb6028 100644 --- a/Makefile.am +++ b/Makefile.am @@ -167,6 +167,7 @@ vkd3d_shader_tests = \ tests/hlsl/reflect.shader_test \ tests/hlsl/refract.shader_test \ tests/hlsl/register-reservations-numeric.shader_test \ + tests/hlsl/register-reservations-profile.shader_test \ tests/hlsl/register-reservations-resources.shader_test \ tests/hlsl/return-implicit-conversion.shader_test \ tests/hlsl/return.shader_test \ diff --git a/tests/hlsl/register-reservations-profile.shader_test b/tests/hlsl/register-reservations-profile.shader_test new file mode 100644 index 000000000..9ad8f6dc0 --- /dev/null +++ b/tests/hlsl/register-reservations-profile.shader_test @@ -0,0 +1,261 @@ +% Tests for register reservations that use the "profile" syntax. + +[require] +shader model >= 4.0 + +[srv 0] +size (2d, 1, 1) +0.0 0.0 0.0 99.0 + +[srv 1] +size (2d, 1, 1) +1.0 1.0 1.0 99.0 + +[srv 2] +size (2d, 1, 1) +2.0 2.0 2.0 99.0 + +[pixel shader todo] +Texture2D tex1 : register(vs, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +% sm6 seems to ignore profile qualifiers entirely. +if(sm>=6) probe all rgba (1, 1, 1, 99) +if(sm<6) probe all rgba (0, 0, 0, 99) + + +[pixel shader todo] +Texture2D tex1 : register(cs, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +if(sm>=6) probe all rgba (1, 1, 1, 99) +if(sm<6) probe all rgba (0, 0, 0, 99) + + +[pixel shader todo] +Texture2D tex1 : register(ps, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +probe all rgba (1, 1, 1, 99) + + +[pixel shader fail todo] +Texture2D tex1 : register(PS, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +[pixel shader fail todo] +Texture2D tex1 : register(qs, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +[pixel shader fail] +Texture2D tex1 : register(ps); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +[pixel shader fail(sm<6) todo] +Texture2D tex1 : register(ps_4, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (1, 1, 1, 99) + + +[pixel shader todo] +Texture2D tex1 : register(ps_4_0, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +probe all rgba (1, 1, 1, 99) + + +[pixel shader fail] +Texture2D tex1 : register(ps.4.0, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +[pixel shader todo] +Texture2D tex1 : register(ps_4_1, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +if(sm>=6) probe all rgba (1, 1, 1, 99) +if(sm<6) probe all rgba (0, 0, 0, 99) + + +[pixel shader todo] +Texture2D tex1 : register(ps_4_1, t1) : register(ps_4_0, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +probe all rgba (2, 2, 2, 99) + +% The documentation explicitly gives this case as an example, implying that a +% more specific profile takes precedence over a less specific one. It doesn't. +% If we're not on the specific version, the compiler silently chooses the +% general case; if we are, the compiler detects a conflict and fails. + +% sm6 is of course differnet. It seems to detect conflicts if the exact same +% reservation is given, but if two different reservations are given it instead +% just picks the last one, without regard to version. + +[pixel shader todo] +Texture2D tex1 : register(ps, t1) : register(ps_5_0, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +if(sm>=6) probe all rgba (2, 2, 2, 99) +if(sm<6) probe all rgba (1, 1, 1, 99) + + +[pixel shader fail(sm<6) todo] +Texture2D tex1 : register(ps, t1) : register(ps_4_0, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (2, 2, 2, 99) + + +[pixel shader fail(sm<6) todo] +Texture2D tex1 : register(ps_4_0, t1) : register(ps, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (2, 2, 2, 99) + + +[pixel shader fail(sm<6) todo] +Texture2D tex1 : register(ps_6_0, t1) : register(ps, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (2, 2, 2, 99) + + +[pixel shader fail todo] +Texture2D tex1 : register(ps_4_0, t1) : register(t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +[pixel shader fail] +Texture2D tex1 : register(t1, ps_4_0); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +% It's also illegal to specify contradictory register reservations with the +% *same* profile, even if they aren't the current one. + +[pixel shader fail todo] +Texture2D tex1 : register(vs_4_0, t1) : register(vs_4_0, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +[pixel shader fail todo] +Texture2D tex1 : register(vs, t1) : register(vs, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +% Specifying the same reservation twice is fine, though. + +[pixel shader todo] +Texture2D tex1 : register(ps, t1) : register(ps, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +probe all rgba (1, 1, 1, 99) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/793
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 1 + .../register-reservations-space.shader_test | 139 ++++++++++++++++++ 2 files changed, 140 insertions(+) create mode 100644 tests/hlsl/register-reservations-space.shader_test diff --git a/Makefile.am b/Makefile.am index 936fb6028..48397cf14 100644 --- a/Makefile.am +++ b/Makefile.am @@ -169,6 +169,7 @@ vkd3d_shader_tests = \ tests/hlsl/register-reservations-numeric.shader_test \ tests/hlsl/register-reservations-profile.shader_test \ tests/hlsl/register-reservations-resources.shader_test \ + tests/hlsl/register-reservations-space.shader_test \ tests/hlsl/return-implicit-conversion.shader_test \ tests/hlsl/return.shader_test \ tests/hlsl/round.shader_test \ diff --git a/tests/hlsl/register-reservations-space.shader_test b/tests/hlsl/register-reservations-space.shader_test new file mode 100644 index 000000000..96ff140a7 --- /dev/null +++ b/tests/hlsl/register-reservations-space.shader_test @@ -0,0 +1,139 @@ +% Tests for register space reservation syntax. We don't bother testing register +% space here, since it's specific to d3d12 and would require reworking a lot of +% the shader test code. + +[require] +shader model >= 5.1 + +[srv 0] +size (2d, 1, 1) +0.0 0.0 0.0 99.0 + +[srv 1] +size (2d, 1, 1) +1.0 1.0 1.0 99.0 + +[srv 2] +size (2d, 1, 1) +2.0 2.0 2.0 99.0 + +[pixel shader todo] +Texture2D tex1 : register(t1, space0); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +probe all rgba (1, 1, 1, 99) + + +[pixel shader todo fail(sm>=6)] +Texture2D tex1 : register(t1, sPaCe0); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +probe all rgba (1, 1, 1, 99) + + +[pixel shader fail todo] +Texture2D tex1 : register(space0, t1); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +[pixel shader fail(sm<6)] +Texture2D tex1 : register(ps, space0); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (0, 0, 0, 99) + + +[pixel shader fail(sm<6)] +Texture2D tex1 : register(space0); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +draw quad +probe all rgba (0, 0, 0, 99) + + +% Specifying a profile is just broken. The first reservation (or, with sm6, the +% last) is taken regardless of whether it actually matches the current profile. + +[pixel shader todo] +Texture2D tex1 : register(vs, t1, space0) : register(ps, t2, space0); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +if(sm>=6) probe all rgba (2, 2, 2, 99) +if(sm<6) probe all rgba (1, 1, 1, 99) + + +% This actually inheres to 5.1+; it doesn't matter whether "space" is specified. + +[pixel shader todo] +Texture2D tex1 : register(vs, t1) : register(ps, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +if(sm>=6) probe all rgba (2, 2, 2, 99) +if(sm<6) probe all rgba (1, 1, 1, 99) + + +% It's still illegal to specify multiple contradictory reservations with the +% same profile... + +[pixel shader fail todo] +Texture2D tex1 : register(vs, t1) : register(vs, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + + +% ...but it's not illegal to specify e.g. ps alongside ps_5_1. + +[pixel shader todo] +Texture2D tex1 : register(ps, t1) : register(ps_5_1, t2); + +float4 main() : sv_target +{ + return tex1.Load(int3(0, 0, 0)); +} + +[test] +todo(sm<6) draw quad +if(sm>=6) probe all rgba (2, 2, 2, 99) +if(sm<6) probe all rgba (1, 1, 1, 99) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/793
participants (2)
-
Zebediah Figura -
Zebediah Figura (＠zfigura)