On Mon, 18 Nov 2019 at 18:11, Hans-Kristian Arntzen post@arntzen-software.no wrote:
Resource index is found in idx[0] in SM 5.0, but idx[1] when using SM 5.1, and register space is encoded reparately. An rb_tree keeps track of the internal resource index idx[0] and can map that to space/binding as required when emitting SPIR-V.
For this to work, we must also make UAV counters register space aware. In earlier implementation, UAV counter mask was assumed to correlate 1:1 with register_index, which breaks on SM 5.1.
This doesn't quite apply on current vkd3d git, but after resolving the conflicts, I get soft GPU lockups in the d3d12 tests. (I.e., "*ERROR* ring gfx timeout, but soft recovered") I didn't do a lot of debugging into that, but potentially could if you are unable to reproduce the issue yourself. Generally speaking, it seems this patch does a bunch of things and could be split. (E.g. introducing the "register_index" field in struct vkd3d_shader_semantic doesn't really depend on the changes to the public vkd3d-shader interface.)
+/* Extends vkd3d_shader_interface_info. */ +struct vkd3d_shader_effective_uav_counter_binding_info +{
- enum vkd3d_shader_structure_type type;
- const void *next;
- unsigned int *uav_register_spaces;
- unsigned int *uav_register_bindings;
- unsigned int uav_counter_count;
+};
Why does that extend vkd3d_shader_interface_info? This seems like something that should be part of vkd3d_shader_scan_info.
+struct vkd3d_shader_scan_info_binding +{
- unsigned int register_space;
- unsigned int register_idx;
+};
This is entirely unused.
+static bool shader_is_sm_5_1(const struct vkd3d_dxbc_compiler *compiler) +{
- return (compiler->shader_version.major * 100 + compiler->shader_version.minor) >= 501;
+}
Does the backend really need to know that the source was a shader model 5.1 shader?
- if (cb->register_space)
FIXME("Unhandled register space %u.\n", cb->register_space);
- if (shader_is_sm_5_1(compiler))
- {
struct vkd3d_sm51_symbol *sym;
sym = vkd3d_calloc(1, sizeof(*sym));
sym->key.idx = reg->idx[0].offset;
sym->key.descriptor_type = VKD3D_SHADER_DESCRIPTOR_TYPE_CBV;
sym->register_space = instruction->declaration.sampler.register_space;
sym->resource_idx = instruction->declaration.sampler.register_index;
if (rb_put(&compiler->sm51_resource_table, &sym->key, &sym->entry) == -1)
vkd3d_free(sym);
- }
<snip>
@@ -5294,8 +5438,18 @@ static void vkd3d_dxbc_compiler_emit_dcl_resource_structured(struct vkd3d_dxbc_c const struct vkd3d_shader_register *reg = &resource->reg.reg; unsigned int stride = resource->byte_stride;
- if (resource->register_space)
FIXME("Unhandled register space %u.\n", resource->register_space);
- if (shader_is_sm_5_1(compiler))
- {
struct vkd3d_sm51_symbol *sym;
sym = vkd3d_calloc(1, sizeof(*sym));
sym->key.idx = resource->reg.reg.idx[0].offset;
sym->key.descriptor_type = resource->reg.reg.type == VKD3DSPR_UAV ? VKD3D_SHADER_DESCRIPTOR_TYPE_UAV : VKD3D_SHADER_DESCRIPTOR_TYPE_SRV;
sym->register_space = resource->register_space;
sym->resource_idx = resource->register_index;
if (rb_put(&compiler->sm51_resource_table, &sym->key, &sym->entry) == -1)
vkd3d_free(sym);
- }
This would probably benefit from a helper function, but I also imagine it could simply add the register space information to the existing vkd3d_symbol.