[PATCH v3 0/9] MR148: vkd3d-shader/hlsl: Support semantics for array types.

[Francisco Casas (＠fcasas)]

This is required by https://bugs.winehq.org/show_bug.cgi?id=54660 .

--
  v3: vkd3d-shader/hlsl: Error out when a semantic is used with multiple types.
      vkd3d-shader/hlsl: Error out when an output semantic is used more than once.
      vkd3d-shader/hlsl: Don't create semantic vars more than once.
      vkd3d-shader/hlsl: Support semantics for array types.
      vkd3d-shader/hlsl: Move get_array_size() and get_array_type() to hlsl.c.
      tests: Test array types with semantics.
      vkd3d-shader/hlsl: Avoid invalid input/output copies for non-numeric types.
      tests: Allow invalid vertex shader tests.
      tests: Expect S_OK result on [vertex shader].
 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 1/9] tests: Expect S_OK result on [vertex shader].

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 tests/shader_runner.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/shader_runner.c b/tests/shader_runner.c
index 9922496b..680a2856 100644
--- a/tests/shader_runner.c
+++ b/tests/shader_runner.c
@@ -1026,6 +1026,7 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o
             else if (!strcmp(line, "[vertex shader]\n"))
             {
                 state = STATE_SHADER_VERTEX;
+                expect_hr = S_OK;
             }
             else if (!strcmp(line, "[input layout]\n"))
             {
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 2/9] tests: Allow invalid vertex shader tests.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 tests/shader_runner.c | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/tests/shader_runner.c b/tests/shader_runner.c
index 680a2856..da5548bc 100644
--- a/tests/shader_runner.c
+++ b/tests/shader_runner.c
@@ -87,6 +87,7 @@ enum parse_state
     STATE_SHADER_PIXEL,
     STATE_SHADER_PIXEL_TODO,
     STATE_SHADER_VERTEX,
+    STATE_SHADER_VERTEX_TODO,
     STATE_TEST,
 };
 
@@ -824,7 +825,9 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o
                     break;
 
                 case STATE_SHADER_VERTEX:
-                    compile_shader(runner, shader_source, shader_source_len, "vs", expect_hr);
+                case STATE_SHADER_VERTEX_TODO:
+                    todo_if (state == STATE_SHADER_VERTEX_TODO)
+                        compile_shader(runner, shader_source, shader_source_len, "vs", expect_hr);
                     free(runner->vs_source);
                     runner->vs_source = shader_source;
                     shader_source = NULL;
@@ -1028,6 +1031,21 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o
                 state = STATE_SHADER_VERTEX;
                 expect_hr = S_OK;
             }
