[PATCH vkd3d v2 4/7] vkd3d-shader: Only mark entry point parameters as uniform, input, or output.

[Zebediah Figura (she/her)]

On 5/10/21 7:19 AM, Matteo Bruni wrote:
> On Tue, Apr 27, 2021 at 7:30 PM Zebediah Figura <zfigura at codeweavers.com> wrote:
>>
>> In fact, don't even mark them directly; only mark the synthetic variables.
>>
>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>> ---
>> v2: New patch.
> 
> Nice patch, just a nitpick.
> 
>>   libs/vkd3d-shader/hlsl.c         | 38 +++++++++++----------------
>>   libs/vkd3d-shader/hlsl.h         |  6 +++--
>>   libs/vkd3d-shader/hlsl.y         | 22 ++--------------
>>   libs/vkd3d-shader/hlsl_codegen.c | 45 +++++++++++++++++++++-----------
>>   4 files changed, 51 insertions(+), 60 deletions(-)
>>
>> diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
>> index 50b40d65..5804c8e5 100644
>> --- a/libs/vkd3d-shader/hlsl.c
>> +++ b/libs/vkd3d-shader/hlsl.c
>> @@ -388,7 +388,7 @@ struct hlsl_ir_expr *hlsl_new_copy(struct hlsl_ir_node *node)
>>   }
>>
>>   struct hlsl_ir_var *hlsl_new_var(const char *name, struct hlsl_type *type, const struct vkd3d_shader_location loc,
>> -        const char *semantic, const struct hlsl_reg_reservation *reg_reservation)
>> +        const char *semantic, unsigned int modifiers, const struct hlsl_reg_reservation *reg_reservation)
>>   {
>>       struct hlsl_ir_var *var;
>>
>> @@ -399,6 +399,7 @@ struct hlsl_ir_var *hlsl_new_var(const char *name, struct hlsl_type *type, const
>>       var->data_type = type;
>>       var->loc = loc;
>>       var->semantic = semantic;
>> +    var->modifiers = modifiers;
>>       var->reg_reservation = reg_reservation;
>>       return var;
>>   }
>> @@ -406,7 +407,7 @@ struct hlsl_ir_var *hlsl_new_var(const char *name, struct hlsl_type *type, const
>>   struct hlsl_ir_var *hlsl_new_synthetic_var(struct hlsl_ctx *ctx, const char *name, struct hlsl_type *type,
>>           const struct vkd3d_shader_location loc)
>>   {
>> -    struct hlsl_ir_var *var = hlsl_new_var(vkd3d_strdup(name), type, loc, NULL, NULL);
>> +    struct hlsl_ir_var *var = hlsl_new_var(vkd3d_strdup(name), type, loc, NULL, 0, NULL);
>>
>>       if (var)
>>           list_add_tail(&ctx->globals->vars, &var->scope_entry);
>> @@ -571,7 +572,7 @@ struct hlsl_ir_function_decl *hlsl_new_func_decl(struct hlsl_ctx *ctx, struct hl
>>           char name[28];
>>
>>           sprintf(name, "<retval-%p>", decl);
>> -        if (!(return_var = hlsl_new_var(vkd3d_strdup(name), return_type, loc, semantic, NULL)))
>> +        if (!(return_var = hlsl_new_var(vkd3d_strdup(name), return_type, loc, semantic, 0, NULL)))
>>           {
>>               vkd3d_free(decl);
>>               return NULL;
>> @@ -868,12 +869,17 @@ static void dump_src(struct vkd3d_string_buffer *buffer, const struct hlsl_src *
>>
>>   static void dump_ir_var(struct vkd3d_string_buffer *buffer, const struct hlsl_ir_var *var)
>>   {
>> -    if (var->is_input_varying)
>> -        vkd3d_string_buffer_printf(buffer, "in ");
>> -    if (var->is_output_varying)
>> -        vkd3d_string_buffer_printf(buffer, "out ");
>> -    if (var->is_uniform)
>> -        vkd3d_string_buffer_printf(buffer, "uniform ");
>> +    if (var->modifiers)
>> +    {
>> +        struct vkd3d_string_buffer_cache string_buffers;
>> +        struct vkd3d_string_buffer *string;
>> +
>> +        vkd3d_string_buffer_cache_init(&string_buffers);
>> +        if ((string = hlsl_modifiers_to_string(&string_buffers, var->modifiers)))
>> +            vkd3d_string_buffer_printf(buffer, "%s ", string->buffer);
>> +        vkd3d_string_buffer_release(&string_buffers, string);
>> +        vkd3d_string_buffer_cache_cleanup(&string_buffers);
>> +    }
>>       vkd3d_string_buffer_printf(buffer, "%s %s", debug_hlsl_type(var->data_type), var->name);
>>       if (var->semantic)
>>           vkd3d_string_buffer_printf(buffer, " : %s", var->semantic);
> 
> I guess this is okay for now (also I don't know how "temporary" this
> code is) but, in general, creating and destroying a
> vkd3d_string_buffer_cache every time kinda defeats the purpose of the
> structure. Passing struct hlsl_ctx around might require some changes
> in this area but it's probably the direction to move towards.
> 

I didn't put any priority towards it as it's in the debug path, but it's 
probably a good idea regardless.