[PATCH 0/10] MR7: vkd3d-shader/hlsl: Translate copy propagation to index paths. Check for non-static object references. Implicit array initialization.
Copy propagation is translated to index paths, invalidation of variable components is made more precise. This, together with checking for non-static object references (which are not allowed in HLSL) allows to set object register sizes back to zero without overlapping registers. I also included implicit array initialization, since, if I remember correctly, what was holding it back was that structs/arrays with object components couldn't be represented with the correct register offsets. After this, we have to decide if (and how to) move register allocation and register offset calculation to each shader model. -- https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
From: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- Otherwise we trigger the assertion with the tests in the next patch. --- libs/vkd3d-shader/hlsl_sm4.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c index cdd73e57..fab2abe2 100644 --- a/libs/vkd3d-shader/hlsl_sm4.c +++ b/libs/vkd3d-shader/hlsl_sm4.c @@ -2082,11 +2082,23 @@ static void write_sm4_resource_load(struct hlsl_ctx *ctx, const struct hlsl_ir_node *texel_offset = load->texel_offset.node; const struct hlsl_ir_node *coords = load->coords.node; + if (resource_type->type != HLSL_CLASS_OBJECT) + { + assert(resource_type->type == HLSL_CLASS_ARRAY || resource_type->type == HLSL_CLASS_STRUCT); + hlsl_fixme(ctx, &load->node.loc, "Resource being a component of another variable."); + return; + } + if (load->sampler.var) { const struct hlsl_type *sampler_type = load->sampler.var->data_type; - assert(sampler_type->type == HLSL_CLASS_OBJECT); + if (sampler_type->type != HLSL_CLASS_OBJECT) + { + assert(sampler_type->type == HLSL_CLASS_ARRAY || sampler_type->type == HLSL_CLASS_STRUCT); + hlsl_fixme(ctx, &load->node.loc, "Sampler being a component of another variable."); + return; + } assert(sampler_type->base_type == HLSL_TYPE_SAMPLER); assert(sampler_type->sampler_dim == HLSL_SAMPLER_DIM_GENERIC); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
From: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- Makefile.am | 1 + tests/object-references.shader_test | 132 ++++++++++++++++++++++++++++ 2 files changed, 133 insertions(+) create mode 100644 tests/object-references.shader_test diff --git a/Makefile.am b/Makefile.am index 60db7794..962036ff 100644 --- a/Makefile.am +++ b/Makefile.am @@ -115,6 +115,7 @@ vkd3d_shader_tests = \ tests/matrix-semantics.shader_test \ tests/multiple-rt.shader_test \ tests/nointerpolation.shader_test \ + tests/object-references.shader_test tests/pow.shader_test \ tests/preproc-if.shader_test \ tests/preproc-ifdef.shader_test \ diff --git a/tests/object-references.shader_test b/tests/object-references.shader_test new file mode 100644 index 00000000..a9fbbc50 --- /dev/null +++ b/tests/object-references.shader_test @@ -0,0 +1,132 @@ +[require] +shader model >= 4.0 + +[texture 0] +size (2, 2) +0.1 0.2 0.3 0.4 0.5 0.7 0.6 0.8 +0.6 0.5 0.2 0.1 0.8 0.0 0.7 1.0 + +[pixel shader] +Texture2D t; + +struct foo { + int3 a; + Texture2D b; +}; + +float4 main(float4 pos : sv_position) : sv_target +{ + struct foo q; + + q.a = int3(pos.xy, 0); + q.b = t; + return q.b.Load(q.a); +} + +[test] +draw quad +probe (0, 0) rgba (0.1, 0.2, 0.3, 0.4) +probe (1, 0) rgba (0.5, 0.7, 0.6, 0.8) +probe (0, 1) rgba (0.6, 0.5, 0.2, 0.1) +probe (1, 1) rgba (0.8, 0.0, 0.7, 1.0) + + +[texture 0] +size (1, 1) +1.0 1.0 1.0 1.0 + +[texture 1] +size (1, 1) +2.0 2.0 2.0 1.0 + +[texture 2] +size (1, 1) +3.0 3.0 3.0 1.0 + +[pixel shader] +Texture2D tex[3]; + +struct foo { + float4 p; + Texture2D t; +}; + +float4 main() : sv_target +{ + struct foo s[3]; + + s[0].t = tex[0]; + s[1].t = tex[1]; + s[2].t = tex[2]; + return 100 * s[2].t.Load(0) + 10 * s[0].t.Load(0) + s[1].t.Load(0); +} + +[test] +todo draw quad +todo probe all rgba (312, 312, 312, 111) + + +[texture 0] +size (1, 1) +1.0 1.0 1.0 1.0 + +[texture 1] +size (1, 1) +2.0 2.0 2.0 1.0 + +[texture 2] +size (1, 1) +3.0 3.0 3.0 1.0 + +[pixel shader] +Texture2D tex1; +Texture2D tex2; +Texture2D tex3; + +float4 main() : sv_target +{ + Texture2D t[3][2]; + + t[0][0] = tex1; // Note: Only invalid in shader model 5.1, array ref. cannot be used as l-value. + t[0][1] = tex2; + t[1][0] = tex3; + t[1][1] = tex1; + t[2][0] = tex2; + t[2][1] = tex3; + return 1000 * t[2][0].Load(0) + 100 * t[1][1].Load(0) + 10 * t[2][1].Load(0) + t[0][1].Load(0); +} + +[test] +draw quad +probe all rgba (2132, 2132, 2132, 1111) + + +[pixel shader fail todo] +Texture2D tex[3]; +uniform int n; + +struct foo { + float4 p; + Texture2D t; +}; + +float4 main() : sv_target +{ + struct foo s[3]; + + s[0].t = tex[0]; + s[1].t = tex[1]; + s[2].t = tex[2]; + return s[n].t.Load(0); +} + + +[pixel shader fail todo] +// Note: Only valid in shader model 5.1 +Texture2D tex[3]; +uniform int n; + +float4 main() : sv_target +{ + return tex[n].Load(0); +} -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
On 8/11/22 15:26, Francisco Casas wrote:
+[texture 0] +size (1, 1) +1.0 1.0 1.0 1.0 + +[texture 1] +size (1, 1) +2.0 2.0 2.0 1.0 + +[texture 2] +size (1, 1) +3.0 3.0 3.0 1.0 + +[pixel shader] +Texture2D tex[3]; + +struct foo { + float4 p; + Texture2D t; +}; + +float4 main() : sv_target +{ + struct foo s[3]; + + s[0].t = tex[0]; + s[1].t = tex[1]; + s[2].t = tex[2]; + return 100 * s[2].t.Load(0) + 10 * s[0].t.Load(0) + s[1].t.Load(0); +} + +[test] +todo draw quad +todo probe all rgba (312, 312, 312, 111) + + +[texture 0] +size (1, 1) +1.0 1.0 1.0 1.0 + +[texture 1] +size (1, 1) +2.0 2.0 2.0 1.0 + +[texture 2] +size (1, 1) +3.0 3.0 3.0 1.0 +
You don't need to redeclare the textures if they don't change.
From: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 17 ++++++++++------- tests/hlsl-initializer-objects.shader_test | 4 ++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index a47e3d2d..f076f9cf 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -558,15 +558,18 @@ unsigned int hlsl_type_component_count(const struct hlsl_type *type) { return hlsl_type_component_count(type->e.array.type) * type->e.array.elements_count; } - if (type->type != HLSL_CLASS_STRUCT) + if (type->type == HLSL_CLASS_OBJECT) { - ERR("Unexpected data type %#x.\n", type->type); - return 0; + return 1; } - - for (i = 0; i < type->e.record.field_count; ++i) - count += hlsl_type_component_count(type->e.record.fields[i].type); - return count; + if (type->type == HLSL_CLASS_STRUCT) + { + for (i = 0; i < type->e.record.field_count; ++i) + count += hlsl_type_component_count(type->e.record.fields[i].type); + return count; + } + assert(0); + return 0; } bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2) diff --git a/tests/hlsl-initializer-objects.shader_test b/tests/hlsl-initializer-objects.shader_test index 2306d07f..70f060c2 100644 --- a/tests/hlsl-initializer-objects.shader_test +++ b/tests/hlsl-initializer-objects.shader_test @@ -71,7 +71,7 @@ float4 main() : sv_target } -[pixel shader fail todo] +[pixel shader fail] Texture2D tex; struct foo @@ -90,7 +90,7 @@ float4 main() : sv_target } -[pixel shader fail todo] +[pixel shader fail] Texture2D tex; struct foo -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
On 8/11/22 15:26, Francisco Casas wrote:
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index a47e3d2d..f076f9cf 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -558,15 +558,18 @@ unsigned int hlsl_type_component_count(const struct hlsl_type *type) { return hlsl_type_component_count(type->e.array.type) * type->e.array.elements_count; } - if (type->type != HLSL_CLASS_STRUCT) + if (type->type == HLSL_CLASS_OBJECT) { - ERR("Unexpected data type %#x.\n", type->type); - return 0; + return 1; } - - for (i = 0; i < type->e.record.field_count; ++i) - count += hlsl_type_component_count(type->e.record.fields[i].type); - return count; + if (type->type == HLSL_CLASS_STRUCT) + { + for (i = 0; i < type->e.record.field_count; ++i) + count += hlsl_type_component_count(type->e.record.fields[i].type); + return count; + } + assert(0); + return 0; }
This function is ripe for a switch statement.
From: Francisco Casas <fcasas(a)codeweavers.com> Otherwise we get false in implicit_compatible_data_types() when passing types that are equal but not convertible according to convertible_data_type(); e.g. getting: "Can't implicitly convert from Texture2D<float4> to Texture2D<float4>." Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.y | 3 +++ tests/hlsl-initializer-objects.shader_test | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 7df7afb9..1e488afa 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -370,6 +370,9 @@ static struct hlsl_ir_node *add_implicit_conversion(struct hlsl_ctx *ctx, struct { struct hlsl_type *src_type = node->data_type; + if (hlsl_types_are_equal(src_type, dst_type)) + return node; + if (!implicit_compatible_data_types(src_type, dst_type)) { struct vkd3d_string_buffer *src_string, *dst_string; diff --git a/tests/hlsl-initializer-objects.shader_test b/tests/hlsl-initializer-objects.shader_test index 70f060c2..dcb60029 100644 --- a/tests/hlsl-initializer-objects.shader_test +++ b/tests/hlsl-initializer-objects.shader_test @@ -25,8 +25,8 @@ float4 main() : sv_target } [test] -todo draw quad -todo probe all rgba (0.2, 0.2, 0.2, 0.1) +draw quad +probe all rgba (0.2, 0.2, 0.2, 0.1) [pixel shader] -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
From: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index f076f9cf..cb04d33e 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1544,6 +1544,10 @@ static void dump_ir_constant(struct vkd3d_string_buffer *buffer, const struct hl vkd3d_string_buffer_printf(buffer, "%.8e ", value->f); break; + case HLSL_TYPE_HALF: + vkd3d_string_buffer_printf(buffer, "%.4e ", value->f); + break; + case HLSL_TYPE_INT: vkd3d_string_buffer_printf(buffer, "%d ", value->i); break; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
On 8/11/22 15:26, Francisco Casas wrote:
diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index f076f9cf..cb04d33e 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -1544,6 +1544,10 @@ static void dump_ir_constant(struct vkd3d_string_buffer *buffer, const struct hl vkd3d_string_buffer_printf(buffer, "%.8e ", value->f); break;
+ case HLSL_TYPE_HALF: + vkd3d_string_buffer_printf(buffer, "%.4e ", value->f); + break; + case HLSL_TYPE_INT: vkd3d_string_buffer_printf(buffer, "%d ", value->i); break;
The full 32 bits of precision are encoded into the shader, so I think it makes more sense just to treat halves identically to floats here (and in almost all places, really.)
From: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- It was necessary to add ``` while (transform_ir(ctx, hlsl_fold_constant_exprs, body, NULL)); ``` just after transforming deref paths back into offsets, so that the constant register offset expressions could be folded after the constant folding passes. We should consider precomputing hlsl_type_component_count() in hlsl_type, the same way we currently do with reg_size. Given the many places where this function is called. In several places I renamed "offset" to either "comp" or "start", depending on the context. I did this to avoid confusion with the register offsets, but it could be arged that the original name is correct, since it can be interpreted as "offset, measured in components" now. --- libs/vkd3d-shader/hlsl.c | 6 +- libs/vkd3d-shader/hlsl.h | 3 + libs/vkd3d-shader/hlsl_codegen.c | 178 ++++++++++++++++++++++--------- 3 files changed, 135 insertions(+), 52 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index cb04d33e..23fe06b0 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -766,7 +766,7 @@ static bool type_is_single_reg(const struct hlsl_type *type) return type->type == HLSL_CLASS_SCALAR || type->type == HLSL_CLASS_VECTOR; } -static bool copy_deref(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct hlsl_deref *other) +bool hlsl_copy_deref(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct hlsl_deref *other) { unsigned int i; @@ -1032,8 +1032,8 @@ struct hlsl_ir_resource_load *hlsl_new_resource_load(struct hlsl_ctx *ctx, struc return NULL; init_node(&load->node, HLSL_IR_RESOURCE_LOAD, data_type, *loc); load->load_type = type; - copy_deref(ctx, &load->resource, resource); - copy_deref(ctx, &load->sampler, sampler); + hlsl_copy_deref(ctx, &load->resource, resource); + hlsl_copy_deref(ctx, &load->sampler, sampler); hlsl_src_from_node(&load->coords, coords); hlsl_src_from_node(&load->texel_offset, texel_offset); return load; diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index 7970665c..f4001e91 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -721,6 +721,7 @@ void hlsl_dump_function(struct hlsl_ctx *ctx, const struct hlsl_ir_function_decl int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry_func, enum vkd3d_shader_target_type target_type, struct vkd3d_shader_code *out); +bool hlsl_copy_deref(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct hlsl_deref *other); void hlsl_cleanup_deref(struct hlsl_deref *deref); void hlsl_replace_node(struct hlsl_ir_node *old, struct hlsl_ir_node *new); @@ -821,6 +822,8 @@ unsigned int hlsl_combine_writemasks(unsigned int first, unsigned int second); unsigned int hlsl_map_swizzle(unsigned int swizzle, unsigned int writemask); unsigned int hlsl_swizzle_from_writemask(unsigned int writemask); +bool hlsl_component_index_range_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, + unsigned int *start, unsigned int *count); bool hlsl_offset_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, unsigned int *offset); unsigned int hlsl_offset_from_deref_safe(struct hlsl_ctx *ctx, const struct hlsl_deref *deref); struct hlsl_reg hlsl_reg_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref); diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 36a417e5..e20a0c05 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -541,7 +541,7 @@ static void copy_propagation_var_def_destroy(struct rb_entry *entry, void *conte } static struct copy_propagation_value *copy_propagation_get_value(const struct copy_propagation_state *state, - const struct hlsl_ir_var *var, unsigned component) + const struct hlsl_ir_var *var, unsigned int component) { for (; state; state = state->parent) { @@ -549,9 +549,11 @@ static struct copy_propagation_value *copy_propagation_get_value(const struct co if (entry) { struct copy_propagation_var_def *var_def = RB_ENTRY_VALUE(entry, struct copy_propagation_var_def, entry); - enum copy_propagation_value_state state = var_def->values[component].state; + unsigned int component_count = hlsl_type_component_count(var->data_type); + enum copy_propagation_value_state state; - assert(component < var_def->var->data_type->reg_size); + assert(component < component_count); + state = var_def->values[component].state; switch (state) { @@ -573,12 +575,13 @@ static struct copy_propagation_var_def *copy_propagation_create_var_def(struct h { struct rb_entry *entry = rb_get(&state->var_defs, var); struct copy_propagation_var_def *var_def; + unsigned int component_count = hlsl_type_component_count(var->data_type); int res; if (entry) return RB_ENTRY_VALUE(entry, struct copy_propagation_var_def, entry); - if (!(var_def = hlsl_alloc(ctx, offsetof(struct copy_propagation_var_def, values[var->data_type->reg_size])))) + if (!(var_def = hlsl_alloc(ctx, offsetof(struct copy_propagation_var_def, values[component_count])))) return NULL; var_def->var = var; @@ -590,31 +593,32 @@ static struct copy_propagation_var_def *copy_propagation_create_var_def(struct h } static void copy_propagation_invalidate_variable(struct copy_propagation_var_def *var_def, - unsigned offset, unsigned char writemask) + unsigned int comp, unsigned char writemask) { unsigned i; - TRACE("Invalidate variable %s[%u]%s.\n", var_def->var->name, offset, debug_hlsl_writemask(writemask)); + TRACE("Invalidate variable %s[%u]%s.\n", var_def->var->name, comp, debug_hlsl_writemask(writemask)); for (i = 0; i < 4; ++i) { if (writemask & (1u << i)) - var_def->values[offset + i].state = VALUE_STATE_DYNAMICALLY_WRITTEN; + var_def->values[comp + i].state = VALUE_STATE_DYNAMICALLY_WRITTEN; } } static void copy_propagation_invalidate_whole_variable(struct copy_propagation_var_def *var_def) { + unsigned int component_count = hlsl_type_component_count(var_def->var->data_type); unsigned i; TRACE("Invalidate variable %s.\n", var_def->var->name); - for (i = 0; i < var_def->var->data_type->reg_size; ++i) + for (i = 0; i < component_count; ++i) var_def->values[i].state = VALUE_STATE_DYNAMICALLY_WRITTEN; } -static void copy_propagation_set_value(struct copy_propagation_var_def *var_def, unsigned int offset, - unsigned char writemask, struct hlsl_ir_node *node) +static void copy_propagation_set_value(struct copy_propagation_var_def *var_def, unsigned int comp, + unsigned char writemask, struct hlsl_ir_node *instr) { unsigned int i, j = 0; @@ -623,59 +627,56 @@ static void copy_propagation_set_value(struct copy_propagation_var_def *var_def, if (writemask & (1u << i)) { TRACE("Variable %s[%u] is written by instruction %p%s.\n", - var_def->var->name, offset + i, node, debug_hlsl_writemask(1u << i)); - var_def->values[offset + i].state = VALUE_STATE_STATICALLY_WRITTEN; - var_def->values[offset + i].node = node; - var_def->values[offset + i].component = j++; + var_def->var->name, comp + i, instr, debug_hlsl_writemask(1u << i)); + var_def->values[comp + i].state = VALUE_STATE_STATICALLY_WRITTEN; + var_def->values[comp + i].node = instr; + var_def->values[comp + i].component = j++; } } } static struct hlsl_ir_node *copy_propagation_compute_replacement(struct hlsl_ctx *ctx, const struct copy_propagation_state *state, const struct hlsl_deref *deref, - unsigned int count, unsigned int *swizzle) + unsigned int *swizzle) { const struct hlsl_ir_var *var = deref->var; - struct hlsl_ir_node *node = NULL; - unsigned int offset, i; + struct hlsl_ir_node *instr = NULL; + unsigned int start, count, i; - if (!hlsl_offset_from_deref(ctx, deref, &offset)) + if (!hlsl_component_index_range_from_deref(ctx, deref, &start, &count)) return NULL; - if (var->data_type->type != HLSL_CLASS_OBJECT) - assert(offset + count <= var->data_type->reg_size); - *swizzle = 0; for (i = 0; i < count; ++i) { - struct copy_propagation_value *value = copy_propagation_get_value(state, var, offset + i); + struct copy_propagation_value *value = copy_propagation_get_value(state, var, start + i); if (!value) return NULL; - if (!node) + if (!instr) { - node = value->node; + instr = value->node; } - else if (node != value->node) + else if (instr != value->node) { - TRACE("No single source for propagating load from %s[%u-%u].\n", var->name, offset, offset + count); + TRACE("No single source for propagating load from %s[%u-%u].\n", var->name, start, start + count); return NULL; } *swizzle |= value->component << i * 2; } TRACE("Load from %s[%u-%u] propagated as instruction %p%s.\n", - var->name, offset, offset + count, node, debug_hlsl_swizzle(*swizzle, count)); - return node; + var->name, start, start + count, instr, debug_hlsl_swizzle(*swizzle, count)); + return instr; } static bool copy_propagation_transform_load(struct hlsl_ctx *ctx, struct hlsl_ir_load *load, struct copy_propagation_state *state) { - struct hlsl_ir_node *node = &load->node, *new_node; - struct hlsl_type *type = node->data_type; + struct hlsl_ir_node *instr = &load->node, *new_instr; + struct hlsl_type *type = instr->data_type; struct hlsl_ir_swizzle *swizzle_node; unsigned int dimx = 0; unsigned int swizzle; @@ -699,17 +700,17 @@ static bool copy_propagation_transform_load(struct hlsl_ctx *ctx, return false; } - if (!(new_node = copy_propagation_compute_replacement(ctx, state, &load->src, dimx, &swizzle))) + if (!(new_instr = copy_propagation_compute_replacement(ctx, state, &load->src, &swizzle))) return false; if (type->type != HLSL_CLASS_OBJECT) { - if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, dimx, new_node, &node->loc))) + if (!(swizzle_node = hlsl_new_swizzle(ctx, swizzle, dimx, new_instr, &instr->loc))) return false; - list_add_before(&node->entry, &swizzle_node->node.entry); - new_node = &swizzle_node->node; + list_add_before(&instr->entry, &swizzle_node->node.entry); + new_instr = &swizzle_node->node; } - hlsl_replace_node(node, new_node); + hlsl_replace_node(instr, new_instr); return true; } @@ -717,17 +718,18 @@ static bool copy_propagation_transform_object_load(struct hlsl_ctx *ctx, struct hlsl_deref *deref, struct copy_propagation_state *state) { struct hlsl_ir_load *load; - struct hlsl_ir_node *node; + struct hlsl_ir_node *instr; unsigned int swizzle; - if (!(node = copy_propagation_compute_replacement(ctx, state, deref, 1, &swizzle))) + if (!(instr = copy_propagation_compute_replacement(ctx, state, deref, &swizzle))) return false; /* Only HLSL_IR_LOAD can produce an object. */ - load = hlsl_ir_load(node); - deref->var = load->src.var; - hlsl_src_remove(&deref->offset); - hlsl_src_from_node(&deref->offset, load->src.offset.node); + load = hlsl_ir_load(instr); + + hlsl_cleanup_deref(deref); + hlsl_copy_deref(ctx, deref, &load->src); + return true; } @@ -748,18 +750,18 @@ static void copy_propagation_record_store(struct hlsl_ctx *ctx, struct hlsl_ir_s struct copy_propagation_var_def *var_def; struct hlsl_deref *lhs = &store->lhs; struct hlsl_ir_var *var = lhs->var; - unsigned int offset; + unsigned int start, count; if (!(var_def = copy_propagation_create_var_def(ctx, state, var))) return; - if (hlsl_offset_from_deref(ctx, lhs, &offset)) + if (hlsl_component_index_range_from_deref(ctx, lhs, &start, &count)) { unsigned int writemask = store->writemask; if (store->rhs.node->data_type->type == HLSL_CLASS_OBJECT) writemask = VKD3DSP_WRITEMASK_0; - copy_propagation_set_value(var_def, offset, writemask, store->rhs.node); + copy_propagation_set_value(var_def, start, writemask, store->rhs.node); } else { @@ -794,13 +796,13 @@ static void copy_propagation_invalidate_from_block(struct hlsl_ctx *ctx, struct struct copy_propagation_var_def *var_def; struct hlsl_deref *lhs = &store->lhs; struct hlsl_ir_var *var = lhs->var; - unsigned int offset; + unsigned int start, count; if (!(var_def = copy_propagation_create_var_def(ctx, state, var))) continue; - if (hlsl_offset_from_deref(ctx, lhs, &offset)) - copy_propagation_invalidate_variable(var_def, offset, store->writemask); + if (hlsl_component_index_range_from_deref(ctx, lhs, &start, &count)) + copy_propagation_invalidate_variable(var_def, start, store->writemask); else copy_propagation_invalidate_whole_variable(var_def); @@ -1996,6 +1998,82 @@ static void allocate_objects(struct hlsl_ctx *ctx, enum hlsl_base_type type) } } +bool hlsl_component_index_range_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, + unsigned int *start, unsigned int *count) +{ + struct hlsl_type *type = deref->var->data_type; + unsigned int i, k; + + *start = 0; + *count = 0; + + for (i = 0; i < deref->path_len; ++i) + { + struct hlsl_ir_node *path_node = deref->path[i].node; + unsigned int idx = 0; + + assert(path_node); + if (path_node->type != HLSL_IR_CONSTANT) + return false; + + /* We should always have generated a cast to UINT. */ + assert(path_node->data_type->type == HLSL_CLASS_SCALAR + && path_node->data_type->base_type == HLSL_TYPE_UINT); + + idx = hlsl_ir_constant(path_node)->value[0].u; + + switch (type->type) + { + case HLSL_CLASS_VECTOR: + if (idx >= type->dimx) + { + hlsl_error(ctx, &path_node->loc, VKD3D_SHADER_ERROR_HLSL_OFFSET_OUT_OF_BOUNDS, + "Vector index is out of bounds. %u/%u", idx, type->dimx); + return false; + } + *start += idx; + break; + + case HLSL_CLASS_MATRIX: + if (idx >= hlsl_type_major_size(type)) + { + hlsl_error(ctx, &path_node->loc, VKD3D_SHADER_ERROR_HLSL_OFFSET_OUT_OF_BOUNDS, + "Matrix index is out of bounds. %u/%u", idx, hlsl_type_major_size(type)); + return false; + } + if (hlsl_type_is_row_major(type)) + *start += idx * type->dimx; + else + *start += idx * type->dimy; + break; + + case HLSL_CLASS_ARRAY: + if (idx >= type->e.array.elements_count) + { + hlsl_error(ctx, &path_node->loc, VKD3D_SHADER_ERROR_HLSL_OFFSET_OUT_OF_BOUNDS, + "Array index is out of bounds. %u/%u", idx, type->e.array.elements_count); + return false; + } + *start += idx * hlsl_type_component_count(type->e.array.type); + break; + + case HLSL_CLASS_STRUCT: + for (k = 0; k < idx; ++k) + *start += hlsl_type_component_count(type->e.record.fields[k].type); + break; + + default: + assert(0); + break; + } + + type = hlsl_get_inner_type_from_path_index(ctx, type, path_node); + } + + *count = hlsl_type_component_count(type); + return true; +} + bool hlsl_offset_from_deref(struct hlsl_ctx *ctx, const struct hlsl_deref *deref, unsigned int *offset) { struct hlsl_ir_node *offset_node = deref->offset.node; @@ -2105,8 +2183,6 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry while (progress); transform_ir(ctx, split_matrix_copies, body, NULL); - transform_ir(ctx, transform_deref_paths_into_offsets, body, NULL); /* TODO: move forward, remove when no longer needed */ - transform_ir(ctx, lower_narrowing_casts, body, NULL); transform_ir(ctx, lower_casts_to_bool, body, NULL); do @@ -2121,6 +2197,10 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry if (ctx->profile->major_version < 4) transform_ir(ctx, lower_division, body, NULL); + /* TODO: move forward, remove when no longer needed */ + transform_ir(ctx, transform_deref_paths_into_offsets, body, NULL); + while (transform_ir(ctx, hlsl_fold_constant_exprs, body, NULL)); + do compute_liveness(ctx, entry_func); while (transform_ir(ctx, dce, body, NULL)); -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
From: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 17 +++++++++ libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-shader/hlsl_codegen.c | 63 ++++++++++++++++++++++++++------ 3 files changed, 69 insertions(+), 12 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 23fe06b0..8b31d021 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -140,6 +140,23 @@ unsigned int hlsl_type_major_size(const struct hlsl_type *type) return type->dimx; } +unsigned int hlsl_type_length(const struct hlsl_type *type) +{ + switch (type->type) + { + case HLSL_CLASS_VECTOR: + return type->dimx; + case HLSL_CLASS_MATRIX: + return hlsl_type_major_size(type); + case HLSL_CLASS_ARRAY: + return type->e.array.elements_count; + case HLSL_CLASS_STRUCT: + return type->e.record.field_count; + default: + return 0; + } +} + static unsigned int get_array_size(const struct hlsl_type *type) { if (type->type == HLSL_CLASS_ARRAY) diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index f4001e91..c60c654b 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -814,6 +814,7 @@ struct hlsl_type *hlsl_type_get_component_type(struct hlsl_ctx *ctx, struct hlsl bool hlsl_type_is_row_major(const struct hlsl_type *type); unsigned int hlsl_type_minor_size(const struct hlsl_type *type); unsigned int hlsl_type_major_size(const struct hlsl_type *type); +unsigned int hlsl_type_length(const struct hlsl_type *type); unsigned int hlsl_type_get_sm4_offset(const struct hlsl_type *type, unsigned int offset); bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2); diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index e20a0c05..eb07ae5c 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -606,15 +606,58 @@ static void copy_propagation_invalidate_variable(struct copy_propagation_var_def } } -static void copy_propagation_invalidate_whole_variable(struct copy_propagation_var_def *var_def) +static void copy_propagation_invalidate_variable_from_deref_recurse(struct hlsl_ctx *ctx, + struct copy_propagation_var_def *var_def, const struct hlsl_deref *deref, + struct hlsl_type *type, unsigned int depth, unsigned int comp_start, unsigned char writemask) { - unsigned int component_count = hlsl_type_component_count(var_def->var->data_type); - unsigned i; + unsigned int i, subtype_comp_count; + struct hlsl_ir_node *path_node; + struct hlsl_type *subtype; - TRACE("Invalidate variable %s.\n", var_def->var->name); + if (depth == deref->path_len) + { + copy_propagation_invalidate_variable(var_def, comp_start, writemask); + return; + } - for (i = 0; i < component_count; ++i) - var_def->values[i].state = VALUE_STATE_DYNAMICALLY_WRITTEN; + path_node = deref->path[depth].node; + subtype = hlsl_get_inner_type_from_path_index(ctx, type, path_node); + + if (type->type == HLSL_CLASS_STRUCT) + { + unsigned int idx = hlsl_ir_constant(path_node)->value[0].u; + + for (i = 0; i < idx; ++i) + comp_start += hlsl_type_component_count(type->e.record.fields[i].type); + + copy_propagation_invalidate_variable_from_deref_recurse(ctx, var_def, deref, subtype, + depth + 1, comp_start, writemask); + } + else + { + subtype_comp_count = hlsl_type_component_count(subtype); + + if (path_node->type == HLSL_IR_CONSTANT) + { + copy_propagation_invalidate_variable_from_deref_recurse(ctx, var_def, deref, subtype, + depth + 1, hlsl_ir_constant(path_node)->value[0].u * subtype_comp_count, writemask); + } + else + { + for (i = 0; i < hlsl_type_length(type); ++i) + { + copy_propagation_invalidate_variable_from_deref_recurse(ctx, var_def, deref, subtype, + depth + 1, i * subtype_comp_count, writemask); + } + } + } +} + +static void copy_propagation_invalidate_variable_from_deref(struct hlsl_ctx *ctx, + struct copy_propagation_var_def *var_def, const struct hlsl_deref *deref, unsigned char writemask) +{ + copy_propagation_invalidate_variable_from_deref_recurse(ctx, var_def, deref, deref->var->data_type, + 0, 0, writemask); } static void copy_propagation_set_value(struct copy_propagation_var_def *var_def, unsigned int comp, @@ -765,7 +808,7 @@ static void copy_propagation_record_store(struct hlsl_ctx *ctx, struct hlsl_ir_s } else { - copy_propagation_invalidate_whole_variable(var_def); + copy_propagation_invalidate_variable_from_deref(ctx, var_def, lhs, store->writemask); } } @@ -796,15 +839,11 @@ static void copy_propagation_invalidate_from_block(struct hlsl_ctx *ctx, struct struct copy_propagation_var_def *var_def; struct hlsl_deref *lhs = &store->lhs; struct hlsl_ir_var *var = lhs->var; - unsigned int start, count; if (!(var_def = copy_propagation_create_var_def(ctx, state, var))) continue; - if (hlsl_component_index_range_from_deref(ctx, lhs, &start, &count)) - copy_propagation_invalidate_variable(var_def, start, store->writemask); - else - copy_propagation_invalidate_whole_variable(var_def); + copy_propagation_invalidate_variable_from_deref(ctx, var_def, lhs, store->writemask); break; } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
On 8/11/22 15:26, Francisco Casas wrote:
From: Francisco Casas <fcasas(a)codeweavers.com>
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 17 +++++++++ libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-shader/hlsl_codegen.c | 63 ++++++++++++++++++++++++++------ 3 files changed, 69 insertions(+), 12 deletions(-)
Nice :D
+unsigned int hlsl_type_length(const struct hlsl_type *type)
"length" feels a bit ambiguous (I thought for a second it might mean "component count"). How about "element count"?
Okay, so, just to confirm and for future reference, at least in the HLSL compiler we are calling: "component": To an "atomic", single type that cannot be indexed, i.e. a scalar or an object[1], within another type, at any nesting level. "element": To one of the outermost parts of another type: * A field within a struct. * An array element within an array. * A column/row within a matrix. * A scalar within a vector. i.e. the resulting type after applying one indexation, going down one nesting level. --- [1] Textures can be indexed in HLSL, but we are handling those cases as a resource_load/resource_store. I am not sure if this would also apply for Shader Model 5.1 resource arrays. On 12-08-22 14:06, Zebediah Figura (she/her) wrote:
On 8/11/22 15:26, Francisco Casas wrote:
From: Francisco Casas <fcasas(a)codeweavers.com>
Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 17 +++++++++ libs/vkd3d-shader/hlsl.h | 1 + libs/vkd3d-shader/hlsl_codegen.c | 63 ++++++++++++++++++++++++++------ 3 files changed, 69 insertions(+), 12 deletions(-)
Nice :D
+unsigned int hlsl_type_length(const struct hlsl_type *type)
"length" feels a bit ambiguous (I thought for a second it might mean "component count"). How about "element count"? _______________________________________________ wine-gitlab mailing list -- wine-gitlab(a)winehq.org To unsubscribe send an email to wine-gitlab-leave(a)winehq.org
On 8/16/22 11:04, Francisco Casas wrote:
Okay, so, just to confirm and for future reference, at least in the HLSL compiler we are calling:
"component": To an "atomic", single type that cannot be indexed, i.e. a scalar or an object[1], within another type, at any nesting level.
"element": To one of the outermost parts of another type: * A field within a struct. * An array element within an array. * A column/row within a matrix. * A scalar within a vector. i.e. the resulting type after applying one indexation, going down one nesting level.
Right, that feels intuitive enough to me. (But we can revisit the terminology if others are having trouble with it.)
---
[1] Textures can be indexed in HLSL, but we are handling those cases as a resource_load/resource_store. I am not sure if this would also apply for Shader Model 5.1 resource arrays.
Yeah, I don't think either one of those should be an array index as far as the IR is concerned.
From: Francisco Casas <fcasas(a)codeweavers.com> It is responsibility of the shader's programmer to ensure that all object references can be solved statically. Resource arrays for ps_5_1 and vs_5_1 are an exception which is not properly handled yet. They probably deserve a different object type. Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl_codegen.c | 30 ++++++++++++++++++++++++ libs/vkd3d-shader/vkd3d_shader_private.h | 1 + tests/object-references.shader_test | 4 ++-- 3 files changed, 33 insertions(+), 2 deletions(-) diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index eb07ae5c..4c843482 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -967,6 +967,34 @@ static bool copy_propagation_execute(struct hlsl_ctx *ctx, struct hlsl_block *bl return progress; } +static bool check_for_non_static_object_references(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, + void *context) +{ + unsigned int start, count; + bool invalid = false; + + if (instr->type == HLSL_IR_RESOURCE_LOAD) + { + struct hlsl_ir_resource_load *load = hlsl_ir_resource_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->resource, &start, &count); + if (load->sampler.var) + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->sampler, &start, &count); + } + else if (instr->type == HLSL_IR_LOAD && instr->data_type->type == HLSL_CLASS_OBJECT) + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->src, &start, &count); + } + + if (invalid) + hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF, + "All object references must be determinable at compile time."); + + return invalid; +} + static bool is_vec1(const struct hlsl_type *type) { return (type->type == HLSL_CLASS_SCALAR) || (type->type == HLSL_CLASS_VECTOR && type->dimx == 1); @@ -2236,6 +2264,8 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry if (ctx->profile->major_version < 4) transform_ir(ctx, lower_division, body, NULL); + transform_ir(ctx, check_for_non_static_object_references, body, NULL); + /* TODO: move forward, remove when no longer needed */ transform_ir(ctx, transform_deref_paths_into_offsets, body, NULL); while (transform_ir(ctx, hlsl_fold_constant_exprs, body, NULL)); diff --git a/libs/vkd3d-shader/vkd3d_shader_private.h b/libs/vkd3d-shader/vkd3d_shader_private.h index 8bde5523..aca5606b 100644 --- a/libs/vkd3d-shader/vkd3d_shader_private.h +++ b/libs/vkd3d-shader/vkd3d_shader_private.h @@ -118,6 +118,7 @@ enum vkd3d_shader_error VKD3D_SHADER_ERROR_HLSL_OFFSET_OUT_OF_BOUNDS = 5019, VKD3D_SHADER_ERROR_HLSL_INCOMPATIBLE_PROFILE = 5020, VKD3D_SHADER_ERROR_HLSL_DIVISION_BY_ZERO = 5021, + VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF = 5022, VKD3D_SHADER_WARNING_HLSL_IMPLICIT_TRUNCATION = 5300, VKD3D_SHADER_WARNING_HLSL_DIVISION_BY_ZERO = 5301, diff --git a/tests/object-references.shader_test b/tests/object-references.shader_test index a9fbbc50..bc36777b 100644 --- a/tests/object-references.shader_test +++ b/tests/object-references.shader_test @@ -101,7 +101,7 @@ draw quad probe all rgba (2132, 2132, 2132, 1111) -[pixel shader fail todo] +[pixel shader fail] Texture2D tex[3]; uniform int n; @@ -121,7 +121,7 @@ float4 main() : sv_target } -[pixel shader fail todo] +[pixel shader fail] // Note: Only valid in shader model 5.1 Texture2D tex[3]; uniform int n; -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
On 8/11/22 15:26, Francisco Casas wrote:
+static bool check_for_non_static_object_references(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, + void *context)
Just a suggestion, I think "validate" would be a good word to use here.
+{ + unsigned int start, count; + bool invalid = false; + + if (instr->type == HLSL_IR_RESOURCE_LOAD) + { + struct hlsl_ir_resource_load *load = hlsl_ir_resource_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->resource, &start, &count); + if (load->sampler.var) + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->sampler, &start, &count); + } + else if (instr->type == HLSL_IR_LOAD && instr->data_type->type == HLSL_CLASS_OBJECT) + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->src, &start, &count);
Do we need this "else if" branch at all? In fact, won't this result in multiple messages if a dynamic dereference is used?
+ } + + if (invalid) + hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF, + "All object references must be determinable at compile time.");
I think it'd be helpful to name the specific expression that can't be resolved (and, along the same lines, to report resources and samplers separately).
+ + return invalid;
Shouldn't this always return false? We don't have any more work to do.
+} +
Hello, On 12-08-22 14:06, Zebediah Figura (she/her) wrote:
On 8/11/22 15:26, Francisco Casas wrote:
+static bool check_for_non_static_object_references(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, + void *context)
Just a suggestion, I think "validate" would be a good word to use here.
Okay, going for 'validate_static_object_references'.
+{ + unsigned int start, count; + bool invalid = false; + + if (instr->type == HLSL_IR_RESOURCE_LOAD) + { + struct hlsl_ir_resource_load *load = hlsl_ir_resource_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->resource, &start, &count); + if (load->sampler.var) + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->sampler, &start, &count); + } + else if (instr->type == HLSL_IR_LOAD && instr->data_type->type == HLSL_CLASS_OBJECT) + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->src, &start, &count);
Do we need this "else if" branch at all? In fact, won't this result in multiple messages if a dynamic dereference is used?
It indeed results in multiple messages. However, I think we may need both branches: On one hand, the native compiler throws an exception on dynamic object references even if they are not used, e.g. with the following ps_5_0 shader: --- uniform int p; Texture2D tex[2]; float4 main() : sv_target { Texture2D u = tex[p]; // Not used, but throws an exception anyways. return float4(0, 1, 2, 3); } --- which makes necessary the "else if" branch (and to do this validation pass before DCE, as we already do). On the other hand, if we delete the "if" branch we would be introducing the assumption that for each resource load there is an object load that will trigger the error (same for samplers), which seems a little hard to remember to maintain this way if we introduce changes in the previous passes. Furthermore, I am not totally sure if it always holds true **now**.
+ } + + if (invalid) + hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF, + "All object references must be determinable at compile time.");
I think it'd be helpful to name the specific expression that can't be resolved (and, along the same lines, to report resources and samplers separately).
Okay. I am thinking on using hlsl_note() after hlsl_error() to report the location of the expression to the user, getting outputs like: --- testm.hlsl:7:20: E5022: Loaded resource must be determinable at compile time. testm.hlsl:7:14: Expression cannot be resolved statically. --- Is this okay? I can also print the instruction pointer as a TRACE() or similar.
+ + return invalid;
Shouldn't this always return false? We don't have any more work to do.
Ah right, I wasn't following that convention. I just thought that having "invalid" as return value could be useful. I will make this function return "false" always.
+} +
wine-gitlab mailing list -- wine-gitlab(a)winehq.org To unsubscribe send an email to wine-gitlab-leave(a)winehq.org
On 8/16/22 16:13, Francisco Casas wrote:
+{ + unsigned int start, count; + bool invalid = false; + + if (instr->type == HLSL_IR_RESOURCE_LOAD) + { + struct hlsl_ir_resource_load *load = hlsl_ir_resource_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->resource, &start, &count); + if (load->sampler.var) + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->sampler, &start, &count); + } + else if (instr->type == HLSL_IR_LOAD && instr->data_type->type == HLSL_CLASS_OBJECT) + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->src, &start, &count);
Do we need this "else if" branch at all? In fact, won't this result in multiple messages if a dynamic dereference is used?
It indeed results in multiple messages. However, I think we may need both branches:
On one hand, the native compiler throws an exception on dynamic object references even if they are not used, e.g. with the following ps_5_0 shader: --- uniform int p; Texture2D tex[2];
float4 main() : sv_target { Texture2D u = tex[p]; // Not used, but throws an exception anyways.
return float4(0, 1, 2, 3); } --- which makes necessary the "else if" branch (and to do this validation pass before DCE, as we already do).
On the other hand, if we delete the "if" branch we would be introducing the assumption that for each resource load there is an object load that will trigger the error (same for samplers), which seems a little hard to remember to maintain this way if we introduce changes in the previous passes. Furthermore, I am not totally sure if it always holds true **now**.
Urgh, that is awkward. I don't think it should block this patch (or maybe it should block this patch but not this series? Can we skip this one and keep the relevant tests todo?) but I also don't like the idea of generating duplicate error lines. One obvious way to avoid that would be to do constant folding at parse time (where necessary) and then just generate error messages at parse time. We probably want that anyway in some places, given things like hlsl-array-dimension.shader_test. That said, interesting fact: if you replace the line Texture2D u = tex[p]; with tex[p]; the shader does compile. (As an interesting footnote, you can get a similar effect with expressions like "1 / 0".) Which almost suggests that we could DCE everything but stores, and then warn as-is...
+ } + + if (invalid) + hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF, + "All object references must be determinable at compile time.");
I think it'd be helpful to name the specific expression that can't be resolved (and, along the same lines, to report resources and samplers separately).
Okay. I am thinking on using hlsl_note() after hlsl_error() to report the location of the expression to the user, getting outputs like: --- testm.hlsl:7:20: E5022: Loaded resource must be determinable at compile time. testm.hlsl:7:14: Expression cannot be resolved statically. --- Is this okay?
I can also print the instruction pointer as a TRACE() or similar.
I think my comment as-is made no sense or was miswritten; we already print the load location. (Is there another location we should be printing?) What I was probably intending was that we should print the variable name; I think that would make sense. We shouldn't need a separate hlsl_note() to do so though.
+ + return invalid;
Shouldn't this always return false? We don't have any more work to do.
Ah right, I wasn't following that convention. I just thought that having "invalid" as return value could be useful. I will make this function return "false" always.
Right. We're not running this function in a loop (or using the return value at all) but I think it makes sense to be consistent about the return value's usage.
On 16-08-22 20:14, Zebediah Figura (she/her) wrote:
On 8/16/22 16:13, Francisco Casas wrote:
+{ + unsigned int start, count; + bool invalid = false; + + if (instr->type == HLSL_IR_RESOURCE_LOAD) + { + struct hlsl_ir_resource_load *load = hlsl_ir_resource_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->resource, &start, &count); + if (load->sampler.var) + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->sampler, &start, &count); + } + else if (instr->type == HLSL_IR_LOAD && instr->data_type->type == HLSL_CLASS_OBJECT) + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->src, &start, &count);
Do we need this "else if" branch at all? In fact, won't this result in multiple messages if a dynamic dereference is used?
It indeed results in multiple messages. However, I think we may need both branches:
On one hand, the native compiler throws an exception on dynamic object references even if they are not used, e.g. with the following ps_5_0 shader: --- uniform int p; Texture2D tex[2];
float4 main() : sv_target { Texture2D u = tex[p]; // Not used, but throws an exception anyways.
return float4(0, 1, 2, 3); } --- which makes necessary the "else if" branch (and to do this validation pass before DCE, as we already do).
On the other hand, if we delete the "if" branch we would be introducing the assumption that for each resource load there is an object load that will trigger the error (same for samplers), which seems a little hard to remember to maintain this way if we introduce changes in the previous passes. Furthermore, I am not totally sure if it always holds true **now**.
Urgh, that is awkward.
I don't think it should block this patch (or maybe it should block this patch but not this series? Can we skip this one and keep the relevant tests todo?) but I also don't like the idea of generating duplicate error lines.
One obvious way to avoid that would be to do constant folding at parse time (where necessary) and then just generate error messages at parse time. We probably want that anyway in some places, given things like hlsl-array-dimension.shader_test.
That said, interesting fact: if you replace the line
Texture2D u = tex[p];
with
tex[p];
the shader does compile. (As an interesting footnote, you can get a similar effect with expressions like "1 / 0".)
Which almost suggests that we could DCE everything but stores, and then warn as-is...
I am not sure we should allow stores to survive DCE, for instance, if the store is not used, but it is static: --- Texture2D tex[2]; float4 main() : sv_target { Texture2D u = tex[0]; // Store is deleted. return float4(0, 1, 2, 3); } --- the shader compiles in native, and ignores the store: --- ps_3_0 def c0, 0, 1, 2, 3 mov oC0, c0 --- After some thinking, I think the following idea is worth trying: Moving the "else if" branch to dce, and only for nodes that are being deleted (because they are not used in a resource_load). ... Btw, I haven't think this too much yet, but, if the resource_load·s pointed to load nodes (both in resource and sampler) instead of having direct derefs, this validation pass with only the "else if" branch should be enough. However, if we consider this change seriously, I think it will be better to introduce it after this patch series.
+ } + + if (invalid) + hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF, + "All object references must be determinable at compile time.");
I think it'd be helpful to name the specific expression that can't be resolved (and, along the same lines, to report resources and samplers separately).
Okay. I am thinking on using hlsl_note() after hlsl_error() to report the location of the expression to the user, getting outputs like: --- testm.hlsl:7:20: E5022: Loaded resource must be determinable at compile time. testm.hlsl:7:14: Expression cannot be resolved statically. --- Is this okay?
I can also print the instruction pointer as a TRACE() or similar.
I think my comment as-is made no sense or was miswritten; we already print the load location. (Is there another location we should be printing?) What I was probably intending was that we should print the variable name; I think that would make sense. We shouldn't need a separate hlsl_note() to do so though.
I thought you were referring to the location of the specific node (or nodes) in the index path that are not constant. But yes, I think that the load location is enough. I will add the variable name in the message.
+ + return invalid;
Shouldn't this always return false? We don't have any more work to do.
Ah right, I wasn't following that convention. I just thought that having "invalid" as return value could be useful. I will make this function return "false" always.
Right. We're not running this function in a loop (or using the return value at all) but I think it makes sense to be consistent about the return value's usage. _______________________________________________ wine-gitlab mailing list -- wine-gitlab(a)winehq.org To unsubscribe send an email to wine-gitlab-leave(a)winehq.org
On 8/17/22 15:21, Francisco Casas wrote:
On 16-08-22 20:14, Zebediah Figura (she/her) wrote:
On 8/16/22 16:13, Francisco Casas wrote:
+{ + unsigned int start, count; + bool invalid = false; + + if (instr->type == HLSL_IR_RESOURCE_LOAD) + { + struct hlsl_ir_resource_load *load = hlsl_ir_resource_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->resource, &start, &count); + if (load->sampler.var) + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->sampler, &start, &count); + } + else if (instr->type == HLSL_IR_LOAD && instr->data_type->type == HLSL_CLASS_OBJECT) + { + struct hlsl_ir_load *load = hlsl_ir_load(instr); + + invalid |= !hlsl_component_index_range_from_deref(ctx, &load->src, &start, &count);
Do we need this "else if" branch at all? In fact, won't this result in multiple messages if a dynamic dereference is used?
It indeed results in multiple messages. However, I think we may need both branches:
On one hand, the native compiler throws an exception on dynamic object references even if they are not used, e.g. with the following ps_5_0 shader: --- uniform int p; Texture2D tex[2];
float4 main() : sv_target { Texture2D u = tex[p]; // Not used, but throws an exception anyways.
return float4(0, 1, 2, 3); } --- which makes necessary the "else if" branch (and to do this validation pass before DCE, as we already do).
On the other hand, if we delete the "if" branch we would be introducing the assumption that for each resource load there is an object load that will trigger the error (same for samplers), which seems a little hard to remember to maintain this way if we introduce changes in the previous passes. Furthermore, I am not totally sure if it always holds true **now**.
Urgh, that is awkward.
I don't think it should block this patch (or maybe it should block this patch but not this series? Can we skip this one and keep the relevant tests todo?) but I also don't like the idea of generating duplicate error lines.
One obvious way to avoid that would be to do constant folding at parse time (where necessary) and then just generate error messages at parse time. We probably want that anyway in some places, given things like hlsl-array-dimension.shader_test.
That said, interesting fact: if you replace the line
Texture2D u = tex[p];
with
tex[p];
the shader does compile. (As an interesting footnote, you can get a similar effect with expressions like "1 / 0".)
Which almost suggests that we could DCE everything but stores, and then warn as-is...
I am not sure we should allow stores to survive DCE, for instance, if the store is not used, but it is static:
--- Texture2D tex[2];
float4 main() : sv_target { Texture2D u = tex[0]; // Store is deleted.
return float4(0, 1, 2, 3); } ---
the shader compiles in native, and ignores the store:
--- ps_3_0 def c0, 0, 1, 2, 3 mov oC0, c0 ---
We definitely don't want stores to *ultimately* survive DCE, but we could potentially have an early DCE pass that *only* throws away non-stores, so that we can match native behaviour wrt things like this. But I don't know that it's worth trying that hard to match. I'd frankly rather ignore this quirk and just apply constant folding at parse time, like the first approach I mentioned.
After some thinking, I think the following idea is worth trying:
Moving the "else if" branch to dce, and only for nodes that are being deleted (because they are not used in a resource_load).
Ech, I don't much like that. Too much conflation of different things.
...
Btw, I haven't think this too much yet, but, if the resource_load·s pointed to load nodes (both in resource and sampler) instead of having direct derefs, this validation pass with only the "else if" branch should be enough.
However, if we consider this change seriously, I think it will be better to introduce it after this patch series.
That's been considered but I don't like it; it means that the load nodes will not correspond to an allocated temp but can't be deleted.
+ } + + if (invalid) + hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF, + "All object references must be determinable at compile time.");
I think it'd be helpful to name the specific expression that can't be resolved (and, along the same lines, to report resources and samplers separately).
Okay. I am thinking on using hlsl_note() after hlsl_error() to report the location of the expression to the user, getting outputs like: --- testm.hlsl:7:20: E5022: Loaded resource must be determinable at compile time. testm.hlsl:7:14: Expression cannot be resolved statically. --- Is this okay?
I can also print the instruction pointer as a TRACE() or similar.
I think my comment as-is made no sense or was miswritten; we already print the load location. (Is there another location we should be printing?) What I was probably intending was that we should print the variable name; I think that would make sense. We shouldn't need a separate hlsl_note() to do so though.
I thought you were referring to the location of the specific node (or nodes) in the index path that are not constant. But yes, I think that the load location is enough. I will add the variable name in the message.
Hmm, that's a good idea actually; I forgot we already do have enough information to print that. But it can wait for later, sure.
From: Francisco Casas <fcasas(a)codeweavers.com> Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- libs/vkd3d-shader/hlsl.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index 8b31d021..e99cc307 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -198,7 +198,6 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type { unsigned int element_size = type->e.array.type->reg_size; - assert(element_size); if (is_sm4) type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size; else @@ -218,8 +217,6 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type struct hlsl_struct_field *field = &type->e.record.fields[i]; unsigned int field_size = field->type->reg_size; - assert(field_size); - type->reg_size = hlsl_type_get_sm4_offset(field->type, type->reg_size); field->reg_offset = type->reg_size; type->reg_size += field_size; @@ -230,8 +227,7 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type } case HLSL_CLASS_OBJECT: - /* For convenience when performing copy propagation. */ - type->reg_size = 1; + type->reg_size = 0; break; } } -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
From: Francisco Casas <fcasas(a)codeweavers.com> HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT (zero) is used as a temporal value for elements_count for implicit size arrays. This value is replaced by the correct one after parsing the initializer. In case the implicit array is not initialized correctly, hlsl_error() is called but the array size is kept at 0. So the rest of the code must handle these cases. In shader model 5.1, unlike in 5.0, declaring a multi-dimensional object-type array with the last dimension implicit results in an error. This happens even in presence of an initializer. So, both gen_struct_fields() and declare_vars() first check if the shader model is 5.1, the array elements are objects, and if there is at least one implicit array size to handle the whole type as an unbounded resource array. Signed-off-by: Francisco Casas <fcasas(a)codeweavers.com> --- v2: - Detection of incorrect use of implicit arrays was moved from the parser rules to the respective add_* functions. v3: - Replaced 'if' with 'elif' in hlsl_type_calculate_reg_size(). - Updated changes to the new implicit array tests. - Removed incorrect empty line. - Fixed typo in error. - Removed field_size assertion in hlsl_type_calculate_reg_size(). v4: - Drop variables and struct fields declared as unbounded resource arrays. v5: - Renamed 'implicit arrays' to 'implicit size arrays'. v6: - Use 'true' instead of '1'. - Reordered some error detection checks. - Checking for shader model 5.1. v7: - Checking for unbounded_res_array first, and then branching, in gen_struct_fields() and declare_vars(). --- libs/vkd3d-shader/hlsl.c | 11 +- libs/vkd3d-shader/hlsl.h | 2 + libs/vkd3d-shader/hlsl.y | 144 +++++++++++++++++- ...lsl-initializer-implicit-array.shader_test | 16 +- 4 files changed, 158 insertions(+), 15 deletions(-) diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c index e99cc307..4779b9bd 100644 --- a/libs/vkd3d-shader/hlsl.c +++ b/libs/vkd3d-shader/hlsl.c @@ -198,7 +198,9 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type { unsigned int element_size = type->e.array.type->reg_size; - if (is_sm4) + if (type->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + type->reg_size = 0; + else if (is_sm4) type->reg_size = (type->e.array.elements_count - 1) * align(element_size, 4) + element_size; else type->reg_size = type->e.array.elements_count * element_size; @@ -1309,7 +1311,12 @@ struct vkd3d_string_buffer *hlsl_type_to_string(struct hlsl_ctx *ctx, const stru } for (t = type; t->type == HLSL_CLASS_ARRAY; t = t->e.array.type) - vkd3d_string_buffer_printf(string, "[%u]", t->e.array.elements_count); + { + if (t->e.array.elements_count == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + vkd3d_string_buffer_printf(string, "[]"); + else + vkd3d_string_buffer_printf(string, "[%u]", t->e.array.elements_count); + } return string; } diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h index c60c654b..e7906ce7 100644 --- a/libs/vkd3d-shader/hlsl.h +++ b/libs/vkd3d-shader/hlsl.h @@ -230,6 +230,8 @@ struct hlsl_src #define HLSL_MODIFIERS_MAJORITY_MASK (HLSL_MODIFIER_ROW_MAJOR | HLSL_MODIFIER_COLUMN_MAJOR) +#define HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT 0 + struct hlsl_reg_reservation { char type; diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 1e488afa..91f0ab60 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -853,6 +853,11 @@ static void free_parse_variable_def(struct parse_variable_def *v) vkd3d_free(v); } +static bool shader_is_sm_5_1(const struct hlsl_ctx *ctx) +{ + return ctx->profile->major_version == 5 && ctx->profile->minor_version >= 1; +} + static bool gen_struct_fields(struct hlsl_ctx *ctx, struct parse_fields *fields, struct hlsl_type *type, unsigned int modifiers, struct list *defs) { @@ -870,11 +875,45 @@ static bool gen_struct_fields(struct hlsl_ctx *ctx, struct parse_fields *fields, LIST_FOR_EACH_ENTRY_SAFE(v, v_next, defs, struct parse_variable_def, entry) { struct hlsl_struct_field *field = &fields->fields[i++]; - unsigned int j; + bool unbounded_res_array = false; + unsigned int k; field->type = type; - for (j = 0; j < v->arrays.count; ++j) - field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[j]); + + if (shader_is_sm_5_1(ctx) && type->type == HLSL_CLASS_OBJECT) + { + for (k = 0; k < v->arrays.count; ++k) + unbounded_res_array |= (v->arrays.sizes[k] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT); + } + + if (unbounded_res_array) + { + if (v->arrays.count == 1) + { + hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays as struct fields."); + free_parse_variable_def(v); + vkd3d_free(field); + continue; + } + else + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Unbounded resource arrays cannot be multi-dimensional."); + } + } + else + { + for (k = 0; k < v->arrays.count; ++k) + { + if (v->arrays.sizes[k] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Implicit size arrays not allowed in struct fields."); + } + + field->type = hlsl_new_array_type(ctx, field->type, v->arrays.sizes[k]); + } + } vkd3d_free(v->arrays.sizes); field->loc = v->loc; field->name = v->name; @@ -916,6 +955,12 @@ static bool add_typedef(struct hlsl_ctx *ctx, DWORD modifiers, struct hlsl_type ret = true; for (i = 0; i < v->arrays.count; ++i) { + if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Implicit size arrays not allowed in typedefs."); + } + if (!(type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]))) { free_parse_variable_def(v); @@ -1775,11 +1820,78 @@ static struct list *declare_vars(struct hlsl_ctx *ctx, struct hlsl_type *basic_t LIST_FOR_EACH_ENTRY_SAFE(v, v_next, var_list, struct parse_variable_def, entry) { + bool unbounded_res_array = false; unsigned int i; type = basic_type; - for (i = 0; i < v->arrays.count; ++i) - type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]); + + if (shader_is_sm_5_1(ctx) && type->type == HLSL_CLASS_OBJECT) + { + for (i = 0; i < v->arrays.count; ++i) + unbounded_res_array |= (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT); + } + + if (unbounded_res_array) + { + if (v->arrays.count == 1) + { + hlsl_fixme(ctx, &v->loc, "Unbounded resource arrays."); + free_parse_variable_def(v); + continue; + } + else + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Unbounded resource arrays cannot be multi-dimensional."); + } + } + else + { + for (i = 0; i < v->arrays.count; ++i) + { + if (v->arrays.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + { + unsigned int size = initializer_size(&v->initializer); + unsigned int elem_components = hlsl_type_component_count(type); + + if (i < v->arrays.count - 1) + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Only innermost array size can be implicit."); + free_parse_initializer(&v->initializer); + v->initializer.args_count = 0; + } + else if (elem_components == 0) + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Cannot declare an implicit size array of a size 0 type."); + free_parse_initializer(&v->initializer); + v->initializer.args_count = 0; + } + else if (size == 0) + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Implicit size arrays need to be initialized."); + free_parse_initializer(&v->initializer); + v->initializer.args_count = 0; + + } + else if (size % elem_components != 0) + { + hlsl_error(ctx, &v->loc, VKD3D_SHADER_ERROR_HLSL_WRONG_PARAMETER_COUNT, + "Cannot initialize implicit size array with %u components, expected a multiple of %u.", + size, elem_components); + free_parse_initializer(&v->initializer); + v->initializer.args_count = 0; + } + else + { + v->arrays.sizes[i] = size / elem_components; + } + } + type = hlsl_new_array_type(ctx, type, v->arrays.sizes[i]); + } + } vkd3d_free(v->arrays.sizes); if (type->type != HLSL_CLASS_MATRIX) @@ -3517,6 +3629,21 @@ arrays: $$.sizes = new_array; $$.sizes[$$.count++] = size; } + | '[' ']' arrays + { + uint32_t *new_array; + + $$ = $3; + + if (!(new_array = hlsl_realloc(ctx, $$.sizes, ($$.count + 1) * sizeof(*new_array)))) + { + vkd3d_free($$.sizes); + YYABORT; + } + + $$.sizes = new_array; + $$.sizes[$$.count++] = HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT; + } var_modifiers: %empty @@ -4058,7 +4185,14 @@ unary_expr: dst_type = $3; for (i = 0; i < $4.count; ++i) + { + if ($4.sizes[i] == HLSL_ARRAY_ELEMENTS_COUNT_IMPLICIT) + { + hlsl_error(ctx, &@3, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE, + "Implicit size arrays not allowed in casts."); + } dst_type = hlsl_new_array_type(ctx, dst_type, $4.sizes[i]); + } if (!compatible_data_types(src_type, dst_type)) { diff --git a/tests/hlsl-initializer-implicit-array.shader_test b/tests/hlsl-initializer-implicit-array.shader_test index 648825eb..38c8234c 100644 --- a/tests/hlsl-initializer-implicit-array.shader_test +++ b/tests/hlsl-initializer-implicit-array.shader_test @@ -7,8 +7,8 @@ float4 main() : SV_TARGET } [test] -todo draw quad -todo probe all rgba (50, 60, 70, 80) +draw quad +probe all rgba (50, 60, 70, 80) [pixel shader] @@ -21,8 +21,8 @@ float4 main() : sv_target } [test] -todo draw quad -todo probe all rgba (5.0, 6.0, 7.0, 8.0) +draw quad +probe all rgba (5.0, 6.0, 7.0, 8.0) [pixel shader] @@ -34,8 +34,8 @@ float4 main() : sv_target } [test] -todo draw quad -todo probe all rgba (7.0, 8.0, 9.0, 10.0) +draw quad +probe all rgba (7.0, 8.0, 9.0, 10.0) [pixel shader] @@ -62,8 +62,8 @@ float4 main() : SV_TARGET } [test] -todo draw quad -todo probe all rgba (318.0, 320.0, 322.0, 324.0) +draw quad +probe all rgba (318.0, 320.0, 322.0, 324.0) [pixel shader fail] -- GitLab https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/7
participants (4)
-
Francisco Casas -
Francisco Casas -
Francisco Casas (＠fcasas)
-
Zebediah Figura (she/her)