+            else if (!strcmp(line, "[vertex shader todo]\n"))
+            {
+                state = STATE_SHADER_VERTEX_TODO;
+                expect_hr = S_OK;
+            }
+            else if (!strcmp(line, "[vertex shader fail]\n"))
+            {
+                state = STATE_SHADER_VERTEX;
+                expect_hr = E_FAIL;
+            }
+            else if (!strcmp(line, "[vertex shader fail todo]\n"))
+            {
+                state = STATE_SHADER_VERTEX_TODO;
+                expect_hr = E_FAIL;
+            }
             else if (!strcmp(line, "[input layout]\n"))
             {
                 state = STATE_INPUT_LAYOUT;
@@ -1058,6 +1076,7 @@ void run_shader_tests(struct shader_runner *runner, const struct shader_runner_o
                 case STATE_SHADER_PIXEL:
                 case STATE_SHADER_PIXEL_TODO:
                 case STATE_SHADER_VERTEX:
+                case STATE_SHADER_VERTEX_TODO:
                 {
                     size_t len = strlen(line);
 
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 3/9] vkd3d-shader/hlsl: Avoid invalid input/output copies for non-numeric types.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 libs/vkd3d-shader/hlsl_codegen.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 5d7e426f..e250dd0b 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -263,6 +263,16 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
     struct hlsl_ir_var *var = lhs->src.var;
     unsigned int i;
 
+    if (type->class > HLSL_CLASS_LAST_NUMERIC)
+    {
+        struct vkd3d_string_buffer *string;
+        if (!(string = hlsl_type_to_string(ctx, type)))
+            return;
+        hlsl_fixme(ctx, &var->loc, "Input semantics for type %s.", string->buffer);
+        hlsl_release_string_buffer(ctx, string);
+        return;
+    }
+
     vector_type = hlsl_get_vector_type(ctx, type->base_type, hlsl_type_minor_size(type));
 
     for (i = 0; i < hlsl_type_major_size(type); ++i)
@@ -358,6 +368,16 @@ static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
     struct hlsl_ir_var *var = rhs->src.var;
     unsigned int i;
 
+    if (type->class > HLSL_CLASS_LAST_NUMERIC)
+    {
+        struct vkd3d_string_buffer *string;
+        if (!(string = hlsl_type_to_string(ctx, type)))
+            return;
+        hlsl_fixme(ctx, &var->loc, "Output semantics for type %s.", string->buffer);
+        hlsl_release_string_buffer(ctx, string);
+        return;
+    }
+
     vector_type = hlsl_get_vector_type(ctx, type->base_type, hlsl_type_minor_size(type));
 
     for (i = 0; i < hlsl_type_major_size(type); ++i)
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 4/9] tests: Test array types with semantics.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 tests/entry-point-semantics.shader_test | 240 ++++++++++++++++++++++++
 1 file changed, 240 insertions(+)

diff --git a/tests/entry-point-semantics.shader_test b/tests/entry-point-semantics.shader_test
index a32a0e7b..e1080955 100644
--- a/tests/entry-point-semantics.shader_test
+++ b/tests/entry-point-semantics.shader_test
@@ -50,3 +50,243 @@ float4 main(float tex : bogus) : bogus;
 [test]
 draw quad
 probe (0, 0) rgba (0.2, 0.2, 0.2, 0.2)
+
+[vertex shader todo]
+void main(out float2 tex[4] : texcoord, inout float4 pos : sv_position)
+{
+    tex[0] = float2(1.0, 2.0);
+    tex[1] = float2(3.0, 4.0);
+    tex[2] = float2(5.0, 6.0);
+    tex[3] = float2(7.0, 8.0);
+}
+
+
+% Array parameters of non-struct elements must have a semantic.
+[pixel shader fail]
+float4 main(in float2 arr[2]) : sv_target
+{
+    return 0.0;
+}
+
+
+% Array elements with a semantic get successive indexes.
+[pixel shader todo]
+struct apple
+{
+    float2 tp[4] : TEXCOORD0;
+};
+
+float4 main(in apple a) : sv_target
+{
+    return float4(a.tp[0].x, a.tp[1].x, a.tp[2].x, a.tp[3].x);
+}
+
+[test]
+todo draw quad
+todo probe (0, 0) rgba (1.0, 3.0, 5.0, 7.0)
+
+
+% Arrays of matrices get successive indexes.
+[pixel shader todo]
+struct apple
+{
+    float2x2 tp[2] : TEXCOORD0;
+};
+
+float4 main(in apple a) : sv_target
+{
+    return float4(a.tp[0][0].x, a.tp[0][1].x, a.tp[1][0].x, a.tp[1][1].x);
+}
+
+[test]
+todo draw quad
+todo probe (0, 0) rgba (1.0, 2.0, 5.0, 6.0)
+
+
+% Arrays (even multi-dimensional) of struct elements are allowed. The fields in the different struct
+% elements get the same indexes.
+[pixel shader todo]
+struct apple {
+    float2 texcoord : TEXCOORD0;
+    float4 arb : UNUSED;
+};
+
+float4 main(in apple aps[2][2]) : sv_target
+{
+    return float4(aps[0][0].texcoord, aps[1][1].texcoord);
+}
+
+[test]
+todo draw quad
+todo probe (0, 0) rgba (1.0, 2.0, 1.0, 2.0)
+
+
+[pixel shader todo]
+struct apple {
+    float2 texcoord : TEXCOORD0;
+};
+
+struct banana {
+    apple apl;
+    float4 arb : UNUSED;
+};
+
+float4 main(in banana bans[2]) : sv_target
+{
+    return float4(bans[0].apl.texcoord, bans[1].apl.texcoord);
+}
+
+[test]
+todo draw quad
+todo probe (0, 0) rgba (1.0, 2.0, 1.0, 2.0)
+
+
+% Arguments with the same semantic aren't aliased.
+[pixel shader]
+float4 main(in float2 t1 : TEXCOORD0, in float2 t2 : TEXCOORD0) : sv_target
+{
+    t1 = float2(10, 20);
+    return float4(t1, t2);
+}
+
+[test]
+todo draw quad
+todo probe (0, 0) rgba (10.0, 20.0, 1.0, 2.0)
+
+
+[pixel shader fail]
+struct apple {
+    float2 miss; // missing semantic.
+};
+
+struct banana {
+    apple apl[2];
+    float4 arb : UNUSED;
+};
+
+float4 main(in banana bans[2]) : sv_target
+{
+    return 0.0;
+}
+
+
+% Different indexes of the same semantic can have different types.
+[pixel shader]
+float4 main(in float2 a : TEXCOORD0, in float b : TEXCOORD1) : sv_target
+{
+    return float4(a, b, 0);
+}
+
+[test]
+todo draw quad
+todo probe (0, 0) rgba (1.0, 2.0, 3.0, 0.0)
+
+
+% Duplicated input semantics can only have different types if they have the same layout and register types.
+[pixel shader fail todo]
+float4 main(in float2 a : TEXCOORD0, in float3 b : TEXCOORD0) : sv_target
+{
+    return 0.0;
+}
+
+
+[pixel shader fail todo]
+float4 main(in float2 a : TEXCOORD0, in int2 b : TEXCOORD0) : sv_target
+{
+    return 0.0;
+}
+
+
+[pixel shader]
+float4 main(in float2 a : TEXCOORD0, in half2 b : TEXCOORD0, in float2x1 c: TEXCOORD0) : sv_target
+{
+    return 0.0;
+}
+
+
+[pixel shader]
+float4 main(in uint2 a : TEXCOORD0, in int2 b : TEXCOORD0, in int2x1 c : TEXCOORD0, in bool2 d : TEXCOORD0) : sv_target
+{
+    return 0.0;
+}
+
+
+% For some reason, vectors from row_major matrices are not considered has having the same layout as
+% regular vectors.
+[pixel shader fail todo]
+float4 main(in float2 a : TEXCOORD0, row_major float1x2 b : TEXCOORD0) : sv_target
+{
+    return 0.0;
+}
+
+[pixel shader fail todo]
+float4 main(in float2 a : TEXCOORD0, row_major float2x1 b : TEXCOORD0) : sv_target
+{
+    return 0.0;
+}
+
+[pixel shader fail todo]
+float4 main(in float4 a : TEXCOORD0, row_major float1x4 b : TEXCOORD0) : sv_target
+{
+    return 0.0;
+}
+
+
+[vertex shader fail]
+struct apple {
+    float2 miss; // missing semantic.
+};
+
+struct banana {
+    apple apl[2];
+    float4 arb : UNUSED;
+};
+
+void main(out banana bans[2])
+{
+    return 0.0;
+}
+
+
+% Output semantics cannot be mapped to more than one value.
+[vertex shader fail]
+struct apple {
+    float2 tex : TEXCOORD0;
+};
+
+void main(out apple apls[2], inout float4 pos : sv_position)
+{
+    apls[0].tex = float2(1, 2);
+    apls[1].tex = float2(3, 4);
+}
+
+
+% Semantic names are case-insensitive.
+[vertex shader fail todo]
+void main(out float2 a : sem0, out float2 b : SEM, inout float4 pos : sv_position)
+{
+    a = float2(1, 2);
+    b = float2(3, 4);
+}
+
+
+[vertex shader todo]
+struct apple {
+    float2 tex[2] : TEXCOORD0;
+};
+
+void main(out apple apl, inout float4 pos : sv_position)
+{
+    apl.tex[0] = float2(1, 2);
+    apl.tex[1] = float2(10, 20);
+}
+
+[pixel shader]
+float4 main(in float2 tex0 : TEXCOORD0, in float2 tex1 : TEXCOORD1) : sv_target
+{
+    return float4(tex0, tex1);
+}
+
+[test]
+todo draw quad
+todo probe (0, 0) rgba (1.0, 2.0, 10.0, 20.0)
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 5/9] vkd3d-shader/hlsl: Move get_array_size() and get_array_type() to hlsl.c.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 libs/vkd3d-shader/hlsl.c     | 23 +++++++++++++++--------
 libs/vkd3d-shader/hlsl.h     |  3 +++
 libs/vkd3d-shader/hlsl_sm1.c | 18 ++----------------
 libs/vkd3d-shader/hlsl_sm4.c | 18 ++----------------
 4 files changed, 22 insertions(+), 40 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index 6d015c20..bc411075 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -157,13 +157,6 @@ unsigned int hlsl_type_element_count(const struct hlsl_type *type)
     }
 }
 
-static unsigned int get_array_size(const struct hlsl_type *type)
-{
-    if (type->class == HLSL_CLASS_ARRAY)
-        return get_array_size(type->e.array.type) * type->e.array.elements_count;
-    return 1;
-}
-
 bool hlsl_type_is_resource(const struct hlsl_type *type)
 {
     if (type->class == HLSL_CLASS_OBJECT)
@@ -278,7 +271,7 @@ static void hlsl_type_calculate_reg_size(struct hlsl_ctx *ctx, struct hlsl_type
                     type->reg_size[k] += field->type->reg_size[k];
                 }
 
-                type->dimx += field->type->dimx * field->type->dimy * get_array_size(field->type);
+                type->dimx += field->type->dimx * field->type->dimy * hlsl_get_multiarray_size(field->type);
             }
             break;
         }
@@ -757,6 +750,20 @@ bool hlsl_types_are_equal(const struct hlsl_type *t1, const struct hlsl_type *t2
     return true;
 }
 
+const struct hlsl_type *hlsl_get_multiarray_element_type(const struct hlsl_type *type)
+{
+    if (type->class == HLSL_CLASS_ARRAY)
+        return hlsl_get_multiarray_element_type(type->e.array.type);
+    return type;
+}
+
+unsigned int hlsl_get_multiarray_size(const struct hlsl_type *type)
+{
+    if (type->class == HLSL_CLASS_ARRAY)
+        return hlsl_get_multiarray_size(type->e.array.type) * type->e.array.elements_count;
+    return 1;
+}
+
 struct hlsl_type *hlsl_type_clone(struct hlsl_ctx *ctx, struct hlsl_type *old,
         unsigned int default_majority, unsigned int modifiers)
 {
diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index 38a7715e..7acbb1e7 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -1108,6 +1108,9 @@ enum hlsl_regset hlsl_type_get_regset(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);
 
+const struct hlsl_type *hlsl_get_multiarray_element_type(const struct hlsl_type *type);
+unsigned int hlsl_get_multiarray_size(const struct hlsl_type *type);
+
 unsigned int hlsl_combine_swizzles(unsigned int first, unsigned int second, unsigned int dim);
 unsigned int hlsl_combine_writemasks(unsigned int first, unsigned int second);
 unsigned int hlsl_map_swizzle(unsigned int swizzle, unsigned int writemask);
diff --git a/libs/vkd3d-shader/hlsl_sm1.c b/libs/vkd3d-shader/hlsl_sm1.c
index be32c8db..4ef01f99 100644
--- a/libs/vkd3d-shader/hlsl_sm1.c
+++ b/libs/vkd3d-shader/hlsl_sm1.c
@@ -224,24 +224,10 @@ static D3DXPARAMETER_TYPE sm1_base_type(const struct hlsl_type *type)
     }
 }
 
