Re: [PATCH v2 0/3] MR382: vkd3d-shader/tpf: Replace sm4 register structs with vkd3d-shader register structs, part 4.

[Francisco Casas (＠fcasas)]

On Tue Oct  3 15:06:23 2023 +0000, Henri Verbeet wrote:
> > > That should cause compilation to fail, right? (And probably with
>
> something like an internal compiler error, because this is something
> vkd3d-shader itself would trigger.)
> > Right. We don't want incorrect shaders to compile. I will put
> > ERR("Unhandled register modifier %#x.\n", src->modifiers);
> > vkd3d_unreachable();
> >
> > then.
>
> I'll approve it, but I don't think that's terribly robust. Much like
> assert() and abort(), this escalates what would by definition be a bug
> in our code from causing compilation failure to causing program termination.
> > @@ -4001,9 +3995,9 @@ static void write_sm4_instruction(const struct
>
> tpf_writer *tpf, const struct sm4
> >  size += instr->modifier_count;
> >  for (i = 0; i < instr->dst_count; ++i)
> > -
> >    size += vkd3d_shader_dst_param_order(&instr->dsts[i]);
> > -
> >    size += sm4_token_count_from_dst_register(&instr->dsts[i]);
> > for (i = 0; i < instr->src_count; ++i)
> > -
> >    size += sm4_src_register_order(&instr->srcs[i]);
> > -
> >    size += sm4_token_count_from_src_register(&instr->srcs[i]);
> > size += instr->idx_count;
> >    if (instr->byte_stride)
> >        ++size;
>
> As an aside, note that we don't necessarily need to calculate the size
> in advance here; we could do something like this:
>     token_position = put_u32(buffer, 0);
>     ...
>     size = (buffer->size - token_position) / sizeof(uint32_t);
>     token |= (size << VKD3D_SM4_INSTRUCTION_LENGTH_SHIFT);
>     set_u32(buffer, token_position, token);
>
> Might be a nice follow-up to this series.

I see. Nice observation.