Re: [PATCH vkd3d v2 3/6] vkd3d-shader/hlsl: Support complex numeric initializers.

[Zebediah Figura (she/her)]

On 2/22/22 16:13, Francisco Casas wrote:
> +static void initialize_numeric_var(struct hlsl_ctx *ctx, struct hlsl_ir_var *var,
> -
>    struct parse_initializer *initializer, unsigned int reg_offset, struct hlsl_type *type,
> -
>    unsigned int *initializer_offset)
>
> +{
> - unsigned int i;
> -
> - if (type->type == HLSL_CLASS_MATRIX)
> -
>    hlsl_fixme(ctx, &var->loc, "Matrix initializer.");
> -
> - for (i = 0; i < type->dimx; i++)
> - {
> -
>    struct hlsl_ir_store *store;
> -
>    struct hlsl_ir_constant *c;
> -
>    struct hlsl_ir_node *node;
> -
> -
>    node = initializer->args[*initializer_offset];
> -
>    *initializer_offset += 1;
> -
> -
>    if (!(node = add_implicit_conversion(ctx, initializer->instrs, node,
> -
>            hlsl_get_scalar_type(ctx, type->base_type), &node->loc)))
> -
>        return;
> -
> -
>    if (hlsl_type_uses_writemask(var->data_type))
> -
>    {
> -
>        if (!(c = hlsl_new_uint_constant(ctx, reg_offset, node->loc)))
> -
>            return;
> -
>        list_add_tail(initializer->instrs, &c->node.entry);
> -
> -
>        if (!(store = hlsl_new_store(ctx, var, &c->node, node, (1u << i), node->loc)))
> -
>            return;
> -
>    }
> -
>    else
> -
>    {
> -
>        if (!(c = hlsl_new_uint_constant(ctx, reg_offset + i, node->loc)))
> -
>            return;
> -
>        list_add_tail(initializer->instrs, &c->node.entry);
> -
> -
>        if (!(store = hlsl_new_store(ctx, var, &c->node, node, 0, node->loc)))
> -
>            return;
> -
>    }

I'm inclined to say we don't want to be concerned with the writemask 
here. That is, we should just deal in offsets in hlsl.y, in a unified 
manner, and let a later pass lower that to a swizzle or writemask. That 
way we don't have to care in add_array_load() either.

I.e. this should basically just look like the "else" branch.
> -
> -
>    list_add_tail(initializer->instrs, &store->node.entry);
> - }
>
> +}
> - static void struct_var_initializer(struct hlsl_ctx *ctx, struct list *list, struct hlsl_ir_var *var,
>         struct parse_initializer *initializer)
> {
>
> @@ -1490,20 +1535,16 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t
>           if (v->initializer.args_count)
>           {
>               unsigned int size = initializer_size(&v->initializer);
> -
>        struct hlsl_ir_load *load;
>
> -
>        if (type->type <= HLSL_CLASS_LAST_NUMERIC
> -
>                && type->dimx * type->dimy != size && size != 1)
> -
>        if (type->type <= HLSL_CLASS_LAST_NUMERIC && v->initializer.args_count != 1
> -
>                && type->dimx * type->dimy != size)
>         {
> -
>            if (size < type->dimx * type->dimy)
> -
>            {
> -
>                hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT,
> -
>                        "Expected %u components in numeric initializer, but got %u.",
> -
>                        type->dimx * type->dimy, size);
> -
>                free_parse_initializer(&v->initializer);
> -
>                vkd3d_free(v);
> -
>                continue;
> -
>            }
> -
>            hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT,
> -
>                    "Expected %u components in numeric initializer, but got %u.",
> -
>                    type->dimx * type->dimy, v->initializer.args_count);
> -
>            free_parse_initializer(&v->initializer);
> -
>            vkd3d_free(v);
> -
>            continue;
>         }
>         if ((type->type == HLSL_CLASS_STRUCT || type->type == HLSL_CLASS_ARRAY)
>                 && hlsl_type_component_count(type) != size)

This seems like a separate change, no?

Changing "size != 1" to "v->initializer.args_count != 1" seems like yet 
another separate change. (Which I guess is technically more correct, 
although still not perfectly fixed yet...)