-static const struct hlsl_type *get_array_type(const struct hlsl_type *type)
-{
-    if (type->class == HLSL_CLASS_ARRAY)
-        return get_array_type(type->e.array.type);
-    return type;
-}
-
-static unsigned int get_array_size(const struct hlsl_type *type)
-{
-    if (type->class == HLSL_CLASS_ARRAY)
-        return get_array_size(type->e.array.type) * type->e.array.elements_count;
-    return 1;
-}
-
 static void write_sm1_type(struct vkd3d_bytecode_buffer *buffer, struct hlsl_type *type, unsigned int ctab_start)
 {
-    const struct hlsl_type *array_type = get_array_type(type);
-    unsigned int array_size = get_array_size(type);
+    const struct hlsl_type *array_type = hlsl_get_multiarray_element_type(type);
+    unsigned int array_size = hlsl_get_multiarray_size(type);
     unsigned int field_count = 0;
     size_t fields_offset = 0;
     size_t i;
diff --git a/libs/vkd3d-shader/hlsl_sm4.c b/libs/vkd3d-shader/hlsl_sm4.c
index 8551cfd8..5c900f90 100644
--- a/libs/vkd3d-shader/hlsl_sm4.c
+++ b/libs/vkd3d-shader/hlsl_sm4.c
@@ -255,20 +255,6 @@ static void write_sm4_signature(struct hlsl_ctx *ctx, struct dxbc_writer *dxbc,
     dxbc_writer_add_section(dxbc, output ? TAG_OSGN : TAG_ISGN, buffer.data, buffer.size);
 }
 
-static const struct hlsl_type *get_array_type(const struct hlsl_type *type)
-{
-    if (type->class == HLSL_CLASS_ARRAY)
-        return get_array_type(type->e.array.type);
-    return type;
-}
-
-static unsigned int get_array_size(const struct hlsl_type *type)
-{
-    if (type->class == HLSL_CLASS_ARRAY)
-        return get_array_size(type->e.array.type) * type->e.array.elements_count;
-    return 1;
-}
-
 static D3D_SHADER_VARIABLE_CLASS sm4_class(const struct hlsl_type *type)
 {
     switch (type->class)
@@ -359,7 +345,7 @@ static D3D_SHADER_VARIABLE_TYPE sm4_base_type(const struct hlsl_type *type)
 
 static void write_sm4_type(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *buffer, struct hlsl_type *type)
 {
-    const struct hlsl_type *array_type = get_array_type(type);
+    const struct hlsl_type *array_type = hlsl_get_multiarray_element_type(type);
     const char *name = array_type->name ? array_type->name : "<unnamed>";
     const struct hlsl_profile_info *profile = ctx->profile;
     unsigned int field_count = 0, array_size = 0;
@@ -373,7 +359,7 @@ static void write_sm4_type(struct hlsl_ctx *ctx, struct vkd3d_bytecode_buffer *b
         name_offset = put_string(buffer, name);
 
     if (type->class == HLSL_CLASS_ARRAY)
-        array_size = get_array_size(type);
+        array_size = hlsl_get_multiarray_size(type);
 
     if (array_type->class == HLSL_CLASS_STRUCT)
     {
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 6/9] vkd3d-shader/hlsl: Support semantics for array types.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 libs/vkd3d-shader/hlsl.h                |   3 +
 libs/vkd3d-shader/hlsl.y                |   7 +-
 libs/vkd3d-shader/hlsl_codegen.c        | 187 +++++++++++++-----------
 tests/entry-point-semantics.shader_test |  36 ++---
 4 files changed, 123 insertions(+), 110 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index 7acbb1e7..b45cd0ae 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -207,6 +207,9 @@ struct hlsl_semantic
 {
     const char *name;
     uint32_t index;
+
+    /* If the variable or field that stores this hlsl_semantic has already reported that it is missing. */
+    bool reported_missing;
 };
 
 /* A field within a struct type declaration, used in hlsl_type.e.fields. */
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 79ec970a..27594ebe 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -4545,7 +4545,7 @@ var_identifier:
 colon_attribute:
       %empty
         {
-            $$.semantic.name = NULL;
+            $$.semantic = (struct hlsl_semantic){0};
             $$.reg_reservation.reg_type = 0;
             $$.reg_reservation.offset_type = 0;
         }
@@ -4557,12 +4557,12 @@ colon_attribute:
         }
     | register_opt
         {
-            $$.semantic.name = NULL;
+            $$.semantic = (struct hlsl_semantic){0};
             $$.reg_reservation = $1;
         }
     | packoffset_opt
         {
-            $$.semantic.name = NULL;
+            $$.semantic = (struct hlsl_semantic){0};
             $$.reg_reservation = $1;
         }
 
@@ -4575,6 +4575,7 @@ semantic:
                 ;
             $$.name = $2;
             $$.index = atoi(p);
+            $$.reported_missing = false;
             *p = 0;
         }
 
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index e250dd0b..ffb80aa5 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -221,8 +221,20 @@ static void prepend_uniform_copy(struct hlsl_ctx *ctx, struct list *instrs, stru
     list_add_after(&load->node.entry, &store->node.entry);
 }
 
+static void validate_field_semantic(struct hlsl_ctx *ctx, struct hlsl_struct_field *field)
+{
+    if (!field->semantic.name && hlsl_get_multiarray_element_type(field->type)->class <= HLSL_CLASS_LAST_NUMERIC
+            && !field->semantic.reported_missing)
+    {
+        hlsl_error(ctx, &field->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_SEMANTIC,
+                "Field '%s' is missing a semantic.", field->name);
+        field->semantic.reported_missing = true;
+    }
+}
+
 static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir_var *var,
-        struct hlsl_type *type, unsigned int modifiers, const struct hlsl_semantic *semantic, bool output)
+        struct hlsl_type *type, unsigned int modifiers, const struct hlsl_semantic *semantic,
+        uint32_t index, bool output)
 {
     struct hlsl_semantic new_semantic;
     struct vkd3d_string_buffer *name;
@@ -230,13 +242,13 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir
 
     if (!(name = hlsl_get_string_buffer(ctx)))
         return NULL;
-    vkd3d_string_buffer_printf(name, "<%s-%s%u>", output ? "output" : "input", semantic->name, semantic->index);
+    vkd3d_string_buffer_printf(name, "<%s-%s%u>", output ? "output" : "input", semantic->name, index);
     if (!(new_semantic.name = hlsl_strdup(ctx, semantic->name)))
     {
         hlsl_release_string_buffer(ctx, name);
         return NULL;
     }
-    new_semantic.index = semantic->index;
+    new_semantic.index = index;
     if (!(ext_var = hlsl_new_var(ctx, hlsl_strdup(ctx, name->buffer),
             type, var->loc, &new_semantic, modifiers, NULL)))
     {
@@ -257,12 +269,44 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir
 }
 
 static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_load *lhs,
-        unsigned int modifiers, const struct hlsl_semantic *semantic)
+        unsigned int modifiers, const struct hlsl_semantic *semantic, uint32_t *semantic_index)
 {
     struct hlsl_type *type = lhs->node.data_type, *vector_type;
     struct hlsl_ir_var *var = lhs->src.var;
+    struct hlsl_ir_constant *c;
     unsigned int i;
 
+    if (type->class == HLSL_CLASS_ARRAY || type->class == HLSL_CLASS_STRUCT)
+    {
+        struct hlsl_ir_load *element_load;
+        struct hlsl_struct_field *field;
+        uint32_t field_semantic_index;
+
+        for (i = 0; i < hlsl_type_element_count(type); ++i)
+        {
+            if (type->class == HLSL_CLASS_STRUCT)
+            {
+                field = &type->e.record.fields[i];
+                validate_field_semantic(ctx, field);
+                semantic = &field->semantic;
+                field_semantic_index = semantic->index;
+                semantic_index = &field_semantic_index;
+            }
+
+            if (!(c = hlsl_new_uint_constant(ctx, i, &var->loc)))
+                return;
+            list_add_after(&lhs->node.entry, &c->node.entry);
+
+            /* This redundant load is expected to be deleted later by DCE. */
+            if (!(element_load = hlsl_new_load_index(ctx, &lhs->src, &c->node, &var->loc)))
+                return;
+            list_add_after(&c->node.entry, &element_load->node.entry);
+
+            prepend_input_copy(ctx, instrs, element_load, modifiers, semantic, semantic_index);
+        }
+        return;
+    }
+
     if (type->class > HLSL_CLASS_LAST_NUMERIC)
     {
         struct vkd3d_string_buffer *string;
@@ -270,23 +314,21 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
             return;
         hlsl_fixme(ctx, &var->loc, "Input semantics for type %s.", string->buffer);
         hlsl_release_string_buffer(ctx, string);
-        return;
     }
+    if (!semantic->name)
+        return;
 
     vector_type = hlsl_get_vector_type(ctx, type->base_type, hlsl_type_minor_size(type));
 
     for (i = 0; i < hlsl_type_major_size(type); ++i)
     {
-        struct hlsl_semantic semantic_copy = *semantic;
         struct hlsl_ir_store *store;
-        struct hlsl_ir_constant *c;
         struct hlsl_ir_var *input;
         struct hlsl_ir_load *load;
 
-        semantic_copy.index = semantic->index + i;
-
-        if (!(input = add_semantic_var(ctx, var, vector_type, modifiers, &semantic_copy, false)))
+        if (!(input = add_semantic_var(ctx, var, vector_type, modifiers, semantic, *semantic_index, false)))
             return;
+        ++*semantic_index;
 
         if (!(load = hlsl_new_var_load(ctx, input, var->loc)))
             return;
@@ -313,41 +355,11 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
     }
 }
 
