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

[Francisco Casas]

Hello

February 16, 2022 2:14 PM, "Giovanni Mascellani" <gmascellani at codeweavers.com> wrote:

> Hi,
> 
> the patch looks mostly ok, with a few comments.
> 
> Il 15/02/22 21:17, Francisco Casas ha scritto:
> 
>> +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)
>> +{
> 
> Isn't "type" here the same thing as "var->data_type" in the only call to this function? If so, you
> could avoid passing "type" explicitly.
> 

For now it is the same thing but 4 patches ahead (in the continuation of this batch) that will no
longer be true.
The idea is that var may also be a struct or an array, and type would be the type of the numeric 
field inside that we want to initialize. 

>> + 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_is_single_reg(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;
>> + }
> 
> Again under the assumption that "type == var->data_type", here it seems that you're handling the
> matrix case, while it is marked as fixme above. I guess the reason for the fixme is that you're
> only iterating on dimx? Still, I find it a bit confusing. If you consider the matrix case not (yet)
> supported, I'd just avoid to add pieces of it until you're really supporting it. Though maybe I
> missing something.
> 

As before, "type == var->data_type" won't hold after 

33e27ff7 vkd3d-shader/hlsl: Use generic variable initializer for struct fields.

which would have been the 7th patch of this batch if I hadn't split it.
So it is not just for the matrix case.
(I implemented matrix initializers too but they are 10 initializer patches ahead.)


Best regards,
Francisco