[PATCH v2 1/2] vkd3d: Deal correctly with SM 5.1 register spaces.

[Henri Verbeet]

On Mon, 18 Nov 2019 at 18:11, Hans-Kristian Arntzen
<post at 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.