-static void prepend_input_struct_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_load *lhs)
-{
-    struct hlsl_type *type = lhs->node.data_type;
-    struct hlsl_ir_var *var = lhs->src.var;
-    size_t i;
-
-    for (i = 0; i < type->e.record.field_count; ++i)
-    {
-        const struct hlsl_struct_field *field = &type->e.record.fields[i];
-        struct hlsl_ir_load *field_load;
-        struct hlsl_ir_constant *c;
-
-        if (!(c = hlsl_new_uint_constant(ctx, i, &var->loc)))
-            return;
-        list_add_after(&lhs->node.entry, &c->node.entry);
-
-        /* This redundant load is expected to be deleted later by DCE. */
-        if (!(field_load = hlsl_new_load_index(ctx, &lhs->src, &c->node, &var->loc)))
-            return;
-        list_add_after(&c->node.entry, &field_load->node.entry);
-
-        if (field->type->class == HLSL_CLASS_STRUCT)
-            prepend_input_struct_copy(ctx, instrs, field_load);
-        else if (field->semantic.name)
-            prepend_input_copy(ctx, instrs, field_load, field->storage_modifiers, &field->semantic);
-        else
-            hlsl_error(ctx, &field->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_SEMANTIC,
-                    "Field '%s' is missing a semantic.", field->name);
-    }
-}
-
 /* Split inputs into two variables representing the semantic and temp registers,
  * and copy the former to the latter, so that writes to input variables work. */
 static void prepend_input_var_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_var *var)
 {
+    uint32_t semantic_index = var->semantic.index;
     struct hlsl_ir_load *load;
 
     /* This redundant load is expected to be deleted later by DCE. */
@@ -355,19 +367,47 @@ static void prepend_input_var_copy(struct hlsl_ctx *ctx, struct list *instrs, st
         return;
     list_add_head(instrs, &load->node.entry);
 
-    if (var->data_type->class == HLSL_CLASS_STRUCT)
-        prepend_input_struct_copy(ctx, instrs, load);
-    else if (var->semantic.name)
-        prepend_input_copy(ctx, instrs, load, var->storage_modifiers, &var->semantic);
+    prepend_input_copy(ctx, instrs, load, var->storage_modifiers, &var->semantic, &semantic_index);
 }
 
 static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_load *rhs,
-        unsigned int modifiers, const struct hlsl_semantic *semantic)
+        unsigned int modifiers, const struct hlsl_semantic *semantic, uint32_t *semantic_index)
 {
     struct hlsl_type *type = rhs->node.data_type, *vector_type;
     struct hlsl_ir_var *var = rhs->src.var;
+    struct hlsl_ir_constant *c;
     unsigned int i;
 
+    if (type->class == HLSL_CLASS_ARRAY || type->class == HLSL_CLASS_STRUCT)
+    {
+        struct hlsl_ir_load *element_load;
+        struct hlsl_struct_field *field;
+        uint32_t field_semantic_index;
+
+        for (i = 0; i < hlsl_type_element_count(type); ++i)
+        {
+            if (type->class == HLSL_CLASS_STRUCT)
+            {
+                field = &type->e.record.fields[i];
+                validate_field_semantic(ctx, field);
+                semantic = &field->semantic;
+                field_semantic_index = semantic->index;
+                semantic_index = &field_semantic_index;
+            }
+
+            if (!(c = hlsl_new_uint_constant(ctx, i, &var->loc)))
+                return;
+            list_add_tail(instrs, &c->node.entry);
+
+            if (!(element_load = hlsl_new_load_index(ctx, &rhs->src, &c->node, &var->loc)))
+                return;
+            list_add_tail(instrs, &element_load->node.entry);
+
+            append_output_copy(ctx, instrs, element_load, modifiers, semantic, semantic_index);
+        }
+        return;
+    }
+
     if (type->class > HLSL_CLASS_LAST_NUMERIC)
     {
         struct vkd3d_string_buffer *string;
@@ -375,23 +415,21 @@ static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
             return;
         hlsl_fixme(ctx, &var->loc, "Output semantics for type %s.", string->buffer);
         hlsl_release_string_buffer(ctx, string);
-        return;
     }
+    if (!semantic->name)
+        return;
 
     vector_type = hlsl_get_vector_type(ctx, type->base_type, hlsl_type_minor_size(type));
 
     for (i = 0; i < hlsl_type_major_size(type); ++i)
     {
-        struct hlsl_semantic semantic_copy = *semantic;
         struct hlsl_ir_store *store;
-        struct hlsl_ir_constant *c;
         struct hlsl_ir_var *output;
         struct hlsl_ir_load *load;
 
-        semantic_copy.index = semantic->index + i;
-
-        if (!(output = add_semantic_var(ctx, var, vector_type, modifiers, &semantic_copy, true)))
+        if (!(output = add_semantic_var(ctx, var, vector_type, modifiers, semantic, *semantic_index, true)))
             return;
+        ++*semantic_index;
 
         if (type->class == HLSL_CLASS_MATRIX)
         {
@@ -418,42 +456,12 @@ static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
     }
 }
 
-static void append_output_struct_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_load *rhs)
-{
-    struct hlsl_type *type = rhs->node.data_type;
-    struct hlsl_ir_var *var = rhs->src.var;
-    size_t i;
-
-    for (i = 0; i < type->e.record.field_count; ++i)
-    {
-        const struct hlsl_struct_field *field = &type->e.record.fields[i];
-        struct hlsl_ir_load *field_load;
-        struct hlsl_ir_constant *c;
-
-        if (!(c = hlsl_new_uint_constant(ctx, i, &var->loc)))
-            return;
-        list_add_tail(instrs, &c->node.entry);
-
-        /* This redundant load is expected to be deleted later by DCE. */
-        if (!(field_load = hlsl_new_load_index(ctx, &rhs->src, &c->node, &var->loc)))
-            return;
-        list_add_tail(instrs, &field_load->node.entry);
-
-        if (field->type->class == HLSL_CLASS_STRUCT)
-            append_output_struct_copy(ctx, instrs, field_load);
-        else if (field->semantic.name)
-            append_output_copy(ctx, instrs, field_load, field->storage_modifiers, &field->semantic);
-        else
-            hlsl_error(ctx, &field->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_SEMANTIC,
-                    "Field '%s' is missing a semantic.", field->name);
-    }
-}
-
 /* Split outputs into two variables representing the temp and semantic
  * registers, and copy the former to the latter, so that reads from output
  * variables work. */
 static void append_output_var_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_var *var)
 {
+    uint32_t semantic_index = var->semantic.index;
     struct hlsl_ir_load *load;
 
     /* This redundant load is expected to be deleted later by DCE. */
@@ -461,10 +469,7 @@ static void append_output_var_copy(struct hlsl_ctx *ctx, struct list *instrs, st
         return;
     list_add_tail(instrs, &load->node.entry);
 
-    if (var->data_type->class == HLSL_CLASS_STRUCT)
-        append_output_struct_copy(ctx, instrs, load);
-    else if (var->semantic.name)
-        append_output_copy(ctx, instrs, load, var->storage_modifiers, &var->semantic);
+    append_output_copy(ctx, instrs, load, var->storage_modifiers, &var->semantic, &semantic_index);
 }
 
 static bool transform_ir(struct hlsl_ctx *ctx, bool (*func)(struct hlsl_ctx *ctx, struct hlsl_ir_node *, void *),
@@ -3396,15 +3401,19 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry
     {
         var = entry_func->parameters.vars[i];
 
-        if (var->data_type->class == HLSL_CLASS_OBJECT || (var->storage_modifiers & HLSL_STORAGE_UNIFORM))
+        if (hlsl_type_is_resource(var->data_type) || (var->storage_modifiers & HLSL_STORAGE_UNIFORM))
         {
             prepend_uniform_copy(ctx, &body->instrs, var);
         }
         else
         {
-            if (var->data_type->class != HLSL_CLASS_STRUCT && !var->semantic.name)
+            if (hlsl_get_multiarray_element_type(var->data_type)->class != HLSL_CLASS_STRUCT
+                    && !var->semantic.name)
+            {
                 hlsl_error(ctx, &var->loc, VKD3D_SHADER_ERROR_HLSL_MISSING_SEMANTIC,
                         "Parameter \"%s\" is missing a semantic.", var->name);
+                var->semantic.reported_missing = true;
+            }
 
             if (var->storage_modifiers & HLSL_STORAGE_IN)
                 prepend_input_var_copy(ctx, &body->instrs, var);
diff --git a/tests/entry-point-semantics.shader_test b/tests/entry-point-semantics.shader_test
index e1080955..460e64b2 100644
--- a/tests/entry-point-semantics.shader_test
+++ b/tests/entry-point-semantics.shader_test
@@ -51,7 +51,7 @@ float4 main(float tex : bogus) : bogus;
 draw quad
 probe (0, 0) rgba (0.2, 0.2, 0.2, 0.2)
 
-[vertex shader todo]
+[vertex shader]
 void main(out float2 tex[4] : texcoord, inout float4 pos : sv_position)
 {
     tex[0] = float2(1.0, 2.0);
@@ -70,7 +70,7 @@ float4 main(in float2 arr[2]) : sv_target
 
 
 % Array elements with a semantic get successive indexes.
-[pixel shader todo]
+[pixel shader]
 struct apple
 {
     float2 tp[4] : TEXCOORD0;
@@ -82,12 +82,12 @@ float4 main(in apple a) : sv_target
 }
 
 [test]
-todo draw quad
-todo probe (0, 0) rgba (1.0, 3.0, 5.0, 7.0)
+draw quad
+probe (0, 0) rgba (1.0, 3.0, 5.0, 7.0)
 
 
 % Arrays of matrices get successive indexes.
-[pixel shader todo]
+[pixel shader]
 struct apple
 {
     float2x2 tp[2] : TEXCOORD0;
@@ -99,13 +99,13 @@ float4 main(in apple a) : sv_target
 }
 
 [test]
-todo draw quad
-todo probe (0, 0) rgba (1.0, 2.0, 5.0, 6.0)
+draw quad
+probe (0, 0) rgba (1.0, 2.0, 5.0, 6.0)
 
 
 % Arrays (even multi-dimensional) of struct elements are allowed. The fields in the different struct
 % elements get the same indexes.
-[pixel shader todo]
+[pixel shader]
 struct apple {
     float2 texcoord : TEXCOORD0;
     float4 arb : UNUSED;
@@ -117,11 +117,11 @@ float4 main(in apple aps[2][2]) : sv_target
 }
 
 [test]
-todo draw quad
+draw quad
 todo probe (0, 0) rgba (1.0, 2.0, 1.0, 2.0)
 
 
-[pixel shader todo]
+[pixel shader]
 struct apple {
     float2 texcoord : TEXCOORD0;
 };
@@ -137,7 +137,7 @@ float4 main(in banana bans[2]) : sv_target
 }
 
 [test]
-todo draw quad
+draw quad
 todo probe (0, 0) rgba (1.0, 2.0, 1.0, 2.0)
 
 
@@ -150,7 +150,7 @@ float4 main(in float2 t1 : TEXCOORD0, in float2 t2 : TEXCOORD0) : sv_target
 }
 
 [test]
-todo draw quad
+draw quad
 todo probe (0, 0) rgba (10.0, 20.0, 1.0, 2.0)
 
 
@@ -178,8 +178,8 @@ float4 main(in float2 a : TEXCOORD0, in float b : TEXCOORD1) : sv_target
 }
 
 [test]
-todo draw quad
-todo probe (0, 0) rgba (1.0, 2.0, 3.0, 0.0)
+draw quad
+probe (0, 0) rgba (1.0, 2.0, 3.0, 0.0)
 
 
 % Duplicated input semantics can only have different types if they have the same layout and register types.
@@ -249,7 +249,7 @@ void main(out banana bans[2])
 
 
 % Output semantics cannot be mapped to more than one value.
-[vertex shader fail]
+[vertex shader fail todo]
 struct apple {
     float2 tex : TEXCOORD0;
 };
@@ -270,7 +270,7 @@ void main(out float2 a : sem0, out float2 b : SEM, inout float4 pos : sv_positio
 }
 
 
-[vertex shader todo]
+[vertex shader]
 struct apple {
     float2 tex[2] : TEXCOORD0;
 };
@@ -288,5 +288,5 @@ float4 main(in float2 tex0 : TEXCOORD0, in float2 tex1 : TEXCOORD1) : sv_target
 }
 
 [test]
-todo draw quad
-todo probe (0, 0) rgba (1.0, 2.0, 10.0, 20.0)
+draw quad
+probe (0, 0) rgba (1.0, 2.0, 10.0, 20.0)
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 7/9] vkd3d-shader/hlsl: Don't create semantic vars more than once.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

