[PATCH 0/4] MR35: vkd3d-shader/hlsl: Support complex implicit casts, complex explicit casts and complex broadcasts. (PART 2/3 v2)
Second version of !33, taking Zeb's suggestions.
The refactoring allowed to encompass all remaining features of the patch series in these 4 patches.
The remaining patches (PART 3/3) just contain additional tests: https://gitlab.winehq.org/fcasas/vkd3d/-/commits/complex_broadcasts_4/
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/hlsl.y | 110 +++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 55 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 62d86b9d..cc0a40ff 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -153,121 +153,121 @@ static bool convertible_data_type(struct hlsl_type *type) return type->type != HLSL_CLASS_OBJECT; }
-static bool compatible_data_types(struct hlsl_type *t1, struct hlsl_type *t2) +static bool compatible_data_types(struct hlsl_type *src, struct hlsl_type *dst) { - if (!convertible_data_type(t1) || !convertible_data_type(t2)) + if (!convertible_data_type(src) || !convertible_data_type(dst)) return false;
- if (t1->type <= HLSL_CLASS_LAST_NUMERIC) + if (src->type <= HLSL_CLASS_LAST_NUMERIC) { /* Scalar vars can be cast to pretty much everything */ - if (t1->dimx == 1 && t1->dimy == 1) + if (src->dimx == 1 && src->dimy == 1) return true;
- if (t1->type == HLSL_CLASS_VECTOR && t2->type == HLSL_CLASS_VECTOR) - return t1->dimx >= t2->dimx; + if (src->type == HLSL_CLASS_VECTOR && dst->type == HLSL_CLASS_VECTOR) + return src->dimx >= dst->dimx; }
/* The other way around is true too i.e. whatever to scalar */ - if (t2->type <= HLSL_CLASS_LAST_NUMERIC && t2->dimx == 1 && t2->dimy == 1) + if (dst->type <= HLSL_CLASS_LAST_NUMERIC && dst->dimx == 1 && dst->dimy == 1) return true;
- if (t1->type == HLSL_CLASS_ARRAY) + if (src->type == HLSL_CLASS_ARRAY) { - if (hlsl_types_are_equal(t1->e.array.type, t2)) + if (hlsl_types_are_equal(src->e.array.type, dst)) /* e.g. float4[3] to float4 is allowed */ return true;
- if (t2->type == HLSL_CLASS_ARRAY || t2->type == HLSL_CLASS_STRUCT) - return hlsl_type_component_count(t1) >= hlsl_type_component_count(t2); + if (dst->type == HLSL_CLASS_ARRAY || dst->type == HLSL_CLASS_STRUCT) + return hlsl_type_component_count(src) >= hlsl_type_component_count(dst); else - return hlsl_type_component_count(t1) == hlsl_type_component_count(t2); + return hlsl_type_component_count(src) == hlsl_type_component_count(dst); }
- if (t1->type == HLSL_CLASS_STRUCT) - return hlsl_type_component_count(t1) >= hlsl_type_component_count(t2); + if (src->type == HLSL_CLASS_STRUCT) + return hlsl_type_component_count(src) >= hlsl_type_component_count(dst);
- if (t2->type == HLSL_CLASS_ARRAY || t2->type == HLSL_CLASS_STRUCT) - return hlsl_type_component_count(t1) == hlsl_type_component_count(t2); + if (dst->type == HLSL_CLASS_ARRAY || dst->type == HLSL_CLASS_STRUCT) + return hlsl_type_component_count(src) == hlsl_type_component_count(dst);
- if (t1->type == HLSL_CLASS_MATRIX || t2->type == HLSL_CLASS_MATRIX) + if (src->type == HLSL_CLASS_MATRIX || dst->type == HLSL_CLASS_MATRIX) { - if (t1->type == HLSL_CLASS_MATRIX && t2->type == HLSL_CLASS_MATRIX && t1->dimx >= t2->dimx && t1->dimy >= t2->dimy) + if (src->type == HLSL_CLASS_MATRIX && dst->type == HLSL_CLASS_MATRIX && src->dimx >= dst->dimx && src->dimy >= dst->dimy) return true;
/* Matrix-vector conversion is apparently allowed if they have the same components count */ - if ((t1->type == HLSL_CLASS_VECTOR || t2->type == HLSL_CLASS_VECTOR) - && hlsl_type_component_count(t1) == hlsl_type_component_count(t2)) + if ((src->type == HLSL_CLASS_VECTOR || dst->type == HLSL_CLASS_VECTOR) + && hlsl_type_component_count(src) == hlsl_type_component_count(dst)) return true; return false; }
- if (hlsl_type_component_count(t1) >= hlsl_type_component_count(t2)) + if (hlsl_type_component_count(src) >= hlsl_type_component_count(dst)) return true; return false; }
-static bool implicit_compatible_data_types(struct hlsl_type *t1, struct hlsl_type *t2) +static bool implicit_compatible_data_types(struct hlsl_type *src, struct hlsl_type *dst) { - if (!convertible_data_type(t1) || !convertible_data_type(t2)) + if (!convertible_data_type(src) || !convertible_data_type(dst)) return false;
- if (t1->type <= HLSL_CLASS_LAST_NUMERIC) + if (src->type <= HLSL_CLASS_LAST_NUMERIC) { /* Scalar vars can be converted to any other numeric data type */ - if (t1->dimx == 1 && t1->dimy == 1 && t2->type <= HLSL_CLASS_LAST_NUMERIC) + if (src->dimx == 1 && src->dimy == 1 && dst->type <= HLSL_CLASS_LAST_NUMERIC) return true; /* The other way around is true too */ - if (t2->dimx == 1 && t2->dimy == 1 && t2->type <= HLSL_CLASS_LAST_NUMERIC) + if (dst->dimx == 1 && dst->dimy == 1 && dst->type <= HLSL_CLASS_LAST_NUMERIC) return true; }
- if (t1->type == HLSL_CLASS_ARRAY && t2->type == HLSL_CLASS_ARRAY) + if (src->type == HLSL_CLASS_ARRAY && dst->type == HLSL_CLASS_ARRAY) { - return hlsl_type_component_count(t1) == hlsl_type_component_count(t2); + return hlsl_type_component_count(src) == hlsl_type_component_count(dst); }
- if ((t1->type == HLSL_CLASS_ARRAY && t2->type <= HLSL_CLASS_LAST_NUMERIC) - || (t1->type <= HLSL_CLASS_LAST_NUMERIC && t2->type == HLSL_CLASS_ARRAY)) + if ((src->type == HLSL_CLASS_ARRAY && dst->type <= HLSL_CLASS_LAST_NUMERIC) + || (src->type <= HLSL_CLASS_LAST_NUMERIC && dst->type == HLSL_CLASS_ARRAY)) { /* e.g. float4[3] to float4 is allowed */ - if (t1->type == HLSL_CLASS_ARRAY && hlsl_types_are_equal(t1->e.array.type, t2)) + if (src->type == HLSL_CLASS_ARRAY && hlsl_types_are_equal(src->e.array.type, dst)) return true; - if (hlsl_type_component_count(t1) == hlsl_type_component_count(t2)) + if (hlsl_type_component_count(src) == hlsl_type_component_count(dst)) return true; return false; }
- if (t1->type <= HLSL_CLASS_VECTOR && t2->type <= HLSL_CLASS_VECTOR) + if (src->type <= HLSL_CLASS_VECTOR && dst->type <= HLSL_CLASS_VECTOR) { - if (t1->dimx >= t2->dimx) + if (src->dimx >= dst->dimx) return true; return false; }
- if (t1->type == HLSL_CLASS_MATRIX || t2->type == HLSL_CLASS_MATRIX) + if (src->type == HLSL_CLASS_MATRIX || dst->type == HLSL_CLASS_MATRIX) { - if (t1->type == HLSL_CLASS_MATRIX && t2->type == HLSL_CLASS_MATRIX) - return t1->dimx >= t2->dimx && t1->dimy >= t2->dimy; + if (src->type == HLSL_CLASS_MATRIX && dst->type == HLSL_CLASS_MATRIX) + return src->dimx >= dst->dimx && src->dimy >= dst->dimy;
/* Matrix-vector conversion is apparently allowed if they have * the same components count, or if the matrix is 1xN or Nx1 * and we are reducing the component count */ - if (t1->type == HLSL_CLASS_VECTOR || t2->type == HLSL_CLASS_VECTOR) + if (src->type == HLSL_CLASS_VECTOR || dst->type == HLSL_CLASS_VECTOR) { - if (hlsl_type_component_count(t1) == hlsl_type_component_count(t2)) + if (hlsl_type_component_count(src) == hlsl_type_component_count(dst)) return true;
- if ((t1->type == HLSL_CLASS_VECTOR || t1->dimx == 1 || t1->dimy == 1) && - (t2->type == HLSL_CLASS_VECTOR || t2->dimx == 1 || t2->dimy == 1)) - return hlsl_type_component_count(t1) >= hlsl_type_component_count(t2); + if ((src->type == HLSL_CLASS_VECTOR || src->dimx == 1 || src->dimy == 1) && + (dst->type == HLSL_CLASS_VECTOR || dst->dimx == 1 || dst->dimy == 1)) + return hlsl_type_component_count(src) >= hlsl_type_component_count(dst); }
return false; }
- if (t1->type == HLSL_CLASS_STRUCT && t2->type == HLSL_CLASS_STRUCT) - return hlsl_types_are_equal(t1, t2); + if (src->type == HLSL_CLASS_STRUCT && dst->type == HLSL_CLASS_STRUCT) + return hlsl_types_are_equal(src, dst);
return false; } @@ -1167,34 +1167,34 @@ static unsigned int evaluate_array_dimension(struct hlsl_ir_node *node) } }
-static bool expr_compatible_data_types(struct hlsl_type *t1, struct hlsl_type *t2) +static bool expr_compatible_data_types(struct hlsl_type *src, struct hlsl_type *dst) { - if (t1->base_type > HLSL_TYPE_LAST_SCALAR || t2->base_type > HLSL_TYPE_LAST_SCALAR) + if (src->base_type > HLSL_TYPE_LAST_SCALAR || dst->base_type > HLSL_TYPE_LAST_SCALAR) return false;
/* Scalar vars can be converted to pretty much everything */ - if ((t1->dimx == 1 && t1->dimy == 1) || (t2->dimx == 1 && t2->dimy == 1)) + if ((src->dimx == 1 && src->dimy == 1) || (dst->dimx == 1 && dst->dimy == 1)) return true;
- if (t1->type == HLSL_CLASS_VECTOR && t2->type == HLSL_CLASS_VECTOR) + if (src->type == HLSL_CLASS_VECTOR && dst->type == HLSL_CLASS_VECTOR) return true;
- if (t1->type == HLSL_CLASS_MATRIX || t2->type == HLSL_CLASS_MATRIX) + if (src->type == HLSL_CLASS_MATRIX || dst->type == HLSL_CLASS_MATRIX) { /* Matrix-vector conversion is apparently allowed if either they have the same components count or the matrix is nx1 or 1xn */ - if (t1->type == HLSL_CLASS_VECTOR || t2->type == HLSL_CLASS_VECTOR) + if (src->type == HLSL_CLASS_VECTOR || dst->type == HLSL_CLASS_VECTOR) { - if (hlsl_type_component_count(t1) == hlsl_type_component_count(t2)) + if (hlsl_type_component_count(src) == hlsl_type_component_count(dst)) return true;
- return (t1->type == HLSL_CLASS_MATRIX && (t1->dimx == 1 || t1->dimy == 1)) - || (t2->type == HLSL_CLASS_MATRIX && (t2->dimx == 1 || t2->dimy == 1)); + return (src->type == HLSL_CLASS_MATRIX && (src->dimx == 1 || src->dimy == 1)) + || (dst->type == HLSL_CLASS_MATRIX && (dst->dimx == 1 || dst->dimy == 1)); }
/* Both matrices */ - if ((t1->dimx >= t2->dimx && t1->dimy >= t2->dimy) - || (t1->dimx <= t2->dimx && t1->dimy <= t2->dimy)) + if ((src->dimx >= dst->dimx && src->dimy >= dst->dimy) + || (src->dimx <= dst->dimx && src->dimy <= dst->dimy)) return true; }
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/hlsl.y | 93 +++++++++++----------- tests/cast-componentwise-equal.shader_test | 16 ++-- 2 files changed, 54 insertions(+), 55 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index f5f114c8..e14760d8 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -153,6 +153,27 @@ static bool convertible_data_type(struct hlsl_type *type) return type->type != HLSL_CLASS_OBJECT; }
+static bool hlsl_types_are_componentwise_equal(struct hlsl_ctx *ctx, struct hlsl_type *src, + struct hlsl_type *dst) +{ + unsigned int k, count = hlsl_type_component_count(src); + + if (count != hlsl_type_component_count(dst)) + return false; + + for (k = 0; k < count; ++k) + { + struct hlsl_type *src_comp_type, *dst_comp_type; + + src_comp_type = hlsl_type_get_component_type(ctx, src, k); + dst_comp_type = hlsl_type_get_component_type(ctx, dst, k); + + if (!hlsl_types_are_equal(src_comp_type, dst_comp_type)) + return false; + } + return true; +} + static bool compatible_data_types(struct hlsl_type *src, struct hlsl_type *dst) { if (!convertible_data_type(src) || !convertible_data_type(dst)) @@ -207,69 +228,47 @@ static bool compatible_data_types(struct hlsl_type *src, struct hlsl_type *dst) return false; }
-static bool implicit_compatible_data_types(struct hlsl_type *src, struct hlsl_type *dst) +static bool implicit_compatible_data_types(struct hlsl_ctx *ctx, struct hlsl_type *src, struct hlsl_type *dst) { - if (!convertible_data_type(src) || !convertible_data_type(dst)) + if ((src->type <= HLSL_CLASS_LAST_NUMERIC) != (dst->type <= HLSL_CLASS_LAST_NUMERIC)) return false;
if (src->type <= HLSL_CLASS_LAST_NUMERIC) { /* Scalar vars can be converted to any other numeric data type */ - if (src->dimx == 1 && src->dimy == 1 && dst->type <= HLSL_CLASS_LAST_NUMERIC) + if (src->dimx == 1 && src->dimy == 1) return true; /* The other way around is true too */ - if (dst->dimx == 1 && dst->dimy == 1 && dst->type <= HLSL_CLASS_LAST_NUMERIC) + if (dst->dimx == 1 && dst->dimy == 1) return true; - }
- if (src->type == HLSL_CLASS_ARRAY && dst->type == HLSL_CLASS_ARRAY) - { - return hlsl_type_component_count(src) == hlsl_type_component_count(dst); - } - - if ((src->type == HLSL_CLASS_ARRAY && dst->type <= HLSL_CLASS_LAST_NUMERIC) - || (src->type <= HLSL_CLASS_LAST_NUMERIC && dst->type == HLSL_CLASS_ARRAY)) - { - /* e.g. float4[3] to float4 is allowed */ - if (src->type == HLSL_CLASS_ARRAY && hlsl_types_are_equal(src->e.array.type, dst)) - return true; - if (hlsl_type_component_count(src) == hlsl_type_component_count(dst)) - return true; - return false; - } + if (src->type == HLSL_CLASS_MATRIX || dst->type == HLSL_CLASS_MATRIX) + { + if (src->type == HLSL_CLASS_MATRIX && dst->type == HLSL_CLASS_MATRIX) + return src->dimx >= dst->dimx && src->dimy >= dst->dimy;
- if (src->type <= HLSL_CLASS_VECTOR && dst->type <= HLSL_CLASS_VECTOR) - { - if (src->dimx >= dst->dimx) - return true; - return false; - } + /* Matrix-vector conversion is apparently allowed if they have + * the same components count, or if the matrix is 1xN or Nx1 + * and we are reducing the component count */ + if (src->type == HLSL_CLASS_VECTOR || dst->type == HLSL_CLASS_VECTOR) + { + if (hlsl_type_component_count(src) == hlsl_type_component_count(dst)) + return true;
- if (src->type == HLSL_CLASS_MATRIX || dst->type == HLSL_CLASS_MATRIX) - { - if (src->type == HLSL_CLASS_MATRIX && dst->type == HLSL_CLASS_MATRIX) - return src->dimx >= dst->dimx && src->dimy >= dst->dimy; + if ((src->type == HLSL_CLASS_VECTOR || src->dimx == 1 || src->dimy == 1) && + (dst->type == HLSL_CLASS_VECTOR || dst->dimx == 1 || dst->dimy == 1)) + return hlsl_type_component_count(src) >= hlsl_type_component_count(dst); + }
- /* Matrix-vector conversion is apparently allowed if they have - * the same components count, or if the matrix is 1xN or Nx1 - * and we are reducing the component count */ - if (src->type == HLSL_CLASS_VECTOR || dst->type == HLSL_CLASS_VECTOR) + return false; + } + else { - if (hlsl_type_component_count(src) == hlsl_type_component_count(dst)) - return true; - - if ((src->type == HLSL_CLASS_VECTOR || src->dimx == 1 || src->dimy == 1) && - (dst->type == HLSL_CLASS_VECTOR || dst->dimx == 1 || dst->dimy == 1)) - return hlsl_type_component_count(src) >= hlsl_type_component_count(dst); + return src->dimx >= dst->dimx; } - - return false; }
- if (src->type == HLSL_CLASS_STRUCT && dst->type == HLSL_CLASS_STRUCT) - return hlsl_types_are_equal(src, dst); - - return false; + return hlsl_types_are_componentwise_equal(ctx, src, dst); }
static struct hlsl_ir_load *add_load_component(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_node *var_instr, @@ -364,7 +363,7 @@ static struct hlsl_ir_node *add_implicit_conversion(struct hlsl_ctx *ctx, struct if (hlsl_types_are_equal(src_type, dst_type)) return node;
- if (!implicit_compatible_data_types(src_type, dst_type)) + if (!implicit_compatible_data_types(ctx, src_type, dst_type)) { struct vkd3d_string_buffer *src_string, *dst_string;
diff --git a/tests/cast-componentwise-equal.shader_test b/tests/cast-componentwise-equal.shader_test index ac329112..d0775f47 100644 --- a/tests/cast-componentwise-equal.shader_test +++ b/tests/cast-componentwise-equal.shader_test @@ -50,8 +50,8 @@ float4 main() : sv_target
[test] -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 1.0) +draw quad +probe all rgba (1.0, 2.0, 3.0, 1.0)
[pixel shader fail] @@ -89,8 +89,8 @@ float4 main() : sv_target
[test] -todo draw quad -todo probe all rgba (5.0, 6.0, 7.0, 8.0) +draw quad +probe all rgba (5.0, 6.0, 7.0, 8.0)
[pixel shader] @@ -120,8 +120,8 @@ float4 main() : sv_target }
[test] -todo draw quad -todo probe all rgba (4.0, 4.0, 4.0, 4.0) +draw quad +probe all rgba (4.0, 4.0, 4.0, 4.0)
[pixel shader] @@ -199,8 +199,8 @@ float4 main() : sv_target }
[test] -todo draw quad -todo probe all rgba (71.0, 73.0, 73.0, 74.0) +draw quad +probe all rgba (71.0, 73.0, 73.0, 74.0)
[pixel shader fail]
On 10/19/22 22:36, Francisco Casas wrote:
- if ((src->type == HLSL_CLASS_ARRAY && dst->type <= HLSL_CLASS_LAST_NUMERIC)
|| (src->type <= HLSL_CLASS_LAST_NUMERIC && dst->type == HLSL_CLASS_ARRAY))- {
/* e.g. float4[3] to float4 is allowed */if (src->type == HLSL_CLASS_ARRAY && hlsl_types_are_equal(src->e.array.type, dst))return true;if (hlsl_type_component_count(src) == hlsl_type_component_count(dst))return true;return false;- }
This patch removes the "broken" cases I mentioned, which is great, except that it's different from the main and stated purpose of the patch. Ideally that's something we should pull into a separate patch (earlier, or maybe later, whichever makes more sense). We also would ideally have tests for those illegal casts.
On 20-10-22 02:04, Zebediah Figura wrote:
On 10/19/22 22:36, Francisco Casas wrote:
- if ((src->type == HLSL_CLASS_ARRAY && dst->type <= HLSL_CLASS_LAST_NUMERIC) - || (src->type <= HLSL_CLASS_LAST_NUMERIC && dst->type == HLSL_CLASS_ARRAY)) - { - /* e.g. float4[3] to float4 is allowed */ - if (src->type == HLSL_CLASS_ARRAY && hlsl_types_are_equal(src->e.array.type, dst)) - return true; - if (hlsl_type_component_count(src) == hlsl_type_component_count(dst)) - return true; - return false; - }
This patch removes the "broken" cases I mentioned, which is great, except that it's different from the main and stated purpose of the patch. Ideally that's something we should pull into a separate patch (earlier, or maybe later, whichever makes more sense). We also would ideally have tests for those illegal casts.
Okay, I am doing that in a separate patch earlier. Also I am moving the additional implicit cast tests forward and adding these cases specifically.
From: Francisco Casas fcasas@codeweavers.com
This extends the support of this function, whether doing broadcasts or component-wise casts, to struct and array types.
---
In the following patches, compatible_data_types() and implicit_compatible_data_types() are improved so that: * Casts that should be allowed actually reach add_cast(). * Casts that shouldn't be allowed because of special conditions (besides component count), e.g. matrix to matrix, don't reach add_cast(). --- libs/vkd3d-shader/hlsl.y | 45 +++++++++---------- tests/cast-broadcast.shader_test | 4 +- .../cast-componentwise-compatible.shader_test | 20 ++++----- 3 files changed, 33 insertions(+), 36 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index cc0a40ff..f5f114c8 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -284,30 +284,30 @@ static struct hlsl_ir_node *add_cast(struct hlsl_ctx *ctx, struct list *instrs, if (hlsl_types_are_equal(src_type, dst_type)) return node;
- if ((src_type->type == HLSL_CLASS_MATRIX || dst_type->type == HLSL_CLASS_MATRIX) - && src_type->type <= HLSL_CLASS_LAST_NUMERIC && dst_type->type <= HLSL_CLASS_LAST_NUMERIC) + if (src_type->type > HLSL_CLASS_VECTOR || dst_type->type > HLSL_CLASS_VECTOR) { + unsigned int src_comp_count = hlsl_type_component_count(src_type); + unsigned int dst_comp_count = hlsl_type_component_count(dst_type); struct hlsl_deref var_deref; + bool broadcast, matrix_cast; struct hlsl_ir_load *load; struct hlsl_ir_var *var; unsigned int dst_idx; - bool broadcast;
- broadcast = src_type->dimx == 1 && src_type->dimy == 1; - assert(dst_type->dimx * dst_type->dimy <= src_type->dimx * src_type->dimy || broadcast); - if (src_type->type == HLSL_CLASS_MATRIX && dst_type->type == HLSL_CLASS_MATRIX && !broadcast) - { - assert(dst_type->dimx <= src_type->dimx); - assert(dst_type->dimy <= src_type->dimy); - } + broadcast = src_type->type <= HLSL_CLASS_LAST_NUMERIC && src_type->dimx == 1 && src_type->dimy == 1; + matrix_cast = !broadcast && dst_comp_count != src_comp_count + && src_type->type == HLSL_CLASS_MATRIX && dst_type->type == HLSL_CLASS_MATRIX; + assert(src_comp_count >= dst_comp_count || broadcast); + assert(!matrix_cast || dst_type->dimx <= src_type->dimx); + assert(!matrix_cast || dst_type->dimy <= src_type->dimy);
if (!(var = hlsl_new_synthetic_var(ctx, "cast", dst_type, loc))) return NULL; hlsl_init_simple_deref_from_var(&var_deref, var);
- for (dst_idx = 0; dst_idx < dst_type->dimx * dst_type->dimy; ++dst_idx) + for (dst_idx = 0; dst_idx < dst_comp_count; ++dst_idx) { - struct hlsl_type *dst_scalar_type; + struct hlsl_type *dst_comp_type; struct hlsl_ir_store *store; struct hlsl_block block; unsigned int src_idx; @@ -316,26 +316,23 @@ static struct hlsl_ir_node *add_cast(struct hlsl_ctx *ctx, struct list *instrs, { src_idx = 0; } - else + else if (matrix_cast) { - if (src_type->type == HLSL_CLASS_MATRIX && dst_type->type == HLSL_CLASS_MATRIX) - { - unsigned int x = dst_idx % dst_type->dimx, y = dst_idx / dst_type->dimx; + unsigned int x = dst_idx % dst_type->dimx, y = dst_idx / dst_type->dimx;
- src_idx = y * src_type->dimx + x; - } - else - { - src_idx = dst_idx; - } + src_idx = y * src_type->dimx + x; + } + else + { + src_idx = dst_idx; }
- dst_scalar_type = hlsl_type_get_component_type(ctx, dst_type, dst_idx); + dst_comp_type = hlsl_type_get_component_type(ctx, dst_type, dst_idx);
if (!(load = add_load_component(ctx, instrs, node, src_idx, loc))) return NULL;
- if (!(cast = hlsl_new_cast(ctx, &load->node, dst_scalar_type, loc))) + if (!(cast = hlsl_new_cast(ctx, &load->node, dst_comp_type, loc))) return NULL; list_add_tail(instrs, &cast->node.entry);
diff --git a/tests/cast-broadcast.shader_test b/tests/cast-broadcast.shader_test index e21e65b0..d8571fc8 100644 --- a/tests/cast-broadcast.shader_test +++ b/tests/cast-broadcast.shader_test @@ -19,8 +19,8 @@ float4 main() : SV_TARGET }
[test] -todo draw quad -todo probe all rgba (84.0, 84.0, 84.0, 84.0) +draw quad +probe all rgba (84.0, 84.0, 84.0, 84.0)
[pixel shader fail] diff --git a/tests/cast-componentwise-compatible.shader_test b/tests/cast-componentwise-compatible.shader_test index c6402cf5..286c180b 100644 --- a/tests/cast-componentwise-compatible.shader_test +++ b/tests/cast-componentwise-compatible.shader_test @@ -16,8 +16,8 @@ float4 main() : sv_target
[test] -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 1.0) +draw quad +probe all rgba (1.0, 2.0, 3.0, 1.0)
[pixel shader] @@ -39,8 +39,8 @@ float4 main() : sv_target
[test] -todo draw quad -todo probe all rgba (5.0, 6.0, 7.0, 8.0) +draw quad +probe all rgba (5.0, 6.0, 7.0, 8.0)
[pixel shader] @@ -55,8 +55,8 @@ float4 main() : sv_target
[test] -todo draw quad -todo probe all rgba (1.0, 2.0, 3.0, 4.0) +draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0)
[pixel shader] @@ -86,8 +86,8 @@ float4 main() : sv_target }
[test] -todo draw quad -todo probe all rgba (7.0, 7.0, 7.0, 7.0) +draw quad +probe all rgba (7.0, 7.0, 7.0, 7.0)
[pixel shader] @@ -119,8 +119,8 @@ float4 main() : sv_target }
[test] -todo draw quad -todo probe all rgba (3.0, 3.0, 3.0, 3.0) +draw quad +probe all rgba (3.0, 3.0, 3.0, 3.0)
[pixel shader fail]
On 10/19/22 22:36, Francisco Casas wrote:
In the following patches, compatible_data_types() and implicit_compatible_data_types() are improved so that:
- Casts that should be allowed actually reach add_cast().
- Casts that shouldn't be allowed because of special conditions (besides component count), e.g. matrix to matrix, don't reach add_cast().
Not an issue with these patches (nor do I think it should change anything about how they're implemented), but for what it's worth, we probably should let even the invalid casts "reach" add_cast(). Or, at least, they shouldn't abort compilation; maybe we can achieve that in some other way.
This is a more general problem we have—we should try to avoid aborting compilation where possible, so that multiple errors can be reported in a single pass.
matrix_cast = !broadcast && dst_comp_count != src_comp_count&& src_type->type == HLSL_CLASS_MATRIX && dst_type->type == HLSL_CLASS_MATRIX;assert(src_comp_count >= dst_comp_count || broadcast);assert(!matrix_cast || dst_type->dimx <= src_type->dimx);assert(!matrix_cast || dst_type->dimy <= src_type->dimy);
Nitpicking, I think this is a bit easier to read as
if (matrix_cast) { assert(dst_type->dimx <= src_type->dimx); assert(dst_type->dimy <= src_type->dimy); }
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/hlsl.y | 84 +++++++++---------- .../cast-componentwise-compatible.shader_test | 8 +- 2 files changed, 46 insertions(+), 46 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index e14760d8..beaa58f8 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -148,9 +148,26 @@ static void check_invalid_matrix_modifiers(struct hlsl_ctx *ctx, DWORD modifiers "'row_major' and 'column_major' modifiers are only allowed for matrices."); }
-static bool convertible_data_type(struct hlsl_type *type) +static bool hlsl_types_are_componentwise_compatible(struct hlsl_ctx *ctx, struct hlsl_type *src, + struct hlsl_type *dst) { - return type->type != HLSL_CLASS_OBJECT; + unsigned int k, count = hlsl_type_component_count(dst); + + if (count > hlsl_type_component_count(src)) + return false; + + for (k = 0; k < count; ++k) + { + struct hlsl_type *src_comp_type, *dst_comp_type; + + src_comp_type = hlsl_type_get_component_type(ctx, src, k); + dst_comp_type = hlsl_type_get_component_type(ctx, dst, k); + + if ((src_comp_type->type != HLSL_CLASS_SCALAR || dst_comp_type->type != HLSL_CLASS_SCALAR) + && !hlsl_types_are_equal(src_comp_type, dst_comp_type)) + return false; + } + return true; }
static bool hlsl_types_are_componentwise_equal(struct hlsl_ctx *ctx, struct hlsl_type *src, @@ -174,58 +191,41 @@ static bool hlsl_types_are_componentwise_equal(struct hlsl_ctx *ctx, struct hlsl return true; }
-static bool compatible_data_types(struct hlsl_type *src, struct hlsl_type *dst) +static bool type_contains_only_numerics(struct hlsl_type *type) { - if (!convertible_data_type(src) || !convertible_data_type(dst)) - return false; + unsigned int i;
- if (src->type <= HLSL_CLASS_LAST_NUMERIC) + if (type->type == HLSL_CLASS_ARRAY) + return type_contains_only_numerics(type->e.array.type); + if (type->type == HLSL_CLASS_STRUCT) { - /* Scalar vars can be cast to pretty much everything */ - if (src->dimx == 1 && src->dimy == 1) - return true; - - if (src->type == HLSL_CLASS_VECTOR && dst->type == HLSL_CLASS_VECTOR) - return src->dimx >= dst->dimx; - } - - /* The other way around is true too i.e. whatever to scalar */ - if (dst->type <= HLSL_CLASS_LAST_NUMERIC && dst->dimx == 1 && dst->dimy == 1) + for (i = 0; i < type->e.record.field_count; ++i) + { + if (!type_contains_only_numerics(type->e.record.fields[i].type)) + return false; + } return true; - - if (src->type == HLSL_CLASS_ARRAY) - { - if (hlsl_types_are_equal(src->e.array.type, dst)) - /* e.g. float4[3] to float4 is allowed */ - return true; - - if (dst->type == HLSL_CLASS_ARRAY || dst->type == HLSL_CLASS_STRUCT) - return hlsl_type_component_count(src) >= hlsl_type_component_count(dst); - else - return hlsl_type_component_count(src) == hlsl_type_component_count(dst); } + return type->type <= HLSL_CLASS_LAST_NUMERIC; +}
- if (src->type == HLSL_CLASS_STRUCT) - return hlsl_type_component_count(src) >= hlsl_type_component_count(dst); - - if (dst->type == HLSL_CLASS_ARRAY || dst->type == HLSL_CLASS_STRUCT) - return hlsl_type_component_count(src) == hlsl_type_component_count(dst); +static bool compatible_data_types(struct hlsl_ctx *ctx, struct hlsl_type *src, struct hlsl_type *dst) +{ + if (src->type <= HLSL_CLASS_LAST_NUMERIC && src->dimx == 1 && dst->dimy == 1 && type_contains_only_numerics(dst)) + return true;
if (src->type == HLSL_CLASS_MATRIX || dst->type == HLSL_CLASS_MATRIX) { - if (src->type == HLSL_CLASS_MATRIX && dst->type == HLSL_CLASS_MATRIX && src->dimx >= dst->dimx && src->dimy >= dst->dimy) + if (src->type == HLSL_CLASS_MATRIX && dst->type == HLSL_CLASS_MATRIX + && src->dimx >= dst->dimx && src->dimy >= dst->dimy) return true;
- /* Matrix-vector conversion is apparently allowed if they have the same components count */ - if ((src->type == HLSL_CLASS_VECTOR || dst->type == HLSL_CLASS_VECTOR) - && hlsl_type_component_count(src) == hlsl_type_component_count(dst)) - return true; - return false; + if ((src->dimy > 1 || dst->dimy > 1) && + hlsl_type_component_count(src) != hlsl_type_component_count(dst)) + return false; }
- if (hlsl_type_component_count(src) >= hlsl_type_component_count(dst)) - return true; - return false; + return hlsl_types_are_componentwise_compatible(ctx, src, dst); }
static bool implicit_compatible_data_types(struct hlsl_ctx *ctx, struct hlsl_type *src, struct hlsl_type *dst) @@ -4447,7 +4447,7 @@ unary_expr: dst_type = hlsl_new_array_type(ctx, dst_type, $4.sizes[i]); }
- if (!compatible_data_types(src_type, dst_type)) + if (!compatible_data_types(ctx, src_type, dst_type)) { struct vkd3d_string_buffer *src_string, *dst_string;
diff --git a/tests/cast-componentwise-compatible.shader_test b/tests/cast-componentwise-compatible.shader_test index 286c180b..4b342efb 100644 --- a/tests/cast-componentwise-compatible.shader_test +++ b/tests/cast-componentwise-compatible.shader_test @@ -159,8 +159,8 @@ float4 main() : sv_target
[test] -todo draw quad -todo probe all rgba (10.0, 20.0, 30.0, 30.0) +draw quad +probe all rgba (10.0, 20.0, 30.0, 30.0)
[pixel shader] @@ -181,5 +181,5 @@ float4 main() : sv_target
[test] -todo draw quad -todo probe all rgba (10.0, 10.0, 11.4, 12.4) +draw quad +probe all rgba (10.0, 10.0, 11.4, 12.4)
On 10/19/22 22:36, Francisco Casas wrote:
+static bool compatible_data_types(struct hlsl_ctx *ctx, struct hlsl_type *src, struct hlsl_type *dst) +{
- if (src->type <= HLSL_CLASS_LAST_NUMERIC && src->dimx == 1 && dst->dimy == 1 && type_contains_only_numerics(dst))
return true;
That looks like a copy-paste error; it should be "src->dimy == 1"? Kinda surprised that didn't cause any test failures...
On 20-10-22 02:04, Zebediah Figura wrote:
On 10/19/22 22:36, Francisco Casas wrote:
+static bool compatible_data_types(struct hlsl_ctx *ctx, struct hlsl_type *src, struct hlsl_type *dst) +{ + if (src->type <= HLSL_CLASS_LAST_NUMERIC && src->dimx == 1 && dst->dimy == 1 && type_contains_only_numerics(dst)) + return true;
That looks like a copy-paste error; it should be "src->dimy == 1"? Kinda surprised that didn't cause any test failures...
It indeed was! And after I fixed it, some of the tests in the following patches, specifically [pixel shader fail]s involving matrices, were not passing.
So this was one of the weird cases where two bugs even each other out, *sigh*.
I am changing compatible_data_types() in the next version to ensure that these shaders indeed fail.
The commit message for d51c8690cfc009ea5febc600fa62a870653b967d has a newline in the subject, which is probably better avoided. If you are concerned about overlong lines, notice that you can save a few strokes rewriting as "Support implicit casts between component-wise equal types".
-
Francisco Casas -
Francisco Casas -
Francisco Casas (＠fcasas)
-
Giovanni Mascellani (＠giomasce)
-
Zebediah Figura