On 4/28/22 08:32, Giovanni Mascellani wrote:
@@ -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");return type;
When can this currently happen?
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);v->arrays.sizes[i] = (size + elem_components - 1)/elem_components;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;}
This doesn't seem like it'll do the right thing for implicit sizes on an inner array, especially considering that the right thing is probably to fail compilation.
It also won't do the right thing for initializers without braces (viz. also fail compilation).
I think we need tests for all of these corner cases, and also a test for missing initializers.
} type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]);} vkd3d_free(v->arrays.sizes); if (type->type != HLSL_CLASS_MATRIX)@@ -2464,6 +2484,7 @@ static bool add_method_call(struct hlsl_ctx *ctx, struct list *instrs, struct hl %token <name> TYPE_IDENTIFIER
%type <arrays> arrays +%type <arrays> implicit_arrays
%type <assign_op> assign_op
@@ -3108,7 +3129,7 @@ variables_def: }
variable_decl:
any_identifier arrays colon_attribute
any_identifier implicit_arrays colon_attribute { $$ = hlsl_alloc(ctx, sizeof(*$$)); $$->loc = @1;@@ -3137,6 +3158,15 @@ state_block:
variable_def: variable_decl
{if ($$->arrays.sizes && $$->arrays.sizes[$$->arrays.count - 1] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT){hlsl_error(ctx, &@1, VKD3D_SHADER_ERROR_HLSL_MISSING_INITIALIZER,"Implicit array requires initializer.");free_parse_variable_def($$);YYABORT;}}
This won't work for unbounded resource arrays, which can be declared with an empty pair of brackets (and no initializer).
It also aborts compilation somewhat unnecessarily.
(As a side note, perhaps we should use zero instead of UINT_MAX. Unbounded resources in shader model 5.1 are encoded as zero in the reflection data, whereas declaring a resource array as e.g. "Texture2D t[0xffffffff]" yields 0xffffffff instead, although the shader bytecode is identical.)
As a special extra, this code is apparently valid, and I think deserves to be a test case:
struct apple { Texture2D t[]; };
uniform struct apple a;
float4 main() : sv_target { return a.t[0].Load(0); }
[We don't currently have support for unbounded arrays in the shader_runner infrastructure, though, so it doesn't urgently need to be a test case. Alternatively it could be written in C.]
| variable_decl '=' complex_initializer { $$ = $1;@@ -3188,6 +3218,24 @@ arrays: $$.sizes[$$.count++] = size; }
+implicit_arrays:
arrays- | '[' ']' arrays
{uint32_t *new_array;$$ = $3;if (!(new_array = hlsl_realloc(ctx, $$.sizes, ($$.count + 1) * sizeof(*new_array)))){vkd3d_free($$.sizes);YYABORT;}$$.sizes = new_array;$$.sizes[$$.count++] = HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT;}
I think this should be part of the "arrays" rule, actually. Besides being simpler YACC, it allows us to generate more helpful messages than "syntax error" if someone tries to use an empty array size in an invalid context.