[PATCH vkd3d v3 16/17] vkd3d-shader/hlsl: Replace register offsets with index paths in input/output copies.

[Francisco Casas]

Hello,

On 28-07-22 16:59, Matteo Bruni wrote:
> On Wed, Jul 20, 2022 at 3:24 PM Francisco Casas <fcasas at codeweavers.com> wrote:
>>
>> Signed-off-by: Francisco Casas <fcasas at codeweavers.com>
>> ---
>>
>> v3:
>> * No changes.
>>
>> The recursive structure of prepend_input_var_copy() and
>> append_output_var_copy() could be preserved creating additional
>> loads to complete the paths. Otherwise we would be requiring
>> passing whole paths as arguments.
>>
>> These additional loads should be handled by DCE.
>>
>> Still, matrix vectors are copied iteratively instead of recursively now,
>> to avoid the boilerplate of creating new loads in this last step.
>>
>> Signed-off-by: Francisco Casas <fcasas at codeweavers.com>
>> ---
>>   libs/vkd3d-shader/hlsl_codegen.c | 188 +++++++++++++++++++------------
>>   1 file changed, 116 insertions(+), 72 deletions(-)
>>
>> diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
>> index 6437006b..7a245007 100644
>> --- a/libs/vkd3d-shader/hlsl_codegen.c
>> +++ b/libs/vkd3d-shader/hlsl_codegen.c
>> @@ -239,59 +239,75 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir
>>       return ext_var;
>>   }
>>
>> -static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_var *var,
>> -        struct hlsl_type *type, unsigned int field_offset, unsigned int modifiers, const struct hlsl_semantic *semantic)
>> +static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_load *lhs,
>> +        struct hlsl_type *type, unsigned int modifiers, const struct hlsl_semantic *semantic)
> 
> I haven't put a lot of thought into this, but it seems to me we could
> pass a deref instead of a redundant load and be mostly set. Mostly
> because we then need to add the instructions to the instruction list
> in some other way (e.g. by passing an explicit list).

I tried it passing a hlsl_blocks to be initialized (like in the 
component load/store initializers) but in the end it didn't seem like a 
good idea to me:

The problem is that we would need to create derefs in the wild (outside 
hlsl_ir_nodes), which would mean exposing several static functions in 
hlsl.c, like init_deref_from_component_index(). We would also need to 
use "hlsl_cleanup_deref()" which for now is only non-static because it 
is needed in the the pass that translates index paths back to register 
offsets.

So I don't think exposing the deref functions is better than adding 
these redundant loads, which are later deleted by the DCE pass, if I am 
not mistaken.

Alternatively, we could create the nodes just as a container for the 
derefs and immediately free them, but that also seems a little ugly.

So my vote is to keep this signature. Maybe remove the "type" parameter 
since it can be obtained from the deref inside the load node.