---
 libs/vkd3d-shader/hlsl_codegen.c        | 10 ++++++++++
 tests/entry-point-semantics.shader_test |  6 +++---
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index ffb80aa5..6e7d5e03 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -243,6 +243,16 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir
     if (!(name = hlsl_get_string_buffer(ctx)))
         return NULL;
     vkd3d_string_buffer_printf(name, "<%s-%s%u>", output ? "output" : "input", semantic->name, index);
+
+    LIST_FOR_EACH_ENTRY(ext_var, &ctx->extern_vars, struct hlsl_ir_var, extern_entry)
+    {
+        if (!ascii_strcasecmp(ext_var->name, name->buffer))
+        {
+            hlsl_release_string_buffer(ctx, name);
+            return ext_var;
+        }
+    }
+
     if (!(new_semantic.name = hlsl_strdup(ctx, semantic->name)))
     {
         hlsl_release_string_buffer(ctx, name);
diff --git a/tests/entry-point-semantics.shader_test b/tests/entry-point-semantics.shader_test
index 460e64b2..cb565b00 100644
--- a/tests/entry-point-semantics.shader_test
+++ b/tests/entry-point-semantics.shader_test
@@ -118,7 +118,7 @@ float4 main(in apple aps[2][2]) : sv_target
 
 [test]
 draw quad
-todo probe (0, 0) rgba (1.0, 2.0, 1.0, 2.0)
+probe (0, 0) rgba (1.0, 2.0, 1.0, 2.0)
 
 
 [pixel shader]
@@ -138,7 +138,7 @@ float4 main(in banana bans[2]) : sv_target
 
 [test]
 draw quad
-todo probe (0, 0) rgba (1.0, 2.0, 1.0, 2.0)
+probe (0, 0) rgba (1.0, 2.0, 1.0, 2.0)
 
 
 % Arguments with the same semantic aren't aliased.
@@ -151,7 +151,7 @@ float4 main(in float2 t1 : TEXCOORD0, in float2 t2 : TEXCOORD0) : sv_target
 
 [test]
 draw quad
-todo probe (0, 0) rgba (10.0, 20.0, 1.0, 2.0)
+probe (0, 0) rgba (10.0, 20.0, 1.0, 2.0)
 
 
 [pixel shader fail]
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 8/9] vkd3d-shader/hlsl: Error out when an output semantic is used more than once.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

