Hello, April 22, 2022 6:51 AM, "Giovanni Mascellani" <gmascellani(a)codeweavers.com> wrote:
Hi,
I am not completely in love with the additional runtime complexity of this solution, but it's true that it's quite clean.
I have a couple of ideas for lowering the complexity: a) Add a "component_count" field to hlsl_type in order to store/cache the result of hlsl_type_component_count(). Which would also speed up other parts of the compiler. b) Add a "comp_offsets" array to "hlsl_type.e" so that the right field that has a given component can be selected in hlsl_compute_component_reg_offset() using a binary search. I can make new patches for these.
Il 22/04/22 12:25, Giovanni Mascellani ha scritto:
+/* Returns the register offset of a given component within a type, given its index. + * *comp_type, will be set to the type of the component, unless it is NULL. */ +unsigned int hlsl_compute_component_reg_offset(struct hlsl_ctx *ctx, struct hlsl_type *type, + unsigned int idx, struct hlsl_type **comp_type, const struct vkd3d_shader_location *loc)
Is the "loc" parameter actually used for something, except being passed around?
I see, it is just being passed around here. I will remove it.
Also, personally I'd just write "offset" rather than "reg_offset". It seems to be clear enough. This applies also to the variables below.
+{ + switch (type->type) + { + case HLSL_CLASS_SCALAR: + case HLSL_CLASS_VECTOR: + { + assert(idx < type->dimx * type->dimy); + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type);
I think you can assume that comp_type will always be valid and skip this check.
I think hlsl_compute_component_reg_offset() can be used to simplify other functions, like compute_offset_from_index(), and in those cases it could be much cleaner to call it passing a NULL comp_type.
+ return idx; + } + case HLSL_CLASS_MATRIX: + { + unsigned int minor, major, x = idx % type->dimx, y = idx / type->dimx; + + assert(idx < type->dimx * type->dimy); + + if (hlsl_type_is_row_major(type)) + { + minor = x; + major = y; + } + else + { + minor = y; + major = x; + } + + if (comp_type) + *comp_type = hlsl_get_scalar_type(ctx, type->base_type); + return 4 * major + minor; + } + + case HLSL_CLASS_ARRAY: + { + unsigned int elem_comp_count = hlsl_type_component_count(type->e.array.type); + unsigned int array_idx = idx / elem_comp_count; + unsigned int idx_in_elem = idx - array_idx * elem_comp_count;
BTW, here you are just computing a modulus, you could directlyuse "%". Addressing arrays is not fundamentally different than addressing matrices, except you don't have to decide which one is major and which one is minor.
Okay, I will write it as unsigned int idx_in_elem = idx % elem_comp_count;
+ assert(array_idx < type->e.array.elements_count); + + return array_idx * hlsl_type_get_array_element_reg_size(type->e.array.type) + + hlsl_compute_component_reg_offset(ctx, type->e.array.type, idx_in_elem, comp_type, loc); + } + + case HLSL_CLASS_STRUCT: + { + struct hlsl_struct_field *field; + + LIST_FOR_EACH_ENTRY(field, type->e.elements, struct hlsl_struct_field, entry) + { + unsigned int elem_comp_count = hlsl_type_component_count(field->type); + + if (idx < elem_comp_count) + { + return field->reg_offset + + hlsl_compute_component_reg_offset(ctx, field->type, idx, comp_type, loc); + } + idx -= elem_comp_count; + } + + assert(0); + return 0; + } + + case HLSL_CLASS_OBJECT: + { + assert(idx == 0); + if (comp_type) + *comp_type = type; + return 0; + } + } + + assert(0); + return 0; +} +
Thanks, Giovanni.