[PATCH vkd3d 2/3] vkd3d-shader/hlsl: Take writemask into account when emitting a SM4 constant.

[Zebediah Figura]

On 11/16/21 3:22 AM, Giovanni Mascellani wrote:
> Hi,
> 
> On 15/11/21 18:48, Zebediah Figura (she/her) wrote:
>> This is probably quibbling, but how about this?
>>
>> j = 0;
>> for (i = 0; i < 4; ++i)
>> {
>>       if (instr.dsts[0].writemask & (1u << i))
>>           instr.srcs[0].reg.immconst_uint[i] = constant->value[j++].u;
>> }
>>
>> Feels a bit simpler to me.
> 
> I agree it's simpler, but as written it's also wrong. :-)
> 
> The problem is that when dimx == 1, we set the register to have
> dimension VKD3D_SM4_DIMENSION_SCALAR, which means that the constant must
> be written in the first register, no matter the writemask.
> 
> I think that trying to accommodate for that thing lead me to uselessly
> convoluted code, because your proposal can be fixed to this:
> 
> j = 0;
> for (i = 0; i < 4; ++i)
> {
>       if (dimx == 1 || instr.dsts[0].writemask & (1u << i))
>           instr.srcs[0].reg.immconst_uint[i] = constant->value[j++].u;
> }
> 
> This produces three useless writes for scalar registers, but hopefully
> that's not a big problem.
> 
> Thanks for the review, Giovanni.
> 

Personally I'd just do

struct sm4_register *reg = &instr.srcs[0].reg;

if (dimx == 1)
{
     reg->dim = VKD3D_SM4_DIMENSION_SCALAR;
     reg->immconst_uint[0] = constant->value[0].u;
}
else
{
     unsigned int i, j = 0;

     reg->dim = VKD3D_SM4_DIMENSION_VEC4;
     for (i = 0; i < 4; ++i)
     {
           if (instr.dsts[0].writemask & (1u << i))
               reg->immconst_uint[i] = constant->value[j++].u;
     }
}


It's more LOC, yeah, but I think it's clearer.