[PATCH 2/5] d3dcompiler: Add the node to the instruction list in new_expr().

[Matteo Bruni]

On Tue, Jun 23, 2020 at 12:48 AM Zebediah Figura <z.figura12 at gmail.com> wrote:
>
> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> ---
>  dlls/d3dcompiler_43/d3dcompiler_private.h | 27 ++++++-----------
>  dlls/d3dcompiler_43/hlsl.y                | 36 ++++++++++++++++++++---
>  dlls/d3dcompiler_43/utils.c               |  5 ++--
>  3 files changed, 44 insertions(+), 24 deletions(-)
>
> diff --git a/dlls/d3dcompiler_43/d3dcompiler_private.h b/dlls/d3dcompiler_43/d3dcompiler_private.h
> index 97b84c44945..50457b2e64e 100644
> --- a/dlls/d3dcompiler_43/d3dcompiler_private.h
> +++ b/dlls/d3dcompiler_43/d3dcompiler_private.h
> @@ -1059,9 +1059,18 @@ static inline void init_node(struct hlsl_ir_node *node, enum hlsl_ir_node_type t
>
>  struct hlsl_ir_node *add_assignment(struct list *instrs, struct hlsl_ir_node *lhs,
>          enum parse_assign_op assign_op, struct hlsl_ir_node *rhs) DECLSPEC_HIDDEN;
> +struct hlsl_ir_expr *add_expr(struct list *instrs, enum hlsl_ir_expr_op op, struct hlsl_ir_node *operands[3],
> +        struct source_location *loc) DECLSPEC_HIDDEN;
>  struct hlsl_ir_node *add_implicit_conversion(struct list *instrs, struct hlsl_ir_node *node, struct hlsl_type *type,
>          struct source_location *loc) DECLSPEC_HIDDEN;
>
> +struct hlsl_ir_expr *new_cast(struct hlsl_ir_node *node, struct hlsl_type *type,
> +        struct source_location *loc) DECLSPEC_HIDDEN;
> +struct hlsl_ir_node *new_binary_expr(enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg1,
> +        struct hlsl_ir_node *arg2) DECLSPEC_HIDDEN;
> +struct hlsl_ir_node *new_unary_expr(enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg,
> +        struct source_location loc) DECLSPEC_HIDDEN;
> +
>  BOOL add_declaration(struct hlsl_scope *scope, struct hlsl_ir_var *decl, BOOL local_var) DECLSPEC_HIDDEN;
>  struct hlsl_ir_var *get_variable(struct hlsl_scope *scope, const char *name) DECLSPEC_HIDDEN;
>  void free_declaration(struct hlsl_ir_var *decl) DECLSPEC_HIDDEN;
> @@ -1075,10 +1084,6 @@ BOOL find_function(const char *name) DECLSPEC_HIDDEN;
>  unsigned int components_count_type(struct hlsl_type *type) DECLSPEC_HIDDEN;
>  BOOL compare_hlsl_types(const struct hlsl_type *t1, const struct hlsl_type *t2) DECLSPEC_HIDDEN;
>  BOOL compatible_data_types(struct hlsl_type *s1, struct hlsl_type *s2) DECLSPEC_HIDDEN;
> -struct hlsl_ir_expr *new_expr(enum hlsl_ir_expr_op op, struct hlsl_ir_node **operands,
> -        struct source_location *loc) DECLSPEC_HIDDEN;
> -struct hlsl_ir_expr *new_cast(struct hlsl_ir_node *node, struct hlsl_type *type,
> -       struct source_location *loc) DECLSPEC_HIDDEN;
>  void push_scope(struct hlsl_parse_ctx *ctx) DECLSPEC_HIDDEN;
>  BOOL pop_scope(struct hlsl_parse_ctx *ctx) DECLSPEC_HIDDEN;
>  void init_functions_tree(struct wine_rb_tree *funcs) DECLSPEC_HIDDEN;
> @@ -1098,20 +1103,6 @@ void free_instr(struct hlsl_ir_node *node) DECLSPEC_HIDDEN;
>  void free_instr_list(struct list *list) DECLSPEC_HIDDEN;
>  void free_function_rb(struct wine_rb_entry *entry, void *context) DECLSPEC_HIDDEN;
>
> -static inline struct hlsl_ir_node *new_unary_expr(enum hlsl_ir_expr_op op,
> -        struct hlsl_ir_node *op1, struct source_location loc)
> -{
> -    struct hlsl_ir_node *operands[3] = {op1};
> -    return &new_expr(op, operands, &loc)->node;
> -}
> -
> -static inline struct hlsl_ir_node *new_binary_expr(enum hlsl_ir_expr_op op,
> -        struct hlsl_ir_node *op1, struct hlsl_ir_node *op2, struct source_location loc)
> -{
> -    struct hlsl_ir_node *operands[3] = {op1, op2};
> -    return &new_expr(op, operands, &loc)->node;
> -}
> -
>  #define MAKE_TAG(ch0, ch1, ch2, ch3) \
>      ((DWORD)(ch0) | ((DWORD)(ch1) << 8) | \
>      ((DWORD)(ch2) << 16) | ((DWORD)(ch3) << 24 ))
> diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y
> index 43ffa43a02a..4e43a802750 100644
> --- a/dlls/d3dcompiler_43/hlsl.y
> +++ b/dlls/d3dcompiler_43/hlsl.y
> @@ -608,6 +608,34 @@ static struct hlsl_ir_constant *new_uint_constant(unsigned int n, const struct s
>      return c;
>  }
>
> +struct hlsl_ir_node *new_unary_expr(enum hlsl_ir_expr_op op, struct hlsl_ir_node *arg, struct source_location loc)
> +{
> +    struct hlsl_ir_expr *expr;
> +
> +    if (!(expr = d3dcompiler_alloc(sizeof(*expr))))
> +        return NULL;
> +    init_node(&expr->node, HLSL_IR_EXPR, arg->data_type, loc);
> +    expr->op = op;
> +    expr->operands[0] = arg;
> +    return &expr->node;
> +}
> +
> +struct hlsl_ir_node *new_binary_expr(enum hlsl_ir_expr_op op,
> +        struct hlsl_ir_node *arg1, struct hlsl_ir_node *arg2)
> +{
> +    struct hlsl_ir_expr *expr;
> +
> +    assert(compare_hlsl_types(arg1->data_type, arg2->data_type));
> +
> +    if (!(expr = d3dcompiler_alloc(sizeof(*expr))))
> +        return NULL;
> +    init_node(&expr->node, HLSL_IR_EXPR, arg1->data_type, arg1->loc);
> +    expr->op = op;
> +    expr->operands[0] = arg1;
> +    expr->operands[1] = arg2;
> +    return &expr->node;
> +}

So, this patch helps in clarifying add_binary_expr() vs
new_binary_expr() and what each one does, which is good. It's not
clear to me what's next for new_unary_expr() and new_binary_expr()
though. I guess you might want to use them from yet-to-be-written
transformation passes. In that case though, wouldn't utils.c be a
better place (if nothing else to add even more separation from e.g.
add_binary_expr())? Or do you plan to move all of this kind of helper
functions into hlsl.y?

Or maybe you want to go in the complete opposite direction and use
them only from hlsl.y? Currently you could make these functions static
and drop them from the private header entirely.