[PATCH vkd3d v3 2/6] vkd3d-shader: Allocate temporary registers for variables.

[Zebediah Figura (she/her)]

On 4/13/21 11:18 PM, Zebediah Figura (she/her) wrote:
> On 4/9/21 1:31 PM, Matteo Bruni wrote:
>> On Fri, Apr 9, 2021 at 6:38 AM Zebediah Figura <zfigura at codeweavers.com> wrote:
>>>
>>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>>> ---
>>>  libs/vkd3d-shader/hlsl.c         |   4 +-
>>>  libs/vkd3d-shader/hlsl.h         |   9 ++
>>>  libs/vkd3d-shader/hlsl_codegen.c | 187 +++++++++++++++++++++++++++++++
>>>  3 files changed, 198 insertions(+), 2 deletions(-)
>>
>> I keep finding nitpicks one at a time...
>>
>>> +static struct hlsl_reg allocate_range(struct liveness *liveness,
>>> +        unsigned int first_write, unsigned int last_read, unsigned int reg_count)
>>> +{
>>> +    const unsigned int component_count = reg_count * 4;
>>> +    unsigned int i, component_idx;
>>> +    struct hlsl_reg ret = {0};
>>> +
>>> +    for (component_idx = 0; component_idx < liveness->size; component_idx += 4)
>>> +    {
>>> +        if (is_range_available(liveness, first_write, component_idx,
>>> +                min(component_count, liveness->size - component_idx)))
>>> +            break;
>>> +    }
>>> +    if (!resize_liveness(liveness, component_idx + component_count))
>>> +        return ret;
>>> +
>>> +    for (i = 0; i < component_count; ++i)
>>> +        liveness->regs[component_idx + i].last_read = last_read;
>>> +    ret.id = component_idx / 4;
>>> +    ret.allocated = true;
>>> +    return ret;
>>> +}
>>
>> This is slightly awkward in that it sets writemask to 0. That is
>> arguably correct (more than 1 full register is allocated, in which
>> case there is no partial register allocation) but we have to be
>> careful with the users of struct hlsl_reg. At the moment there is not
>> much of that, just the TRACE() in allocate_variable_temp_register()
>> that prints something like "r0.". Speaking of which...
>>
> 
> I think I designed it this way on purpose. In a sense it allows us to be
> more careful; I believe I've caught a few errors this way when a
> register had an empty writemask.
> 
> I could have sworn I had fixed those traces, but oh well, I'll add an
> extra patch for those...

Ah, never mind, it seems I had a bad rebase, and confused how I was
keeping track of register size.

> 
>>> +
>>> +static const char *debug_register(char class, struct hlsl_reg reg, const struct hlsl_type *type)
>>> +{
>>> +    if (type->reg_size > 4)
>>> +        return vkd3d_dbg_sprintf("%c%u-%c%u", class, reg.id, class,
>>> +                reg.id + type->reg_size - 1);
>>> +    return vkd3d_dbg_sprintf("%c%u%s", class, reg.id, debug_hlsl_writemask(reg.writemask));
>>> +}
>>> +
>>> +static void allocate_variable_temp_register(struct hlsl_ir_var *var, 
struct liveness *liveness)
>>> +{
>>> +    if (var->is_input_varying || var->is_output_varying || var->is_uniform)
>>> +        return;
>>> +
>>> +    if (!var->reg.allocated && var->last_read)
>>> +    {
>>> +        if (var->data_type->reg_size > 1)
>>> +            var->reg = allocate_range(liveness, var->first_write,
>>> +                    var->last_read, var->data_type->reg_size);
>>> +        else
>>> +            var->reg = allocate_register(liveness, var->first_write,
>>> +                    var->last_read, var->data_type->dimx);
>>> +        TRACE("Allocated %s to %s (liveness %u-%u).\n", var->name,
>>> +                debug_register('r', var->reg, var->data_type), var->first_write, var->last_read);
>>
>> This doesn't tell the whole story for register ranges, maybe we could
>> print more info in that case.
>>
> 
> 


-------------- 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/20210413/dbee2eb5/attachment.sig>