From: Giovanni Mascellani <gmascellani@codeweavers.com>
Subject: Re: [PATCH vkd3d 2/8] vkd3d-shader/hlsl: Rename replace_node() to hlsl_replace_node().
Message-Id: <b3b04cc4-0796-7788-3675-d3ee4697a068@codeweavers.com>
Date: Fri, 7 Jan 2022 10:18:54 +0100
In-Reply-To: <20220106173949.82958-2-fcasas@codeweavers.com>
References: <20220106173949.82958-2-fcasas@codeweavers.com>

Hi,

I think that this patch should be squashed with 1/8. The idea is that 
the code should be "clean" at all times, even between the patches of a 
single patch set, and my understanding is that being clean includes 
having a "hlsl_" prefix for all the symbols in hlsl.h (this also applies 
to 3/8).

Also, I think that renaming the function and moving it to another file 
are changes simple enough that they can be merged in the same patch. But 
if you really prefer doing that in two patches, I'd first change the 
name and then move.

Giovanni.


On 06/01/22 18:39, Francisco Casas wrote:
> Signed-off-by: Francisco Casas <fcasas@codeweavers.com>
> ---
>   libs/vkd3d-shader/hlsl.c         |  2 +-
>   libs/vkd3d-shader/hlsl.h         |  2 +-
>   libs/vkd3d-shader/hlsl_codegen.c | 12 ++++++------
>   3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
> index 3fb84b28..d92e731e 100644
> --- a/libs/vkd3d-shader/hlsl.c
> +++ b/libs/vkd3d-shader/hlsl.c
> @@ -1384,7 +1384,7 @@ void hlsl_dump_function(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl
>       vkd3d_string_buffer_cleanup(&buffer);
>   }
>   
> -void replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new)
> +void hlsl_replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new)
>   {
>       struct hlsl_src *src, *next;
>   
> diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
> index 393b3f1f..530d12ad 100644
> --- a/libs/vkd3d-shader/hlsl.h
> +++ b/libs/vkd3d-shader/hlsl.h
> @@ -680,7 +680,7 @@ void hlsl_dump_function(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl
>   
>   int hlsl_emit_dxbc(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_func, struct vkd3d_shader_code *out);
>   
> -void replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new);
> +void hlsl_replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new);
>   
>   void hlsl_free_instr(struct hlsl_ir_node *node);
>   void hlsl_free_instr_list(struct list *list);
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index 00f3dae3..6cb4df8b 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -254,7 +254,7 @@ static bool lower_broadcasts(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, v
>               return false;
>           list_add_after(&new_cast->node.entry, &swizzle->node.entry);
>   
> -        replace_node(&cast->node, &swizzle->node);
> +        hlsl_replace_node(&cast->node, &swizzle->node);
>           return true;
>       }
>   
> @@ -402,7 +402,7 @@ static bool copy_propagation_analyze_load(struct hlsl_ctx *ctx, struct hlsl_ir_l
>       if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, type->dimx, new_node, &node->loc)))
>           return false;
>       list_add_before(&node->entry, &swizzle_node->node.entry);
> -    replace_node(node, &swizzle_node->node);
> +    hlsl_replace_node(node, &swizzle_node->node);
>       return true;
>   }
>   
> @@ -490,7 +490,7 @@ static bool fold_redundant_casts(struct hlsl_ctx *ctx, struct hlsl_ir_node *inst
>           if (hlsl_types_are_equal(src_type, dst_type)
>                   || (src_type->base_type == dst_type->base_type && is_vec1(src_type) && is_vec1(dst_type)))
>           {
> -            replace_node(&expr->node, expr->operands[0].node);
> +            hlsl_replace_node(&expr->node, expr->operands[0].node);
>               return true;
>           }
>       }
> @@ -632,7 +632,7 @@ static bool lower_narrowing_casts(struct hlsl_ctx *ctx, struct hlsl_ir_node *ins
>               return false;
>           list_add_after(&new_cast->node.entry, &swizzle->node.entry);
>   
> -        replace_node(&cast->node, &swizzle->node);
> +        hlsl_replace_node(&cast->node, &swizzle->node);
>           return true;
>       }
>   
> @@ -741,7 +741,7 @@ static bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, voi
>       }
>   
>       list_add_before(&expr->node.entry, &res->node.entry);
> -    replace_node(&expr->node, &res->node);
> +    hlsl_replace_node(&expr->node, &res->node);
>       return true;
>   }
>   
> @@ -761,7 +761,7 @@ static bool remove_trivial_swizzles(struct hlsl_ctx *ctx, struct hlsl_ir_node *i
>           if (((swizzle->swizzle >> (2 * i)) & 3) != i)
>               return false;
>   
> -    replace_node(instr, swizzle->val.node);
> +    hlsl_replace_node(instr, swizzle->val.node);
>   
>       return true;
>   }