[PATCH 1/5] d3dcompiler: Synthesize a variable when subscripting a non-deref node.

[Zebediah Figura]

On 5/19/20 2:31 PM, Matteo Bruni wrote:
> On Mon, May 4, 2020 at 10:04 PM Zebediah Figura <z.figura12 at gmail.com> wrote:
>>
>> Signed-off-by: Zebediah Figura <zfigura at codeweavers.com>
>> ---
>> There are of course other options to handle this situation, e.g. allowing
>> hlsl_deref to contain either hlsl_ir_var or hlsl_ir_node, or introducing another
>> deref type (e.g. HLSL_IR_DEREF and HLSL_IR_OFFSET).
>>
>> I'm inclined to go with this one, mostly because we don't need to handle
>> multiple cases when dealing with derefs (or another case when dealing with node
>> types).
> 
> First of all, sorry for the late review...
> 
>>
>>  dlls/d3dcompiler_43/hlsl.y | 68 +++++++++++++++++++++++++++++++++++---
>>  1 file changed, 64 insertions(+), 4 deletions(-)
>>
>> diff --git a/dlls/d3dcompiler_43/hlsl.y b/dlls/d3dcompiler_43/hlsl.y
>> index db305361738..01b210419c8 100644
>> --- a/dlls/d3dcompiler_43/hlsl.y
>> +++ b/dlls/d3dcompiler_43/hlsl.y
>> @@ -125,6 +125,11 @@ static void check_invalid_matrix_modifiers(DWORD modifiers, struct source_locati
>>      }
>>  }
>>
>> +static BOOL type_is_single_reg(const struct hlsl_type *type)
>> +{
>> +    return type->type == HLSL_CLASS_SCALAR || type->type == HLSL_CLASS_VECTOR;
> 
> In theory there are more possible cases (e.g. matrix with suitable
> dimensions and majority) although I'm not sure you want to handle
> those in the same way.

Yeah, that thought did occur to me. I decided to leave it alone for now,
though making this a separate function does help give us the freedom to
change it later.

> 
>> +}
>> +
>>  static BOOL declare_variable(struct hlsl_ir_var *decl, BOOL local)
>>  {
>>      BOOL ret;
>> @@ -507,6 +512,41 @@ static struct hlsl_ir_jump *new_return(struct hlsl_ir_node *value, struct source
>>      return jump;
>>  }
>>
>> +static struct hlsl_ir_var *new_synthetic_var(const char *name, struct hlsl_type *type,
>> +        const struct source_location loc)
>> +{
>> +    struct hlsl_ir_var *var;
>> +
>> +    if (!(var = d3dcompiler_alloc(sizeof(*var))))
>> +    {
>> +        hlsl_ctx.status = PARSE_ERR;
>> +        return NULL;
>> +    }
>> +
>> +    var->name = strdup(name);
>> +    var->data_type = type;
>> +    var->loc = loc;
>> +    list_add_tail(&hlsl_ctx.globals->vars, &var->scope_entry);
>> +    return var;
>> +}
>> +
>> +static struct hlsl_ir_assignment *make_simple_assignment(struct hlsl_ir_var *lhs, struct hlsl_ir_node *rhs)
>> +{
>> +    struct hlsl_ir_assignment *assign;
>> +
>> +    if (!(assign = d3dcompiler_alloc(sizeof(*assign))))
>> +        return NULL;
>> +
>> +    init_node(&assign->node, HLSL_IR_ASSIGNMENT, rhs->data_type, rhs->loc);
>> +    assign->lhs.type = HLSL_IR_DEREF_VAR;
>> +    assign->lhs.v.var = lhs;
>> +    assign->rhs = rhs;
>> +    if (type_is_single_reg(lhs->data_type))
>> +        assign->writemask = (1 << lhs->data_type->dimx) - 1;
>> +
>> +    return assign;
>> +}
>> +
>>  static struct hlsl_ir_deref *new_var_deref(struct hlsl_ir_var *var, const struct source_location loc)
>>  {
>>      struct hlsl_ir_deref *deref = d3dcompiler_alloc(sizeof(*deref));
>> @@ -525,13 +565,33 @@ static struct hlsl_ir_deref *new_var_deref(struct hlsl_ir_var *var, const struct
>>  static struct hlsl_ir_deref *new_record_deref(struct hlsl_ir_node *record,
>>          struct hlsl_struct_field *field, const struct source_location loc)
>>  {
>> -    struct hlsl_ir_deref *deref = d3dcompiler_alloc(sizeof(*deref));
>> +    struct hlsl_ir_deref *deref;
>>
>> -    if (!deref)
>> +    if (record->type != HLSL_IR_DEREF)
>>      {
>> -        ERR("Out of memory.\n");
>> -        return NULL;
>> +        struct hlsl_ir_assignment *assign;
>> +        struct hlsl_ir_var *var;
>> +        char name[27];
>> +
>> +        sprintf(name, "<deref-%p>", record);
>> +        if (!(var = new_synthetic_var(name, record->data_type, record->loc)))
>> +            return NULL;
>> +
>> +        TRACE("Synthesized variable %p for %s node.\n", var, debug_node_type(record->type));
>> +
>> +        if (!(assign = make_simple_assignment(var, record)))
>> +            return NULL;
>> +        list_add_after(&record->entry, &assign->node.entry);
>> +
>> +        if (!(deref = new_var_deref(var, var->loc)))
>> +            return NULL;
>> +        list_add_after(&assign->node.entry, &deref->node.entry);
>> +        record = &deref->node;
>>      }
>> +
>> +    if (!(deref = d3dcompiler_alloc(sizeof(*deref))))
>> +        return NULL;
>> +
>>      init_node(&deref->node, HLSL_IR_DEREF, field->type, loc);
>>      deref->src.type = HLSL_IR_DEREF_RECORD;
>>      deref->src.v.record.record = record;
> 
> This looks fine to me. It would have been nicer if we had a test
> exercising this new path already in the test suite... but I don't know
> that the parser currently supports any of those.
> 

I think the only way to get this is by returning a struct from a
function, unless there's something I'm missing. But I'll certainly put
it on the list of tests that need adding.