Re: [PATCH vkd3d 07/12] vkd3d-shader/hlsl: Support matrix indexing.

[Matteo Bruni]

On Mon, Apr 18, 2022 at 8:34 AM Giovanni Mascellani
 wrote:
> Signed-off-by: Giovanni Mascellani
>
> libs/vkd3d-shader/hlsl.y               | 87 +++++++++++++++++++++++++-
>  tests/hlsl-matrix-indexing.shader_test |  8 +--
>  2 files changed, 88 insertions(+), 7 deletions(-)
>
> diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
> index 291f8392..b6a8e496 100644
> --- a/libs/vkd3d-shader/hlsl.y
> +++ b/libs/vkd3d-shader/hlsl.y
> @@ -581,6 +581,89 @@ static bool add_record_load(struct hlsl_ctx *ctx, struct list *instrs, struct hl
>      return !!add_load(ctx, instrs, record, &c->node, field->type, loc);
>  }
>
> +static struct hlsl_ir_expr *add_binary_arithmetic_expr(struct hlsl_ctx *ctx, struct list *instrs,
> -
>    enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg1, struct hlsl_ir_node *arg2,
> -
>    const struct vkd3d_shader_location *loc);
> -
>
> +static struct hlsl_ir_node *compute_matrix_offset(struct hlsl_ctx *ctx, struct list *instrs,
> -
>    struct hlsl_type *type, struct hlsl_ir_node *x, struct hlsl_ir_node *y,
> -
>    const struct vkd3d_shader_location *loc)
>
> +{
> - struct hlsl_ir_node *major, *minor;
> - struct hlsl_ir_expr *mul, *add;
> - struct hlsl_ir_constant *four;
> -
> - if (type->modifiers & HLSL_MODIFIER_ROW_MAJOR)
> - {
> -
>    minor = x;
> -
>    major = y;
> - }
> - else
> - {
> -
>    minor = y;
> -
>    major = x;
> - }
> -
> - if (!(four = hlsl_new_uint_constant(ctx, 4, loc)))
> -
>    return NULL;
> - list_add_tail(instrs, &four->node.entry);
> -
> - if (!(mul = add_binary_arithmetic_expr(ctx, instrs, HLSL_OP2_MUL, &four->node, major, loc)))
> -
>    return NULL;
> -
> - if (!(add = add_binary_arithmetic_expr(ctx, instrs, HLSL_OP2_ADD, &mul->node, minor, loc)))
> -
>    return NULL;
> -
> - return &add->node;
>
> +}
> -
>
> +static bool add_matrix_load(struct hlsl_ctx *ctx, struct list *instrs,
> -
>    struct hlsl_ir_node *matrix, struct hlsl_ir_node *index, const struct vkd3d_shader_location *loc)
>
> +{
> - struct hlsl_type *mat_type = matrix->data_type, *ret_type, *scal_type;
> - static unsigned int counter = 0;
> - struct hlsl_ir_load *load;
> - struct hlsl_ir_var *var;
> - unsigned int i;
> - char name[32];
> -
> - ret_type = hlsl_get_vector_type(ctx, mat_type->base_type, mat_type->dimx);
> - scal_type = hlsl_get_scalar_type(ctx, mat_type->base_type);

Nitpick, might as well name it scalar_type.
> -
> - sprintf(name, "<index-%x>", counter++);

I know that char name[32] is certainly going to be enough here, but
I'd prefer if we always used vkd3d_string_buffer for this kind of
temporary strings, going forward.