On 5/19/20 2:31 PM, Matteo Bruni wrote:
On Mon, May 4, 2020 at 10:04 PM Zebediah Figura z.figura12@gmail.com wrote:
Signed-off-by: Zebediah Figura zfigura@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.