On 5/25/20 2:08 PM, Henri Verbeet wrote:
On Sat, 23 May 2020 at 00:56, Zebediah Figura zfigura@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.