The use of the hlsl_semantic.reported_duplicated_output_next_index field
allows reporting multiple overlapping indexes, such as in the following
vertex shader:

    void main(out float1x3 x : OVERLAP0, out float1x3 y : OVERLAP1)
    {
        x = float3(1.0, 2.0, 3.2);
        y = float3(5.0, 6.0, 5.0);
    }

    apple.hlsl:1:41: E5013: Output semantic "OVERLAP1" is used multiple times.
    apple.hlsl:1:13: First use of "OVERLAP1" is here.
    apple.hlsl:1:41: E5013: Output semantic "OVERLAP2" is used multiple times.
    apple.hlsl:1:13: First use of "OVERLAP2" is here.

While at the same time avoiding reporting overalps more than once for
large arrays:

    struct apple
    {
        float2 p : sv_position;
    };

    void main(out apple aps[4])
    {
    }

    apple.hlsl:3:8: E5013: Output semantic "sv_position0" is used multiple times.
    apple.hlsl:3:8: First use of "sv_position0" is here.
---
 libs/vkd3d-shader/hlsl.h                |  3 +++
 libs/vkd3d-shader/hlsl.y                |  1 +
 libs/vkd3d-shader/hlsl_codegen.c        | 34 ++++++++++++++++++-------
 tests/entry-point-semantics.shader_test |  4 +--
 tests/matrix-semantics.shader_test      |  2 +-
 5 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index b45cd0ae..968299fb 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -210,6 +210,9 @@ struct hlsl_semantic
 
     /* If the variable or field that stores this hlsl_semantic has already reported that it is missing. */
     bool reported_missing;
+    /* In case the variable or field that stores this semantic has already reported to use a
+     *   duplicated output semantic, this value stores the last reported index + 1. Otherwise it is 0. */
+    uint32_t reported_duplicated_output_next_index;
 };
 
 /* A field within a struct type declaration, used in hlsl_type.e.fields. */
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 27594ebe..a148d207 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -4576,6 +4576,7 @@ semantic:
             $$.name = $2;
             $$.index = atoi(p);
             $$.reported_missing = false;
+            $$.reported_duplicated_output_next_index = 0;
             *p = 0;
         }
 
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 6e7d5e03..4c64b3db 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -233,8 +233,8 @@ static void validate_field_semantic(struct hlsl_ctx *ctx, struct hlsl_struct_fie
 }
 
 static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir_var *var,
-        struct hlsl_type *type, unsigned int modifiers, const struct hlsl_semantic *semantic,
-        uint32_t index, bool output)
+        struct hlsl_type *type, unsigned int modifiers, struct hlsl_semantic *semantic,
+        uint32_t index, bool output, const struct vkd3d_shader_location *loc)
 {
     struct hlsl_semantic new_semantic;
     struct vkd3d_string_buffer *name;
@@ -248,6 +248,18 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir
     {
         if (!ascii_strcasecmp(ext_var->name, name->buffer))
         {
+            if (output)
+            {
+                if (index >= semantic->reported_duplicated_output_next_index)
+                {
+                    hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SEMANTIC,
+                            "Output semantic \"%s%u\" is used multiple times.", semantic->name, index);
+                    hlsl_note(ctx, &ext_var->loc, HLSL_LEVEL_ERROR,
+                            "First use of \"%s%u\" is here.", semantic->name, index);
+                    semantic->reported_duplicated_output_next_index = index + 1;
+                }
+            }
+
             hlsl_release_string_buffer(ctx, name);
             return ext_var;
         }
@@ -260,7 +272,7 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir
     }
     new_semantic.index = index;
     if (!(ext_var = hlsl_new_var(ctx, hlsl_strdup(ctx, name->buffer),
-            type, var->loc, &new_semantic, modifiers, NULL)))
+            type, *loc, &new_semantic, modifiers, NULL)))
     {
         hlsl_release_string_buffer(ctx, name);
         hlsl_cleanup_semantic(&new_semantic);
@@ -279,9 +291,10 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir
 }
 
 static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_load *lhs,
-        unsigned int modifiers, const struct hlsl_semantic *semantic, uint32_t *semantic_index)
+        unsigned int modifiers, struct hlsl_semantic *semantic, uint32_t *semantic_index)
 {
     struct hlsl_type *type = lhs->node.data_type, *vector_type;
+    struct vkd3d_shader_location *loc = &lhs->node.loc;
     struct hlsl_ir_var *var = lhs->src.var;
     struct hlsl_ir_constant *c;
     unsigned int i;
@@ -301,6 +314,7 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
                 semantic = &field->semantic;
                 field_semantic_index = semantic->index;
                 semantic_index = &field_semantic_index;
+                loc = &field->loc;
             }
 
             if (!(c = hlsl_new_uint_constant(ctx, i, &var->loc)))
@@ -308,7 +322,7 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
             list_add_after(&lhs->node.entry, &c->node.entry);
 
             /* This redundant load is expected to be deleted later by DCE. */
-            if (!(element_load = hlsl_new_load_index(ctx, &lhs->src, &c->node, &var->loc)))
+            if (!(element_load = hlsl_new_load_index(ctx, &lhs->src, &c->node, loc)))
                 return;
             list_add_after(&c->node.entry, &element_load->node.entry);
 
@@ -336,7 +350,7 @@ static void prepend_input_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
         struct hlsl_ir_var *input;
         struct hlsl_ir_load *load;
 
