[PATCH v3 0/5] MR577: hlsl: Improve condition type validation.
-- v3: tests: Add tests for valid conditional types. vkd3d-shader/hlsl: Always cast to bool in if() statements. vkd3d-shader/hlsl: Validate that condition expressions are numeric. vkd3d-shader/hlsl: Validate the condition data type for loops as well. vkd3d-shader/hlsl: Copy some missing fields in hlsl_type_clone(). https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 6a5a6d0e3..d43ea7943 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -953,6 +953,8 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old, type->modifiers |= default_majority; type->sampler_dim = old->sampler_dim; type->is_minimum_precision = old->is_minimum_precision; + type->sample_count = old->sample_count; + switch (old->class) { case HLSL_CLASS_ARRAY: @@ -1002,11 +1004,11 @@ struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old, } case HLSL_CLASS_OBJECT: - { if (type->base_type == HLSL_TYPE_TECHNIQUE) type->e.version = old->e.version; + if (old->base_type == HLSL_TYPE_TEXTURE || old->base_type == HLSL_TYPE_UAV) + type->e.resource_format = old->e.resource_format; break; - } default: break; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index c308916e0..caa88fe47 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -292,6 +292,21 @@ static bool implicit_compatible_data_types(struct hlsl_ctx *ctx, struct hlsl_typ return hlsl_types_are_componentwise_equal(ctx, src, dst); } +static void check_condition_type(struct hlsl_ctx *ctx, const struct hlsl_ir_node *cond) +{ + const struct hlsl_type *type = cond->data_type; + + if (type->dimx > 1 || type->dimy > 1) + { + struct vkd3d_string_buffer *string; + + if ((string = hlsl_type_to_string(ctx, type))) + hlsl_error(ctx, &cond->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Condition type '%s' is not a scalar numeric type.", string->buffer); + hlsl_release_string_buffer(ctx, string); + } +} + static struct hlsl_ir_node *add_cast(struct hlsl_ctx *ctx, struct hlsl_block *block, struct hlsl_ir_node *node, struct hlsl_type *dst_type, const struct vkd3d_shader_location *loc) { @@ -431,6 +446,9 @@ static bool append_conditional_break(struct hlsl_ctx *ctx, struct hlsl_block *co return true; condition = node_from_block(cond_block); + + check_condition_type(ctx, condition); + if (!(not = hlsl_new_unary_expr(ctx, HLSL_OP1_LOGIC_NOT, condition, &condition->loc))) return false; hlsl_block_add_instr(cond_block, not); @@ -6697,15 +6715,9 @@ selection_statement: } destroy_block($6.then_block); destroy_block($6.else_block); - if (condition->data_type->dimx > 1 || condition->data_type->dimy > 1) - { - struct vkd3d_string_buffer *string; - if ((string = hlsl_type_to_string(ctx, condition->data_type))) - hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, - "if condition type %s is not scalar.", string->buffer); - hlsl_release_string_buffer(ctx, string); - } + check_condition_type(ctx, condition); + $$ = $4; hlsl_block_add_instr($$, instr); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577
From: Zebediah Figura <zfigura(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index caa88fe47..110db99da 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -296,7 +296,7 @@ static void check_condition_type(struct hlsl_ctx *ctx, const struct hlsl_ir_node { const struct hlsl_type *type = cond->data_type; - if (type->dimx > 1 || type->dimy > 1) + if (type->class > HLSL_CLASS_LAST_NUMERIC || type->dimx > 1 || type->dimy > 1) { struct vkd3d_string_buffer *string; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577
From: Zebediah Figura <zfigura(a)codeweavers.com> We emit sm4 if_nz for these, but that does a bitwise comparison to zero, which is wrong for floats. --- libs/vkd3d-shader/hlsl.y | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 110db99da..e3dc2d92c 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -6707,6 +6707,15 @@ selection_statement: } } + check_condition_type(ctx, condition); + + if (!(condition = add_cast(ctx, $4, condition, hlsl_get_scalar_type(ctx, HLSL_TYPE_BOOL), &@4))) + { + destroy_block($6.then_block); + destroy_block($6.else_block); + YYABORT; + } + if (!(instr = hlsl_new_if(ctx, condition, $6.then_block, $6.else_block, &@2))) { destroy_block($6.then_block); @@ -6716,8 +6725,6 @@ selection_statement: destroy_block($6.then_block); destroy_block($6.else_block); - check_condition_type(ctx, condition); - $$ = $4; hlsl_block_add_instr($$, instr); } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577
From: Zebediah Figura <zfigura(a)codeweavers.com> --- Makefile.am | 1 + tests/hlsl/cf-cond-types.shader_test | 131 +++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) create mode 100644 tests/hlsl/cf-cond-types.shader_test diff --git a/Makefile.am b/Makefile.am index 648dfb255..7a6381ca4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -72,6 +72,7 @@ vkd3d_shader_tests = \ tests/hlsl/cast-to-uint.shader_test \ tests/hlsl/cbuffer.shader_test \ tests/hlsl/ceil.shader_test \ + tests/hlsl/cf-cond-types.shader_test \ tests/hlsl/clamp.shader_test \ tests/hlsl/clip.shader_test \ tests/hlsl/combined-samplers.shader_test \ diff --git a/tests/hlsl/cf-cond-types.shader_test b/tests/hlsl/cf-cond-types.shader_test new file mode 100644 index 000000000..9a6e800ab --- /dev/null +++ b/tests/hlsl/cf-cond-types.shader_test @@ -0,0 +1,131 @@ +% Condition to if/for/while must be numeric. They must also be 1-component +% (although not per se scalar — vectors and matrices are allowed, but not +% arrays). +% +% In theory an implicit conversion to bool is being performed, except that, +% unlike other places where implicit conversions are performed (assignments, +% return values) vectors are not truncated, but instead result in an error. + +[pixel shader fail] +static const float2 f; +float4 main() : sv_target +{ + if (f); + return 0; +} + +[pixel shader fail] +static const float f[1]; +float4 main() : sv_target +{ + if (f); + return 0; +} + +[pixel shader fail] +static const Texture2D f; +float4 main() : sv_target +{ + if (f); + return 0; +} + +[pixel shader fail] +static const float2 f; +float4 main() : sv_target +{ + while (f); + return 0; +} + +[pixel shader fail] +static const float f[1]; +float4 main() : sv_target +{ + while (f); + return 0; +} + +[pixel shader fail] +static const Texture2D f; +float4 main() : sv_target +{ + while (f); + return 0; +} + +[pixel shader fail] +static const float2 f; +float4 main() : sv_target +{ + do; while(f); + return 0; +} + +[pixel shader fail] +static const float f[1]; +float4 main() : sv_target +{ + do; while(f); + return 0; +} + +[pixel shader fail] +static const Texture2D f; +float4 main() : sv_target +{ + do; while(f); + return 0; +} + +[pixel shader fail] +static const float2 f; +float4 main() : sv_target +{ + for (; f;); + return 0; +} + +[pixel shader fail] +static const float f[1]; +float4 main() : sv_target +{ + for (; f;); + return 0; +} + +[pixel shader fail] +static const Texture2D f; +float4 main() : sv_target +{ + for (; f;); + return 0; +} + +[pixel shader todo] +static const float1x1 f; +float4 main() : sv_target +{ + if (f); + for (; f;); + while (f); + do; while (f); + return 0; +} + +[pixel shader] +uniform float1 f; +float4 main() : sv_target +{ + if (f) + return 1; + return 0; +} + +[test] +uniform 0 float4 -2.0 0.0 0.0 0.0 +draw quad +probe all rgba (1.0, 1.0, 1.0, 1.0) +uniform 0 float4 -0.0 0.0 0.0 0.0 +draw quad +probe all rgba (0.0, 0.0, 0.0, 0.0) -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577
Not sure why that test failed here and succeeded on the testbot, but I've just gone ahead and written the patch that fixes it. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577#note_58476
On Mon Jan 22 21:45:07 2024 +0000, Zebediah Figura wrote:
Not sure why that test failed here and succeeded on the testbot, but I've just gone ahead and written the patch that fixes it. I wouldn't put trying to gaslight us past our code or our CI.
-- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577#note_58491
This merge request was approved by Giovanni Mascellani. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577
This merge request was approved by Henri Verbeet. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/577
participants (4)
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)