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>]");elsevkd3d_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.