From: "Zebediah Figura (she/her)" <zfigura@codeweavers.com>
Subject: Re: [PATCH vkd3d 5/8] vkd3d-shader/hlsl: Support addition for all numeric types in fold_constants().
Message-Id: <84c45e1a-9268-11e7-9503-5bff51cd32b2@codeweavers.com>
Date: Mon, 17 Jan 2022 17:51:14 -0600
In-Reply-To: <20220106173949.82958-5-fcasas@codeweavers.com>
References: <20220106173949.82958-5-fcasas@codeweavers.com>

On 1/6/22 11:39, Francisco Casas wrote:
> Signed-off-by: Francisco Casas <fcasas@codeweavers.com>
> ---
>   libs/vkd3d-shader/hlsl_constant_ops.c | 45 ++++++++++++++++++++++++---
>   1 file changed, 40 insertions(+), 5 deletions(-)
> 
> diff --git a/libs/vkd3d-shader/hlsl_constant_ops.c b/libs/vkd3d-shader/hlsl_constant_ops.c
> index 3a778837..8279c58b 100644
> --- a/libs/vkd3d-shader/hlsl_constant_ops.c
> +++ b/libs/vkd3d-shader/hlsl_constant_ops.c
> @@ -20,6 +20,43 @@
>   
>   #include "hlsl.h"
>   
> +static int constant_op2_add(struct hlsl_ctx *ctx, struct hlsl_ir_constant *tgt,

"dst" is a bit more usual than "tgt".

Also, why does this function return int instead of bool?

> +        struct hlsl_ir_constant *src1, struct hlsl_ir_constant *src2)
> +{
> +    enum hlsl_base_type type = tgt->node.data_type->base_type;
> +    uint32_t u1, u2;
> +
> +    assert(type == src1->node.data_type->base_type);
> +    assert(type == src2->node.data_type->base_type);
> +
> +    for (int k = 0; k < 4; k++)
> +    {
> +        switch (type)
> +        {
> +            case HLSL_TYPE_FLOAT:
> +            case HLSL_TYPE_HALF:
> +                tgt->value[k].f = src1->value[k].f + src2->value[k].f;
> +                break;
> +            case HLSL_TYPE_DOUBLE:
> +                tgt->value[k].d = src1->value[k].d + src2->value[k].d;
> +                break;
> +            case HLSL_TYPE_INT:
> +                u1 = src1->value[k].i;
> +                u2 = src2->value[k].i;
> +                tgt->value[k].i = (int32_t)(u1 + u2);
> +                break;

It seems simpler just to fall through to the next case.

> +            case HLSL_TYPE_UINT:
> +                tgt->value[k].u = src1->value[k].u + src2->value[k].u;
> +                break;
> +            default:
> +                FIXME("Fold \"%s\" for type %s.", debug_hlsl_expr_op(HLSL_OP2_ADD),
> +                        debug_hlsl_type(ctx, tgt->node.data_type));
> +                return 0;
> +        }
> +    }
> +    return 1;
> +}
> +