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: <0edba0a8-c671-7fa5-5d75-fd03d5ff9f4a@codeweavers.com>
Date: Wed, 19 Jan 2022 17:45:34 -0600
In-Reply-To: <cbd16fd310c38ebafddb7f4a49d7cfda@codeweavers.com>
References: <84c45e1a-9268-11e7-9503-5bff51cd32b2@codeweavers.com> <20220106173949.82958-5-fcasas@codeweavers.com> <cbd16fd310c38ebafddb7f4a49d7cfda@codeweavers.com>

On 1/19/22 12:30, Francisco Casas wrote:
>>> + 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.
> 
> 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-undefined-behavior
> and the answers don't seem conclusive.
> 

Type punning is allowed, and if int32_t can't be correctly reinterpreted 
as uint32_t we have bigger problems.