Hi, I think the patch generally looks good, but there are a few details to fix. Il 28/04/22 15:32, Giovanni Mascellani ha scritto:
@@ -328,6 +330,9 @@ struct hlsl_type *hlsl_new_array_type(struct hlsl_ctx *ctx, struct hlsl_type *ba
list_add_tail(&ctx->types, &type->entry);
+ if (type->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + TRACE("Implicit element count.\n"); +
I don't understand what this should be useful for, is it perhaps a leftover of development?
return type; }
@@ -410,6 +415,7 @@ unsigned int hlsl_type_component_count(struct hlsl_type *type) } if (type->type == HLSL_CLASS_ARRAY) { + assert(type->e.array.elements_count != HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT); return hlsl_type_component_count(type->e.array.type) * type->e.array.elements_count; } if (type->type != HLSL_CLASS_STRUCT) @@ -1023,7 +1029,12 @@ struct vkd3d_string_buffer *hlsl_type_to_string(struct hlsl_ctx *ctx, const stru }
for (t = type; t->type == HLSL_CLASS_ARRAY; t = t->e.array.type) - vkd3d_string_buffer_printf(string, "[%u]", t->e.array.elements_count); + { + if (t->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + vkd3d_string_buffer_printf(string, "[<undefined>]"); + else + vkd3d_string_buffer_printf(string, "[%u]", t->e.array.elements_count); + }
I guess elements_count should never be IMPLICIT here, should it? As soon as the hunk in declare_vars() is done, all the counts are known. At any rate, if it can happen that the IMPLICIT branch can be taken here, then I'd just use "[]" instead of "[<undefined>]", which is the standard C way.
return string; }
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 28b2ff1b..94597999 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -227,6 +227,8 @@ struct hlsl_src
#define HLSL_MODIFIERS_MAJORITY_MASK (HLSL_MODIFIER_ROW_MAJOR | HLSL_MODIFIER_COLUMN_MAJOR)
+#define HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT 0xffffffff + struct hlsl_reg_reservation { char type; diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 905dbfc5..e7fe74d8 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1606,7 +1606,27 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t
type = basic_type; for (i = 0; i < v->arrays.count; ++i) + { + if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + { + unsigned int size = initializer_size(&v->initializer); + unsigned int elem_components = hlsl_type_component_count(type); + + assert(v->initializer.args_count);
Maybe it's just me, but here I would also assert that i == v->arrays.count - 1. This should already be forced by the parser, but it's a useful code documentation anyway.
+ v->arrays.sizes[i] = (size + elem_components - 1)/elem_components;
Why "+ elem_components - 1", given that immediately below we error out if size is not a multiple of elem_components? Also, notice that elem_components could be zero. You should handle that gracefully.
+ + if (size % elem_components != 0) + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, + "Cannot initialize implicit array with %u components, expected a multiple of %u.", + size, elem_components); + free_parse_initializer(&v->initializer); + v->initializer.args_count = 0; + } + } type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]); + } vkd3d_free(v->arrays.sizes);
Thanks, Giovanni.