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, + 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; + 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; +} + bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context) { struct hlsl_ir_constant *arg1, *arg2 = NULL, *res; @@ -49,6 +86,9 @@ bool fold_constants(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *cont
switch (expr->op) { + case HLSL_OP2_ADD: + success = constant_op2_add(ctx, res, arg1, arg2); + break; default: goto fallback; } @@ -120,11 +160,6 @@ fallback: res->value[i].u = -arg1->value[i].u; break;
- case HLSL_OP2_ADD: - for (i = 0; i < instr->data_type->dimx; ++i) - res->value[i].u = arg1->value[i].u + arg2->value[i].u; - break; - case HLSL_OP2_MUL: for (i = 0; i < instr->data_type->dimx; ++i) res->value[i].u = arg1->value[i].u * arg2->value[i].u;
Hi,
On 06/01/22 18:39, Francisco Casas wrote:
Maybe it makes sense to drop a comment here to make it clear why you're doing this? It might not be completely obvious to anybody. Something like /* Using unsigned to avoid undefined behavior on signed overflow. */. Same thing for the other patches.
Giovanni.
January 17, 2022 8:51 PM, "Zebediah Figura (she/her)" zfigura@codeweavers.com wrote:
Ok, I will change the name.
Also, why does this function return int instead of bool?
Sorry, force of habit.
That's clever, but maybe too clever! While it should work, I am inclined to leave it as it is, for readability and for having the peace of mind that it won't surprise us in the future.
We would have to remember to keep hlsl_constant_value as a union and, for instance, I have the same concern as this person: https://stackoverflow.com/questions/11373203/accessing-inactive-union-member... and the answers don't seem conclusive.
On 1/19/22 12:30, Francisco Casas wrote:
Type punning is allowed, and if int32_t can't be correctly reinterpreted as uint32_t we have bigger problems.
Hi,
Il 19/01/22 19:30, Francisco Casas ha scritto:
I agree with Zebediah, in our case it's ok to do that.
More specifically, notice that that question was about C++, not about C, and often the two languages tend to differ in this kind of things. Answers are both about C and C++, but at least for C this is not undefined behavior (see for example the first part of https://stackoverflow.com/a/11996970/807307). It is still implementation-defined, but here and in many other places we assume that we use GCC (or a compatible thing), which defines it the way we like:
https://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/Structures-unions-enumerations-...
Giovanni.
January 20, 2022 5:48 AM, "Giovanni Mascellani" gmascellani@codeweavers.com wrote:
Roger that. In that case I will also do it for multiplication and negation.
Francisco.