[PATCH vkd3d 2/8] vkd3d-shader/hlsl: Rename replace_node() to hlsl_replace_node().

[Francisco Casas]

Signed-off-by: Francisco Casas 
---
 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;
 }

[Giovanni Mascellani]

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
>
> 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;
> }

[Francisco Casas]

January 7, 2022 6:19 AM, "Giovanni Mascellani"  wrote:
> 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.

Okay!
I have been told that's preferable to move something and change it on different patches,
but maybe that is when the changes are more than just changing the name.

[Zebediah Figura (she/her)]

On 1/7/22 06:12, Francisco Casas wrote:
> January 7, 2022 6:19 AM, "Giovanni Mascellani"  wrote:
> > 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.
>
> Okay!
> I have been told that's preferable to move something and change it on different patches,
> but maybe that is when the changes are more than just changing the name.

The point is to make it clear what actually changed; if you move 
something and change it at the same time it's not obvious from the diff.

In a case like this it's fine to do it at the same time (and probably 
preferable).