[PATCH vkd3d 4/8] vkd3d-shader/hlsl: Prepare to swap switch order on fold_constants().

[Francisco Casas]

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>

---

Don't mind about the 'goto fallback' too much, it is used transitionally
so that the tests still pass. It is removed in later patches.

Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>
---
 libs/vkd3d-shader/hlsl_constant_ops.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c
index 924e0380..3a778837 100644
--- a/libs/vkd3d-shader/hlsl_constant_ops.c
+++ b/libs/vkd3d-shader/hlsl_constant_ops.c
@@ -25,6 +25,7 @@ bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *cont
     struct hlsl_ir_constant *arg1, *arg2 = NULL, *res;
     struct hlsl_ir_expr *expr;
     unsigned int i, dimx;
+    bool success;
 
     if (instr->type != HLSL_IR_EXPR)
         return false;
@@ -44,6 +45,28 @@ bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *cont
         return false;
     init_node(&res->node, HLSL_IR_CONSTANT, instr->data_type, instr->loc);
 
+    success = false;
+
+    switch (expr->op)
+    {
+        default:
+            goto fallback;
+    }
+
+    if (success)
+    {
+        list_add_before(&expr->node.entry, &res->node.entry);
+        hlsl_replace_node(&expr->node, &res->node);
+        return true;
+    }
+    else
+    {
+        vkd3d_free(res);
+        return false;
+    }
+
+fallback:
+
     switch (instr->data_type->base_type)
     {
         case HLSL_TYPE_FLOAT:
-- 
2.25.1

[Giovanni Mascellani]

Hi,

under the same "code clean at all times" rule of my last email, I don't 
think we want to have this intermediate state. From what I've seen, 
usually boilerplate code is to be added with the first "useful" code 
that uses it (and removed with the last "useful" code that uses it).

Personally I'd also say that you can just convert the code in just one 
commit, without creating a commit for each single operation, which would 
let you drop the "goto fallback" trick. After all it's just four 
operations, not that hard to check. But I'm not sure others will agree 
with this.

Giovanni.


On 06/01/22 18:39, Francisco Casas wrote:
> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>
>
> ---
>
> Don't mind about the 'goto fallback' too much, it is used transitionally
> so that the tests still pass. It is removed in later patches.
>
> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com>
> ---
>   libs/vkd3d-shader/hlsl_constant_ops.c | 23 +++++++++++++++++++++++
>   1 file changed, 23 insertions(+)
>
> diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c
> index 924e0380..3a778837 100644
> --- a/libs/vkd3d-shader/hlsl_constant_ops.c
> +++ b/libs/vkd3d-shader/hlsl_constant_ops.c
> @@ -25,6 +25,7 @@ bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *cont
>       struct hlsl_ir_constant *arg1, *arg2 = NULL, *res;
>       struct hlsl_ir_expr *expr;
>       unsigned int i, dimx;
> +    bool success;
>
> if (instr->type != HLSL_IR_EXPR)
>           return false;
> @@ -44,6 +45,28 @@ bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *cont
>           return false;
>       init_node(&res->node, HLSL_IR_CONSTANT, instr->data_type, instr->loc);
>
> +    success = false;
> +
> +    switch (expr->op)
> +    {
> +        default:
> +            goto fallback;
> +    }
> +
> +    if (success)
> +    {
> +        list_add_before(&expr->node.entry, &res->node.entry);
> +        hlsl_replace_node(&expr->node, &res->node);
> +        return true;
> +    }
> +    else
> +    {
> +        vkd3d_free(res);
> +        return false;
> +    }
> +
> +fallback:
> +
>       switch (instr->data_type->base_type)
>       {
>           case HLSL_TYPE_FLOAT:

[Zebediah Figura (she/her)]

On 1/7/22 03:31, Giovanni Mascellani wrote:
> Hi,
>
> under the same "code clean at all times" rule of my last email, I don't 
> think we want to have this intermediate state. From what I've seen, 
> usually boilerplate code is to be added with the first "useful" code 
> that uses it (and removed with the last "useful" code that uses it).
>
> Personally I'd also say that you can just convert the code in just one 
> commit, without creating a commit for each single operation, which would 
> let you drop the "goto fallback" trick. After all it's just four 
> operations, not that hard to check. But I'm not sure others will agree 
> with this.

FWIW, I'd agree with this. Semantically it's one change, and it's clear 
enough what's being done.