Just a few miscellaneous fixes I think are simple enough to make it into the next version.
-- v3: vkd3d-shader/hlsl: Revert expr_compatible_data_types() args names to "t1" and "t2". vkd3d-shader/hlsl: Don't produce a parse error on empty buffer_body. vkd3d-shader/hlsl: Check for non-static object references on resource stores. tests: Add missing entry for max.shader_test in Makefile.am
From: Francisco Casas fcasas@codeweavers.com
--- Makefile.am | 1 + 1 file changed, 1 insertion(+)
diff --git a/Makefile.am b/Makefile.am index 0db71d67..74c45aed 100644 --- a/Makefile.am +++ b/Makefile.am @@ -121,6 +121,7 @@ vkd3d_shader_tests = \ tests/logic-operations.shader_test \ tests/math.shader_test \ tests/matrix-semantics.shader_test \ + tests/max.shader_test \ tests/multiple-rt.shader_test \ tests/nointerpolation.shader_test \ tests/object-references.shader_test \
From: Francisco Casas fcasas@codeweavers.com
--- libs/vkd3d-shader/hlsl_codegen.c | 12 ++++++++++++ tests/object-references.shader_test | 12 ++++++++++++ 2 files changed, 24 insertions(+)
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c index 58d44c4d..6e4168fc 100644 --- a/libs/vkd3d-shader/hlsl_codegen.c +++ b/libs/vkd3d-shader/hlsl_codegen.c @@ -1024,6 +1024,18 @@ static bool validate_static_object_references(struct hlsl_ctx *ctx, struct hlsl_ note_non_static_deref_expressions(ctx, &load->sampler, "resource load sampler"); } } + else if (instr->type == HLSL_IR_RESOURCE_STORE) + { + struct hlsl_ir_resource_store *store = hlsl_ir_resource_store(instr); + + if (!hlsl_component_index_range_from_deref(ctx, &store->resource, &start, &count)) + { + hlsl_error(ctx, &instr->loc, VKD3D_SHADER_ERROR_HLSL_NON_STATIC_OBJECT_REF, + "Accessed resource from "%s" must be determinable at compile time.", + store->resource.var->name); + note_non_static_deref_expressions(ctx, &store->resource, "accessed resource"); + } + }
return false; } diff --git a/tests/object-references.shader_test b/tests/object-references.shader_test index a33bd14a..12f745e6 100644 --- a/tests/object-references.shader_test +++ b/tests/object-references.shader_test @@ -120,6 +120,18 @@ float4 main() : sv_target }
+[pixel shader fail] +// Note: Only valid in shader model 5.1 +RWTexture2D<float4> tex[3]; +uniform int n; + +float4 main() : sv_target +{ + tex[n][int2(0, 0)] = 0.6; + return 0; +} + + [pixel shader todo] Texture2D tex; uniform float f;
From: Francisco Casas fcasas@codeweavers.com
--- Makefile.am | 1 + libs/vkd3d-shader/hlsl.y | 2 +- tests/cbuffer.shader_test | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/cbuffer.shader_test
diff --git a/Makefile.am b/Makefile.am index 74c45aed..85cd4642 100644 --- a/Makefile.am +++ b/Makefile.am @@ -65,6 +65,7 @@ vkd3d_shader_tests = \ tests/cast-to-half.shader_test \ tests/cast-to-int.shader_test \ tests/cast-to-uint.shader_test \ + tests/cbuffer.shader_test \ tests/compute.shader_test \ tests/conditional.shader_test \ tests/floor.shader_test \ diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index 6461ade5..c1fb003d 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -3361,7 +3361,7 @@ buffer_type: }
declaration_statement_list: - declaration_statement + %empty | declaration_statement_list declaration_statement
preproc_directive: diff --git a/tests/cbuffer.shader_test b/tests/cbuffer.shader_test new file mode 100644 index 00000000..07ef18db --- /dev/null +++ b/tests/cbuffer.shader_test @@ -0,0 +1,17 @@ +[pixel shader] +// Test empty constant buffer. +cbuffer Constants : register(b1) +{ +}; + +float4 foo; + +float4 main() : sv_target +{ + return foo; +} + +[test] +uniform 0 float4 1.0 2.0 3.0 4.0 +draw quad +probe all rgba (1.0, 2.0, 3.0, 4.0)
From: Francisco Casas fcasas@codeweavers.com
Unlike compatible_data_types() and implicit_compatible_data_types(), this function is intended to be symmetrical. So it makes sense to preserve the names "t1" and "t2" for the arguments. --- libs/vkd3d-shader/hlsl.y | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y index c1fb003d..eedc85bd 100644 --- a/libs/vkd3d-shader/hlsl.y +++ b/libs/vkd3d-shader/hlsl.y @@ -1175,34 +1175,34 @@ static unsigned int evaluate_array_dimension(struct hlsl_ir_node *node) } }
-static bool expr_compatible_data_types(struct hlsl_type *src, struct hlsl_type *dst) +static bool expr_compatible_data_types(struct hlsl_type *t1, struct hlsl_type *t2) { - if (src->base_type > HLSL_TYPE_LAST_SCALAR || dst->base_type > HLSL_TYPE_LAST_SCALAR) + if (t1->base_type > HLSL_TYPE_LAST_SCALAR || t2->base_type > HLSL_TYPE_LAST_SCALAR) return false;
/* Scalar vars can be converted to pretty much everything */ - if ((src->dimx == 1 && src->dimy == 1) || (dst->dimx == 1 && dst->dimy == 1)) + if ((t1->dimx == 1 && t1->dimy == 1) || (t2->dimx == 1 && t2->dimy == 1)) return true;
- if (src->type == HLSL_CLASS_VECTOR && dst->type == HLSL_CLASS_VECTOR) + if (t1->type == HLSL_CLASS_VECTOR && t2->type == HLSL_CLASS_VECTOR) return true;
- if (src->type == HLSL_CLASS_MATRIX || dst->type == HLSL_CLASS_MATRIX) + if (t1->type == HLSL_CLASS_MATRIX || t2->type == HLSL_CLASS_MATRIX) { /* Matrix-vector conversion is apparently allowed if either they have the same components count or the matrix is nx1 or 1xn */ - if (src->type == HLSL_CLASS_VECTOR || dst->type == HLSL_CLASS_VECTOR) + if (t1->type == HLSL_CLASS_VECTOR || t2->type == HLSL_CLASS_VECTOR) { - if (hlsl_type_component_count(src) == hlsl_type_component_count(dst)) + if (hlsl_type_component_count(t1) == hlsl_type_component_count(t2)) return true;
- return (src->type == HLSL_CLASS_MATRIX && (src->dimx == 1 || src->dimy == 1)) - || (dst->type == HLSL_CLASS_MATRIX && (dst->dimx == 1 || dst->dimy == 1)); + return (t1->type == HLSL_CLASS_MATRIX && (t1->dimx == 1 || t1->dimy == 1)) + || (t2->type == HLSL_CLASS_MATRIX && (t2->dimx == 1 || t2->dimy == 1)); }
/* Both matrices */ - if ((src->dimx >= dst->dimx && src->dimy >= dst->dimy) - || (src->dimx <= dst->dimx && src->dimy <= dst->dimy)) + if ((t1->dimx >= t2->dimx && t1->dimy >= t2->dimy) + || (t1->dimx <= t2->dimx && t1->dimy <= t2->dimy)) return true; }
On Fri Nov 11 18:38:03 2022 +0000, Francisco Casas wrote:
changed this line in [version 3 of the diff](/wine/vkd3d/-/merge_requests/48/diffs?diff_id=18224&start_sha=bf038eb3d525f54046b623a8d6b24fad68aaf235#9155b9453b4ec8ea0b9b025dfb55c061bd931610_3352_3348)
Yeah, I meant that latter one. Sorry, I should have been clearer.
On Fri Nov 11 21:06:25 2022 +0000, Zebediah Figura wrote:
Yeah, I meant that latter one. Sorry, I should have been clearer.
Ugh, and now that I notice it we're also leaking / failing to handle declaration_statement here. Annoying that bison doesn't warn here when declaration_statement_list has no type but declaration_statement does.
Ugh, and now that I notice it we're also leaking / failing to handle declaration_statement here. Annoying that bison doesn't warn here when declaration_statement_list has no type but declaration_statement does.
To be clear that's a preëxisting problem, doesn't need a fix in this patch set.
This merge request was approved by Giovanni Mascellani.
I agree it makes sense to make a freeze exception for this MR.
This merge request was approved by Henri Verbeet.
-
Francisco Casas -
Francisco Casas (＠fcasas)
-
Giovanni Mascellani (＠giomasce)
-
Henri Verbeet (＠hverbeet)
-
Zebediah Figura (＠zfigura)