-        if (!(input = add_semantic_var(ctx, var, vector_type, modifiers, semantic, *semantic_index, false)))
+        if (!(input = add_semantic_var(ctx, var, vector_type, modifiers, semantic, *semantic_index, false, loc)))
             return;
         ++*semantic_index;
 
@@ -381,9 +395,10 @@ static void prepend_input_var_copy(struct hlsl_ctx *ctx, struct list *instrs, st
 }
 
 static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct hlsl_ir_load *rhs,
-        unsigned int modifiers, const struct hlsl_semantic *semantic, uint32_t *semantic_index)
+        unsigned int modifiers, struct hlsl_semantic *semantic, uint32_t *semantic_index)
 {
     struct hlsl_type *type = rhs->node.data_type, *vector_type;
+    struct vkd3d_shader_location *loc = &rhs->node.loc;
     struct hlsl_ir_var *var = rhs->src.var;
     struct hlsl_ir_constant *c;
     unsigned int i;
@@ -403,13 +418,14 @@ static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
                 semantic = &field->semantic;
                 field_semantic_index = semantic->index;
                 semantic_index = &field_semantic_index;
+                loc = &field->loc;
             }
 
             if (!(c = hlsl_new_uint_constant(ctx, i, &var->loc)))
                 return;
             list_add_tail(instrs, &c->node.entry);
 
-            if (!(element_load = hlsl_new_load_index(ctx, &rhs->src, &c->node, &var->loc)))
+            if (!(element_load = hlsl_new_load_index(ctx, &rhs->src, &c->node, loc)))
                 return;
             list_add_tail(instrs, &element_load->node.entry);
 
@@ -437,7 +453,7 @@ static void append_output_copy(struct hlsl_ctx *ctx, struct list *instrs, struct
         struct hlsl_ir_var *output;
         struct hlsl_ir_load *load;
 
-        if (!(output = add_semantic_var(ctx, var, vector_type, modifiers, semantic, *semantic_index, true)))
+        if (!(output = add_semantic_var(ctx, var, vector_type, modifiers, semantic, *semantic_index, true, loc)))
             return;
         ++*semantic_index;
 
diff --git a/tests/entry-point-semantics.shader_test b/tests/entry-point-semantics.shader_test
index cb565b00..c9d0d9cc 100644
--- a/tests/entry-point-semantics.shader_test
+++ b/tests/entry-point-semantics.shader_test
@@ -249,7 +249,7 @@ void main(out banana bans[2])
 
 
 % Output semantics cannot be mapped to more than one value.
-[vertex shader fail todo]
+[vertex shader fail]
 struct apple {
     float2 tex : TEXCOORD0;
 };
@@ -262,7 +262,7 @@ void main(out apple apls[2], inout float4 pos : sv_position)
 
 
 % Semantic names are case-insensitive.
-[vertex shader fail todo]
+[vertex shader fail]
 void main(out float2 a : sem0, out float2 b : SEM, inout float4 pos : sv_position)
 {
     a = float2(1, 2);
diff --git a/tests/matrix-semantics.shader_test b/tests/matrix-semantics.shader_test
index 43f467ec..b704dc1a 100644
--- a/tests/matrix-semantics.shader_test
+++ b/tests/matrix-semantics.shader_test
@@ -63,7 +63,7 @@ probe render target 1 all r (2.0)
 probe render target 2 all r (3.0)
 probe render target 3 all r (4.0)
 
-[pixel shader fail todo]
+[pixel shader fail]
 void main(out float1x2 x : sv_target0, out float1x2 y : sv_target1)
 {
     x = float2(1.0, 2.0);
-- 
GitLab


https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[PATCH v3 9/9] vkd3d-shader/hlsl: Error out when a semantic is used with multiple types.

[Francisco Casas]

From: Francisco Casas <fcasas(a)codeweavers.com>

Considering row vectors from row_major matrices as having a different
layout as regular vectors, and error out in that case, is left as todo.
---
 libs/vkd3d-shader/hlsl.h                |  4 +++
 libs/vkd3d-shader/hlsl.y                |  1 +
 libs/vkd3d-shader/hlsl_codegen.c        | 33 +++++++++++++++++++++++++
 tests/entry-point-semantics.shader_test |  6 ++---
 4 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.h b/libs/vkd3d-shader/hlsl.h
index 968299fb..bca05f51 100644
--- a/libs/vkd3d-shader/hlsl.h
+++ b/libs/vkd3d-shader/hlsl.h
@@ -213,6 +213,10 @@ struct hlsl_semantic
     /* In case the variable or field that stores this semantic has already reported to use a
      *   duplicated output semantic, this value stores the last reported index + 1. Otherwise it is 0. */
     uint32_t reported_duplicated_output_next_index;
+    /* In case the variable or field that stores this semantic has already reported to use a
+     *   duplicated input semantic with incompatible values, this value stores the last reported
+     *   index + 1. Otherwise it is 0. */
+    uint32_t reported_duplicated_input_incompatible_next_index;
 };
 
 /* A field within a struct type declaration, used in hlsl_type.e.fields. */
diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index a148d207..dfef622c 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -4577,6 +4577,7 @@ semantic:
             $$.index = atoi(p);
             $$.reported_missing = false;
             $$.reported_duplicated_output_next_index = 0;
+            $$.reported_duplicated_input_incompatible_next_index = 0;
             *p = 0;
         }
 
diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 4c64b3db..f25fcefb 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -232,6 +232,26 @@ static void validate_field_semantic(struct hlsl_ctx *ctx, struct hlsl_struct_fie
     }
 }
 
+static enum hlsl_base_type base_type_get_semantic_equivalent(enum hlsl_base_type base)
+{
+    if (base == HLSL_TYPE_BOOL)
+        return HLSL_TYPE_UINT;
+    if (base == HLSL_TYPE_INT)
+        return HLSL_TYPE_UINT;
+    if (base == HLSL_TYPE_HALF)
+        return HLSL_TYPE_FLOAT;
+    return base;
+}
+
+static bool types_are_semantic_equivalent(const struct hlsl_type *type1, const struct hlsl_type *type2)
+{
+    if (type1->dimx != type2->dimx)
+        return false;
+
+    return base_type_get_semantic_equivalent(type1->base_type)
+            == base_type_get_semantic_equivalent(type2->base_type);
+}
+
 static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir_var *var,
         struct hlsl_type *type, unsigned int modifiers, struct hlsl_semantic *semantic,
         uint32_t index, bool output, const struct vkd3d_shader_location *loc)
@@ -259,6 +279,19 @@ static struct hlsl_ir_var *add_semantic_var(struct hlsl_ctx *ctx, struct hlsl_ir
                     semantic->reported_duplicated_output_next_index = index + 1;
                 }
             }
+            else
+            {
+                if (index >= semantic->reported_duplicated_input_incompatible_next_index
+                        && !types_are_semantic_equivalent(ext_var->data_type, type))
+                {
+                    hlsl_error(ctx, loc, VKD3D_SHADER_ERROR_HLSL_INVALID_SEMANTIC,
+                            "Input semantic \"%s%u\" is used multiple times with incompatible types.",
+                            semantic->name, index);
+                    hlsl_note(ctx, &ext_var->loc, HLSL_LEVEL_ERROR,
+                            "First declaration of \"%s%u\" is here.", semantic->name, index);
+                    semantic->reported_duplicated_input_incompatible_next_index = index + 1;
+                }
+            }
 
             hlsl_release_string_buffer(ctx, name);
             return ext_var;
diff --git a/tests/entry-point-semantics.shader_test b/tests/entry-point-semantics.shader_test
index c9d0d9cc..7983f309 100644
--- a/tests/entry-point-semantics.shader_test
+++ b/tests/entry-point-semantics.shader_test
@@ -183,14 +183,14 @@ probe (0, 0) rgba (1.0, 2.0, 3.0, 0.0)
 
 
 % Duplicated input semantics can only have different types if they have the same layout and register types.
