[PATCH vkd3d v2 5/6] vkd3d-shader: Write the SM1 constant table.

[Zebediah Figura (she/her)]

On 4/20/21 3:16 AM, Matteo Bruni wrote:
> On Fri, Apr 16, 2021 at 2:04 AM Zebediah Figura <zfigura at codeweavers.com> wrote:
>>
>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>> ---
>>  Makefile.am                      |   2 +
>>  include/.gitignore               |   1 +
>>  include/vkd3d_d3d9types.h        |   6 +
>>  include/vkd3d_d3dx9shader.idl    |  76 +++++++++
>>  libs/vkd3d-shader/hlsl.h         |   7 +-
>>  libs/vkd3d-shader/hlsl.y         |   1 +
>>  libs/vkd3d-shader/hlsl_codegen.c | 254 ++++++++++++++++++++++++++++++-
>>  7 files changed, 344 insertions(+), 3 deletions(-)
>>  create mode 100644 include/vkd3d_d3dx9shader.idl
>>
>> diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
>> index 3a0bf38d..3dc7a26b 100644
>> --- a/libs/vkd3d-shader/hlsl.h
>> +++ b/libs/vkd3d-shader/hlsl.h
>> @@ -115,7 +115,6 @@ struct hlsl_type
>>      unsigned int modifiers;
>>      unsigned int dimx;
>>      unsigned int dimy;
>> -    unsigned int reg_size;
>>      union
>>      {
>>          struct list *elements;
>> @@ -125,6 +124,9 @@ struct hlsl_type
>>              unsigned int elements_count;
>>          } array;
>>      } e;
>> +
>> +    unsigned int reg_size;
>> +    unsigned int bytecode_offset;
>>  };
>>
>>  struct hlsl_struct_field
>> @@ -134,7 +136,9 @@ struct hlsl_struct_field
>>      struct hlsl_type *type;
>>      const char *name;
>>      const char *semantic;
>> +
>>      unsigned int reg_offset;
>> +    unsigned int name_offset;
> 
> Although these are indeed two offsets, they offset into very different
> things. I don't know that I have any good suggestions
> (name_bc_offset?) and I guess I'm not too disturbed by the current
> naming anyway, but I feel there is room for improvement.

Yes, you're right; honestly the mistake here is grouping them together
via whitespace I think, but of course the naming is what led me to do
that. "name_bytecode_offset" strikes me as at least a better name than
"name_offset".

I've vacillated for a while, also; it may make more sense not to store
reg_offset + reg_size but instead to calculate them as needed. But I've
tried that approach a couple times and the code doesn't look prettier :-(

> 
>> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
>> index 25227057..73f8bdb0 100644
>> --- a/libs/vkd3d-shader/hlsl_codegen.c
>> +++ b/libs/vkd3d-shader/hlsl_codegen.c
> 
>> +    LIST_FOR_EACH_ENTRY(var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
>> +    {
>> +        if (!var->semantic && var->reg.allocated)
>> +        {
>> +            put_dword(buffer, 0); /* name */
>> +            put_dword(buffer, D3DXRS_FLOAT4 | (var->reg.id << 16));
> 
> This is fine for now, but maybe a comment mentioning that we might not
> always want the float register set here would be useful.
> Relatedly, we might have the same variable allocated to multiple
> register sets (e.g. if it's used in a MOV and in a FOR) so in theory
> there needs to be a loop (and we need to support multiple register
> allocations per var, or something to the same effect). Again, no need
> to change the code right away, just something that might be worth a
> comment.
> 

Fair enough. Actually I'd be inclined to handle this by creating
multiple hlsl_ir_var objects.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 495 bytes
Desc: OpenPGP digital signature
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20210420/86d0cf3f/attachment.sig>