-- 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().
From: Zebediah Figura zfigura@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;
From: Zebediah Figura zfigura@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); }
From: Zebediah Figura zfigura@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;
From: Zebediah Figura zfigura@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); }
From: Zebediah Figura zfigura@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)
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.
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.
This merge request was approved by Giovanni Mascellani.
This merge request was approved by Henri Verbeet.
-
Giovanni Mascellani (＠giomasce) -
Henri Verbeet (＠hverbeet)
-
Zebediah Figura
-
Zebediah Figura (＠zfigura)