[PATCH vkd3d v3] vkd3d-shader/hlsl: Explicitly specify the swizzle type for sm4 source registers.

[Zebediah Figura]

On 11/17/21 3:29 AM, Matteo Bruni wrote:
> On Mon, Nov 15, 2021 at 7:40 PM Francisco Casas <fcasas at codeweavers.com> wrote:
>>
>> Some register types do not use a consistent swizzle type, so the
>> sm4_swizzle_type() function is removed.
>>
>> The swizzle type now must be specified using the swizzle_type field.
>>
>> Signed-off-by: Francisco Casas <fcasas at codeweavers.com>
>> ---
>>   libs/vkd3d-shader/hlsl.h         |  2 +-
>>   libs/vkd3d-shader/hlsl_codegen.c |  2 +-
>>   libs/vkd3d-shader/hlsl_sm4.c     | 96 ++++++++++++++++----------------
>>   3 files changed, 50 insertions(+), 50 deletions(-)
> 
>> diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
>> index e597425a..e857a681 100644
>> --- a/libs/vkd3d-shader/hlsl_sm4.c
>> +++ b/libs/vkd3d-shader/hlsl_sm4.c
>> @@ -26,7 +26,7 @@
>>   static void write_sm4_block(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, const struct hlsl_block *block);
>>
>>   bool hlsl_sm4_register_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_semantic *semantic,
>> -        bool output, enum vkd3d_sm4_register_type *type, bool *has_idx)
>> +        bool output, enum vkd3d_sm4_register_type *type, enum vkd3d_sm4_swizzle_type *swizzle_type, bool *has_idx)
>>   {
>>       unsigned int i;
>>
>> @@ -36,19 +36,20 @@ bool hlsl_sm4_register_from_semantic(struct hlsl_ctx *ctx, const struct hlsl_sem
>>           bool output;
>>           enum vkd3d_shader_type shader_type;
>>           enum vkd3d_sm4_register_type type;
>> +        enum vkd3d_sm4_swizzle_type swizzle_type;
>>           bool has_idx;
>>       }
>>       register_table[] =
>>       {
>> -        {"sv_primitiveid",  false, VKD3D_SHADER_TYPE_GEOMETRY, VKD3D_SM4_RT_PRIMID, false},
>> +        {"sv_primitiveid",  false, VKD3D_SHADER_TYPE_GEOMETRY, VKD3D_SM4_RT_PRIMID, VKD3D_SM4_SWIZZLE_NONE, false},
>>
>>           /* Put sv_target in this table, instead of letting it fall through to
>>            * default varying allocation, so that the register index matches the
>>            * usage index. */
>> -        {"color",           true, VKD3D_SHADER_TYPE_PIXEL, VKD3D_SM4_RT_OUTPUT,     true},
>> -        {"depth",           true, VKD3D_SHADER_TYPE_PIXEL, VKD3D_SM4_RT_DEPTHOUT,   false},
>> -        {"sv_depth",        true, VKD3D_SHADER_TYPE_PIXEL, VKD3D_SM4_RT_DEPTHOUT,   false},
>> -        {"sv_target",       true, VKD3D_SHADER_TYPE_PIXEL, VKD3D_SM4_RT_OUTPUT,     true},
>> +        {"color",           true, VKD3D_SHADER_TYPE_PIXEL, VKD3D_SM4_RT_OUTPUT, VKD3D_SM4_SWIZZLE_VEC4, true},
>> +        {"depth",           true, VKD3D_SHADER_TYPE_PIXEL, VKD3D_SM4_RT_DEPTHOUT, VKD3D_SM4_SWIZZLE_VEC4, false},
>> +        {"sv_depth",        true, VKD3D_SHADER_TYPE_PIXEL, VKD3D_SM4_RT_DEPTHOUT, VKD3D_SM4_SWIZZLE_VEC4, false},
>> +        {"sv_target",       true, VKD3D_SHADER_TYPE_PIXEL, VKD3D_SM4_RT_OUTPUT, VKD3D_SM4_SWIZZLE_VEC4, true},
>>       };
> 
> Is VEC4 right for depth / sv_depth?
> I see that below in some, but not all, places we're manually replacing
> the swizzle type for depthout with VKD3D_SM4_DIMENSION_SCALAR. It
> seems like a good idea to directly report the correct swizzle type
> here instead.
> Unless I'm missing something.
> 

It's not really wrong or right, I think. As far as I can tell there's no 
way to actually get output semantics to show up in source registers 
(even SM 5.1 doesn't), so we might as well just put something here.

Of course there's other ways to deal with this, but I at least don't 
dislike this one enough to complain.