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.