[PATCH vkd3d 3/8] vkd3d-shader/hlsl: Write SM4 signatures.
Matteo Bruni
matteo.mystral at gmail.com
Sun Aug 22 14:13:39 CDT 2021
On Fri, Aug 20, 2021 at 1:45 AM Zebediah Figura <zfigura at codeweavers.com> wrote:
>
> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
> ---
> include/private/vkd3d_common.h | 14 +++
> include/vkd3d_d3dcommon.idl | 37 ++++++
> libs/vkd3d-shader/hlsl.h | 6 +
> libs/vkd3d-shader/hlsl_codegen.c | 59 ++++++---
> libs/vkd3d-shader/hlsl_sm4.c | 200 ++++++++++++++++++++++++++++++-
> 5 files changed, 300 insertions(+), 16 deletions(-)
> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
> index b5f7832f..939b356c 100644
> --- a/libs/vkd3d-shader/hlsl_codegen.c
> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> @@ -1012,13 +1012,26 @@ static void allocate_temp_registers(struct hlsl_ctx *ctx, struct hlsl_ir_functio
>
> static void allocate_semantic_register(struct hlsl_ctx *ctx, struct hlsl_ir_var *var, unsigned int *counter, bool output)
> {
> + static const char *shader_names[] =
> + {
> + [VKD3D_SHADER_TYPE_PIXEL] = "Pixel",
> + [VKD3D_SHADER_TYPE_VERTEX] = "Vertex",
> + [VKD3D_SHADER_TYPE_GEOMETRY] = "Geometry",
> + [VKD3D_SHADER_TYPE_HULL] = "Hull",
> + [VKD3D_SHADER_TYPE_DOMAIN] = "Domain",
> + [VKD3D_SHADER_TYPE_COMPUTE] = "Compute",
> + };
> +
> + unsigned int type;
> + uint32_t reg;
> + bool builtin;
> +
> assert(var->semantic.name);
>
> if (ctx->profile->major_version < 4)
> {
> - D3DSHADER_PARAM_REGISTER_TYPE type;
> - uint32_t reg, usage_idx;
> D3DDECLUSAGE usage;
> + uint32_t usage_idx;
>
> if (!hlsl_sm1_usage_from_semantic(&var->semantic, &usage, &usage_idx))
> {
> @@ -1027,19 +1040,35 @@ static void allocate_semantic_register(struct hlsl_ctx *ctx, struct hlsl_ir_var
> return;
> }
>
> - if (hlsl_sm1_register_from_semantic(ctx, &var->semantic, output, &type, ®))
> - {
> - TRACE("%s %s semantic %s[%u] matches predefined register %#x[%u].\n",
> - ctx->profile->type == VKD3D_SHADER_TYPE_PIXEL ? "Pixel" : "Vertex", output ? "output" : "input",
> - var->semantic.name, var->semantic.index, type, reg);
> - }
> - else
> + if ((!output && !var->last_read) || (output && !var->first_write))
> + return;
> +
> + builtin = hlsl_sm1_register_from_semantic(ctx, &var->semantic, output, &type, ®);
This appears to slip in a functional change, where now we don't
allocate registers for SM1 "user" (not system value / predefined
register) inputs / outputs if they're unused. That's not the case as
this is just moving a check from allocate_semantic_registers(). As it
turns out, things aren't quite working right for those semantics
anyway.
Let me go fully OT and digress into the details of this SM1 issue.
Assume we have a PS with this signature:
float4 main(in float4 color : COLOR, in float4 normal : NORMAL) : SV_TARGET
and compile it for the ps_2_0 profile. If both our inputs are used,
they will be both allocated to v0, which seems bad. If normal isn't
used, because of the change above it won't be allocated, but since
write_sm1_semantic_dcl() doesn't check for the allocated flag it will
generate a dcl instruction anyway (to v0, since the allocation info is
zero-initialized).
There are a few things to fix here. First of all, these "user"
semantics are supposed to be mapped to the t# registers in ps_2_0
(there are 2 color inputs, v0-1, and 8 t0-7 "texcoords", but actually,
generic inputs). Then we need to make sure to not step onto registers
used by "builtin" semantics.
I suggest reworking allocate_semantic_registers() so that both builtin
and user inputs / outputs get allocation info; also keep track of the
register type for allocations (maybe as SM-specific values); make sure
that user inputs / outputs don't step over already allocated
registers. Finally, have everything downstream from register
allocation solely refer to the allocation data.
I haven't really tried to write the patches so there might be
something / a lot missing in the above.
> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
> index 38c71626..894c513f 100644
> --- a/libs/vkd3d-shader/hlsl_sm4.c
> +++ b/libs/vkd3d-shader/hlsl_sm4.c
> +bool hlsl_sm4_usage_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_semantic *semantic,
> + bool output, D3D_NAME *usage)
> +{
> + unsigned int i;
> +
> + static const struct
> + {
> + const char *name;
> + bool output;
> + enum vkd3d_shader_type shader_type;
> + D3DDECLUSAGE usage;
> + }
> + semantics[] =
> + {
> + {"position", false, VKD3D_SHADER_TYPE_GEOMETRY, D3D_NAME_POSITION},
> + {"sv_position", false, VKD3D_SHADER_TYPE_GEOMETRY, D3D_NAME_POSITION},
> + {"sv_primitiveid", false, VKD3D_SHADER_TYPE_GEOMETRY, D3D_NAME_PRIMITIVE_ID},
> +
> + {"position", true, VKD3D_SHADER_TYPE_GEOMETRY, D3D_NAME_POSITION},
> + {"sv_position", true, VKD3D_SHADER_TYPE_GEOMETRY, D3D_NAME_POSITION},
> + {"sv_primitiveid", true, VKD3D_SHADER_TYPE_GEOMETRY, D3D_NAME_PRIMITIVE_ID},
> +
> + {"position", false, VKD3D_SHADER_TYPE_PIXEL, D3D_NAME_POSITION},
> + {"sv_position", false, VKD3D_SHADER_TYPE_PIXEL, D3D_NAME_POSITION},
> +
> + {"color", true, VKD3D_SHADER_TYPE_PIXEL, D3D_NAME_TARGET},
> + {"depth", true, VKD3D_SHADER_TYPE_PIXEL, D3D_NAME_DEPTH},
> + {"sv_target", true, VKD3D_SHADER_TYPE_PIXEL, D3D_NAME_TARGET},
> + {"sv_depth", true, VKD3D_SHADER_TYPE_PIXEL, D3D_NAME_DEPTH},
> +
> + {"sv_position", false, VKD3D_SHADER_TYPE_VERTEX, D3D_NAME_UNDEFINED},
> +
> + {"position", true, VKD3D_SHADER_TYPE_VERTEX, D3D_NAME_POSITION},
> + {"sv_position", true, VKD3D_SHADER_TYPE_VERTEX, D3D_NAME_POSITION},
> + };
Eventually we might want to introduce bitmask flags for the shader
type and group up identical lines together (I imagine this will become
quite a bit more verbose with tessellation shaders).
> +
> + for (i = 0; i < ARRAY_SIZE(semantics); ++i)
> + {
> + if (!ascii_strcasecmp(semantic->name, semantics[i].name)
> + && output == semantics[i].output
> + && ctx->profile->type == semantics[i].shader_type
> + && !ascii_strncasecmp(semantic->name, "sv_", 3))
> + {
> + *usage = semantics[i].usage;
> + return true;
> + }
> + }
> +
> + if (!ascii_strncasecmp(semantic->name, "sv_", 3))
> + return false;
> +
> + *usage = D3D_NAME_UNDEFINED;
> + return true;
> +}
I don't quite understand what's going on here. If I read it correctly
you're basically ignoring all the table entries that don't start with
"sv_". Is there some planned extension of this function where it will
start to do something different?
> +
> +static void write_sm4_signature(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc, bool output)
> +{
> + struct vkd3d_bytecode_buffer buffer = {0};
> + struct vkd3d_string_buffer *string;
> + const struct hlsl_ir_var *var;
> + size_t count_position;
> + unsigned int i;
> + bool ret;
> +
> + count_position = put_u32(&buffer, 0);
> + put_u32(&buffer, 8); /* unknown */
> +
> + LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
> + {
> + unsigned int width = (1u << var->data_type->dimx) - 1, use_mask;
> + enum vkd3d_sm4_register_type type;
> + uint32_t usage_idx, reg_idx;
> + D3D_NAME usage;
> +
> + if ((output && !var->is_output_semantic) || (!output && !var->is_input_semantic))
> + continue;
> +
> + ret = hlsl_sm4_usage_from_semantic(ctx, &var->semantic, output, &usage);
> + assert(ret);
> + usage_idx = var->semantic.index;
> +
> + if (!hlsl_sm4_register_from_semantic(ctx, &var->semantic, output, &type, ®_idx))
> + {
> + assert(var->reg.allocated);
> + type = VKD3D_SM4_RT_INPUT;
> + reg_idx = var->reg.id;
> + }
Somewhat similarly here, type doesn't seem to be used at all (yet).
> +
> + use_mask = width; /* FIXME: accurately report use mask */
> + if (output)
> + use_mask = 0xf ^ use_mask;
Huh, that's interesting. I wonder if we're taking that into account elsewhere.
More information about the wine-devel
mailing list