[PATCH vkd3d v2 2/3] vkd3d-shader: Parse SM5.1 register ranges.
Henri Verbeet
hverbeet at gmail.com
Mon Jun 14 09:39:24 CDT 2021
On Thu, 3 Jun 2021 at 07:02, Conor McCarthy <cmccarthy at codeweavers.com> wrote:
> -static unsigned int shader_sm4_map_resource_idx(struct vkd3d_shader_register *reg, const struct vkd3d_sm4_data *priv)
> +static void shader_sm4_read_register_indices(struct vkd3d_shader_resource *resource,
> + struct vkd3d_shader_instruction *ins, struct vkd3d_sm4_data *priv)
> {
> if (shader_is_sm_5_1(priv))
> - return reg->idx[1].offset;
> + {
> + resource->register_first = resource->reg.reg.idx[1].offset;
> + resource->register_last = resource->reg.reg.idx[2].offset;
> + if (resource->register_last < resource->register_first)
> + {
> + FIXME("Invalid register range [%u:%u].\n", resource->register_first, resource->register_last);
> + ins->handler_idx = VKD3DSIH_INVALID;
> + }
> + }
> else
> - return reg->idx[0].offset;
> + {
> + resource->register_first = resource->reg.reg.idx[0].offset;
> + resource->register_last = resource->register_first;
> + }
> }
>
I don't think shader_sm4_read_register_indices() is a particularly
great name for what this does. In particular, actual register indices
(i.e., the "idx" field in the vkd3d_shader_register structure) are
read as part of shader_sm4_read_param(), and this resolves
descriptor/resource array ranges instead.
Error reporting should happen through vkd3d_shader_verror() instead,
so that we can report a proper compiler message to the application.
(See also e.g. hlsl_error(), preproc_error(),
vkd3d_dxbc_compiler_error(), etc. for examples.) We do of course have
existing code in the SM4 parser that still uses VKD3DSIH_INVALID
and/or FIXMEs/ERRs; it would be nice to get those cleaned up as well.
> static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins,
> @@ -635,6 +647,7 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins,
> struct vkd3d_sm4_data *priv)
> {
> struct vkd3d_shader_semantic *semantic = &ins->declaration.semantic;
> + struct vkd3d_shader_resource *resource = &semantic->resource;
> enum vkd3d_sm4_resource_type resource_type;
> const DWORD *end = &tokens[token_count];
> enum vkd3d_sm4_data_type data_type;
> @@ -653,8 +666,7 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins,
> semantic->resource_type = resource_type_table[resource_type];
> }
> reg_data_type = opcode == VKD3D_SM4_OP_DCL_RESOURCE ? VKD3D_DATA_RESOURCE : VKD3D_DATA_UAV;
> - shader_sm4_read_dst_param(priv, &tokens, end, reg_data_type, &semantic->resource.reg);
> - semantic->resource.register_index = shader_sm4_map_resource_idx(&semantic->resource.reg.reg, priv);
> + shader_sm4_read_dst_param(priv, &tokens, end, reg_data_type, &resource->reg);
>
> components = *tokens++;
> for (i = 0; i < VKD3D_VEC4_SIZE; i++)
> @@ -675,23 +687,21 @@ static void shader_sm4_read_dcl_resource(struct vkd3d_shader_instruction *ins,
> if (reg_data_type == VKD3D_DATA_UAV)
> ins->flags = (opcode_token & VKD3D_SM5_UAV_FLAGS_MASK) >> VKD3D_SM5_UAV_FLAGS_SHIFT;
>
> - shader_sm4_read_register_space(priv, &tokens, end, &semantic->resource.register_space);
> + shader_sm4_read_register_space(priv, &tokens, end, &resource->register_space);
> + shader_sm4_read_register_indices(resource, ins, priv);
> }
>
An easy way to split this patch would be to introduce the changes for
each instruction as a separate patch.
> @@ -640,7 +640,7 @@ struct vkd3d_shader_resource
> {
> struct vkd3d_shader_dst_param reg;
> unsigned int register_space;
> - unsigned int register_index;
> + unsigned int register_first, register_last;
> };
>
> enum vkd3d_decl_usage
> @@ -715,14 +715,16 @@ struct vkd3d_shader_register_semantic
> struct vkd3d_shader_sampler
> {
> struct vkd3d_shader_src_param src;
> - unsigned int register_space, register_index;
> + unsigned int register_space;
> + unsigned int register_first, register_last;
> };
>
> struct vkd3d_shader_constant_buffer
> {
> struct vkd3d_shader_src_param src;
> unsigned int size;
> - unsigned int register_space, register_index;
> + unsigned int register_space;
> + unsigned int register_first, register_last;
> };
>
This will fail to compile; there are references to "register_index" in
e.g. spirv.c.
In any case, it seems like it may be helpful to introduce a structure like this:
struct vkd3d_shader_descriptor_range
{
unsigned int space;
unsigned int first, last;
};
You could then do something like the following instead of
shader_sm4_read_register():
static void shader_sm4_resolve_descriptor_range(struct vkd3d_sm4_data *priv,
const struct vkd3d_shader_register *reg, struct
vkd3d_shader_descriptor_range *range)
{
[...]
}
and also use that structure for e.g. vkd3d_shader_scan_add_descriptor().
It would also be nice to make the corresponding changes to trace.c as
well, so that descriptor ranges are also properly disassembled.
More information about the wine-devel
mailing list