-[pixel shader fail todo]
+[pixel shader fail]
 float4 main(in float2 a : TEXCOORD0, in float3 b : TEXCOORD0) : sv_target
 {
     return 0.0;
 }
 
 
-[pixel shader fail todo]
+[pixel shader fail]
 float4 main(in float2 a : TEXCOORD0, in int2 b : TEXCOORD0) : sv_target
 {
     return 0.0;
@@ -219,7 +219,7 @@ float4 main(in float2 a : TEXCOORD0, row_major float1x2 b : TEXCOORD0) : sv_targ
     return 0.0;
 }
 
-[pixel shader fail todo]
+[pixel shader fail]
 float4 main(in float2 a : TEXCOORD0, row_major float2x1 b : TEXCOORD0) : sv_target
 {
     return 0.0;
-- 
GitLab

https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148

[Francisco Casas (＠fcasas)]

On Tue Apr 11 03:05:16 2023 +0000, Zebediah Figura wrote:
> > Now, regarding the reporting the previous variable's location. It begs
> > the question of what to do when there are more than 2 repetitions. If we
> > do this, we probably would want to report all locations where the
> > semantic is used, and also find a way of reporting when a semantic is
> > used more than once in a single location that doesn't imply printing the
> > location as many times as it is repeated (consider a very large array of
> > structs with the semantic in a field).
> >
> > I spent some time thinking about this, but I can't see a way that
> > wouldn't require storing all the reported locations. So, IMO, the
> > additional complexity doesn't overweight the benefit. But if you think
> > that it is important enough, I will do it.
> > I don't think you need to do that. Rather, you just want to do it like a
> > normal redefinition error: every time there's a definition which
> > conflicts with an earlier definition, you "note" the first earlier
> > definition that conflicts.
> > I also don't see why you'd want to report the previous location more
> > than once for an array?
> > I think I did a poor job at explaining the problem, consider this example:

```hlsl
struct apple
{
    float4 a[2] : REPEATED_OUTPUT;
};

float4 main(out apple a[5], out float4 r : REPEATED_OUTPUT) : sv_target
{
    // ...
}
```

In that case we will iterate 5 times through the apple struct, and try to add the REPEATED_OUTPUT0 semantic var 5 times, and also REPEATED_OUTPUT1 5 times. For each, the first time we create the semantic var, the **second time** we have to report the error, indicating that the first time the location was the same as now, and the following 3 iterations we need to ignore the error. But we also have to report the error for the `r` parameter.

So the challenge here is knowing when we already have reported the specific error for a (semantic,index,location) trio.

It took me some time but in v3 I came up with an strategy that achieves this by including additional flags in the hlsl_semantic struct, for whether it is in an hlsl_ir_var or in a hlsl_struct_field), that keep track of the errors already reported.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148#note_29862

[Francisco Casas (＠fcasas)]

On Tue Apr 11 02:54:59 2023 +0000, Zebediah Figura wrote:
> > Okay, I am reporting that as an `hlsl_note()` though, since the error
> > will be emitted by the `if (var->is_missing_semantics)` check at the
> > level of `hlsl_emit_bytecode()`.
> > My idea was that you'd report the error in prepend_input_var_copy() or
> > whatever, and then you don't need the is_missing_semantics error.
> > Okay, in v3 the error is being reported when traversing the field itself. 
> > Since a field can be traversed multiple times (say, for an array of structs), a flag it is still necessary to avoid reporting the error multiple times (`hlsl_semantic.reported_missing` in this implementation.).

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148#note_29863

[Zebediah Figura (＠zfigura)]

On Wed Apr 12 21:47:16 2023 +0000, Francisco Casas wrote:
> I think I did a poor job at explaining the problem, consider this example:
> ```hlsl
> struct apple
> {
>     float4 a[2] : REPEATED_OUTPUT;
> };
> float4 main(out apple a[5], out float4 r : REPEATED_OUTPUT) : sv_target
> {
>     // ...
> }
> ```
> In that case we will iterate 5 times through the apple struct, and try
> to add the REPEATED_OUTPUT0 semantic var 5 times, and also
> REPEATED_OUTPUT1 5 times. For each, the first time we create the
> semantic var, the **second time** we have to report the error,
> indicating that the first time the location was the same as now, and the
> following 3 iterations we need to ignore the error. But we also have to
> report the error for the `r` parameter.
> So the challenge here is knowing when we already have reported the
> specific error for a (semantic,index,location) trio.
> It took me some time but in v3 I came up with an strategy that achieves
> this by including additional flags in the hlsl_semantic struct, for
> whether it is in an hlsl_ir_var or in a hlsl_struct_field), that keep
> track of the errors already reported.
> Honestly, I'm tempted to say that "you used an output semantic in an array variable; that can't work" is the kind of thing that deserves a slightly different error message than "you used an output semantic twice" anyway. Like, if I just saw "line 139: output semantic was already used, previous use at line 139", I feel like that takes a little more mental effort to figure out what I did wrong than "line 139: output semantic used inside of an array".

Along those lines, you'd want to just track whether there was an array (with size > 1) when iterating over semantics.

I'm open to disagreement on that point, though.

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148#note_29864

[Francisco Casas (＠fcasas)]

On Wed Apr 12 21:58:44 2023 +0000, Zebediah Figura wrote:
> Honestly, I'm tempted to say that "you used an output semantic in an
> array variable; that can't work" is the kind of thing that deserves a
> slightly different error message than "you used an output semantic
> twice" anyway. Like, if I just saw "line 139: output semantic was
> already used, previous use at line 139", I feel like that takes a little
> more mental effort to figure out what I did wrong than "line 139: output
> semantic used inside of an array".
> Along those lines, you'd want to just track whether there was an array
> (with size > 1) when iterating over semantics.
> I'm open to disagreement on that point, though.
> Well, "you used an output semantic in an array variable; that can't work" (or more specifically, that the semantic is used within a struct within an array) isn't the only way that makes the first location the same as the second location.

It can also happen that two input semantics use the same struct, like in the following vertex shader:

```
struct apple {
    float2 f : SEMANTIC;
};

void main(out apple a, out apple b) 
{
    a.f = float2(1, 2);
    b.f = float2(3, 4);
}
```
which... I better add as a test.

Would it be a good compromise to make the hlsl_note slightly different when both locations coincide, but keeping it generic?

I am thinking on something like:
"This struct field appears more than once in the input/output parameters."

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148#note_29865

[Zebediah Figura (＠zfigura)]

On Wed Apr 12 22:29:06 2023 +0000, Francisco Casas wrote:
> Well, "you used an output semantic in an array variable; that can't
> work" (or more specifically, that the semantic is used within a struct
> within an array) isn't the only way that makes the first location the
> same as the second location. 
> It can also happen that two input semantics use the same struct, like in
> the following vertex shader:
> ```
> struct apple {
>     float2 f : SEMANTIC;
> };
> void main(out apple a, out apple b) 
> {
>     a.f = float2(1, 2);
>     b.f = float2(3, 4);
> }
> ```
> which... I better add as a test.
> Would it be a good compromise to make the hlsl_note slightly different
> when both locations coincide, but keeping it generic?
> I am thinking on something like:
> "This struct field appears more than once in the input/output parameters."
> Hmm, I hadn't thought of that. Being maximally descriptive is probably going to be hard...

And it's probably a rare mistake to make anyway, I can't imagine any sane programmer would embed semantics in an array or use the same struct multiple times anyway. So maybe we can just leave it at v3...

-- 
https://gitlab.winehq.org/wine/vkd3d/-/merge_requests/148#note_29866