[PATCH vkd3d 1/8] vkd3d-shader: Read the resource index from idx[1] instead of idx[0] when looking up SM5.1 bindings.

[Zebediah Figura]

On 5/25/20 2:08 PM, Henri Verbeet wrote:
> On Sat, 23 May 2020 at 00:56, Zebediah Figura <zfigura at codeweavers.com> wrote:
>> We still use the first index to uniquely identify the register in vkd3d_symbol,
>> only changing which field is used to declare it.
>>
>> One alternative to the "resource_idx" field of struct vkd3d_shader_register
>> could be to store that long with "register_space", and pass it as an extra
>> parameter to vkd3d_dxbc_compiler_get_descriptor_binding().
>>
> The way I understand shader model 5.1 resource/sampler declarations,
> they essentially assign an unique ID to a slice of a register space.
> In that sense, the most appropriate place for this would indeed be in
> the "declaration" part of struct vkd3d_shader_instruction, like the
> register space. I think the fact that "resource_idx" is unused outside
> of declaration instructions is consistent with that.
> 
>> More generally, there may be something to be said for exposing fields with
>> specific meaning from vkd3d_shader_register, instead of a flat array idx[3], so
>> as to hide the differences between SM5 and SM5.1 from spirv.c. If an SM1-3
>> frontend is added as well, this may be an even greater help (mostly considering
>> constant buffers.) In that case, resource_idx could become one such field.
>>
> In general, maybe. For this specific case, see above; I think it makes
> more sense to think of "resource_idx" as part of the declaration than
> as part of the register. The fact that SM 5.1 bytecode stores it as
> part of a register token seems like it should largely just be an
> interesting detail.

Sure, that makes sense.

I guess the idea was motivated not only by dcl_* instructions but also
by generic source accesses, whence also patches 2/8 and 4/8. Of course,
as you suggested in review of 2/8, normalizing offsets to 5.1 in dxbc.c
without bothering to give symbolic names also works...

> 
>>  static struct vkd3d_shader_descriptor_binding vkd3d_dxbc_compiler_get_descriptor_binding(
>>          struct vkd3d_dxbc_compiler *compiler, const struct vkd3d_shader_register *reg,
>> -        enum vkd3d_shader_resource_type resource_type, bool is_uav_counter)
>> +        unsigned int register_space, enum vkd3d_shader_resource_type resource_type, bool is_uav_counter)
> That looks like it should be in 5/8.
>