[PATCH v5 0/5] MR463: hlsl: Fix some type handling for ternaries.

[Zebediah Figura (＠zfigura)]

--
  v5: vkd3d-shader/hlsl: Allow non-numeric types in the ternary operator.
      vkd3d-shader/hlsl: Separate an add_ternary() helper.
      tests: Add many more tests for ternary expressions.

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

[PATCH v5 1/5] vkd3d-shader/hlsl: Remove a redundant definition of the "float" type.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl.c b/libs/vkd3d-shader/hlsl.c
index 501bff8cf..b95d2d592 100644
--- a/libs/vkd3d-shader/hlsl.c
+++ b/libs/vkd3d-shader/hlsl.c
@@ -3344,7 +3344,6 @@ static void declare_predefined_types(struct hlsl_ctx *ctx)
     effect_types[] =
     {
         {"dword",           HLSL_CLASS_SCALAR, HLSL_TYPE_UINT,          1, 1},
-        {"float",           HLSL_CLASS_SCALAR, HLSL_TYPE_FLOAT,         1, 1},
         {"vector",          HLSL_CLASS_VECTOR, HLSL_TYPE_FLOAT,         4, 1},
         {"matrix",          HLSL_CLASS_MATRIX, HLSL_TYPE_FLOAT,         4, 4},
         {"STRING",          HLSL_CLASS_OBJECT, HLSL_TYPE_STRING,        1, 1},

[PATCH v5 2/5] vkd3d-shader/hlsl: Do not try to lower ternaries of types other than scalar or vector.

[Zebediah Figura]

From: Zebediah Figura

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

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 5a70878bc..c5049d9f0 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -2764,6 +2764,12 @@ static bool lower_ternary(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, stru
     first = expr->operands[1].node;
     second = expr->operands[2].node;

+    if (cond->data_type->class > HLSL_CLASS_VECTOR || instr->data_type->class > HLSL_CLASS_VECTOR)
+    {
+        hlsl_fixme(ctx, &instr->loc, "Lower ternary of type other than scalar or vector.\n");
+        return false;
+    }
+
     if (ctx->profile->major_version < 4 && ctx->profile->type == VKD3D_SHADER_TYPE_PIXEL)
     {
         struct hlsl_ir_node *abs, *neg;

[PATCH v5 3/5] tests: Add many more tests for ternary expressions.

[Zebediah Figura]

From: Zebediah Figura

---
 Makefile.am                        |   1 +
 tests/hlsl/sm6-ternary.shader_test |  46 +++++
 tests/hlsl/ternary.shader_test     | 313 +++++++++++++++++++++++++++--
 3 files changed, 346 insertions(+), 14 deletions(-)
 create mode 100644 tests/hlsl/sm6-ternary.shader_test

diff --git a/Makefile.am b/Makefile.am
index 2666194a6..fa5afe7a8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -161,6 +161,7 @@ vkd3d_shader_tests = \
    tests/hlsl/side-effects.shader_test \
    tests/hlsl/sign.shader_test \
    tests/hlsl/single-numeric-initializer.shader_test \
+	tests/hlsl/sm6-ternary.shader_test \
    tests/hlsl/smoothstep.shader_test \
    tests/hlsl/sqrt.shader_test \
    tests/hlsl/state-block-syntax.shader_test \
diff --git a/tests/hlsl/sm6-ternary.shader_test b/tests/hlsl/sm6-ternary.shader_test
new file mode 100644
index 000000000..349d32676
--- /dev/null
+++ b/tests/hlsl/sm6-ternary.shader_test
@@ -0,0 +1,46 @@
+% The ternary operator works differently in sm6.
+% It now shortcuts, and is no longer per-component.
+
+[require]
+shader model >= 6.0
+
+[pixel shader]
+uniform float4 x;
+
+float4 main() : sv_target
+{
+    return x.x ? x : x - 1;
+}
+
+[test]
+uniform 0 float4 2.0 3.0 4.0 5.0
+todo draw quad
+probe all rgba (2.0, 3.0, 4.0, 5.0)
+uniform 0 float4 0.0 10.0 11.0 12.0
+todo draw quad
+probe all rgba (-1.0, 9.0, 10.0, 11.0)
+
+
+[pixel shader]
+float4 f;
+
+float4 main() : sv_target
+{
+    float f1 = 0.1, f2 = 0.2, f3;
+    f3 = f.x ? (f1 = 0.5) + 0.2 : (f2 = 0.6);
+    return float4(f1, f2, f3, 0.0);
+}
+
+[test]
+uniform 0 float4 1.0 0.0 0.0 0.0
+draw quad
+probe all rgba (0.5, 0.2, 0.7, 0.0)
+
+
+[pixel shader fail]
+float4 x, y, z;
+
+float4 main() : sv_target
+{
+    return x ? y : z;
+}
diff --git a/tests/hlsl/ternary.shader_test b/tests/hlsl/ternary.shader_test
index 7b4c71f7b..70ae20f96 100644
--- a/tests/hlsl/ternary.shader_test
+++ b/tests/hlsl/ternary.shader_test
@@ -1,3 +1,8 @@
+[require]
+% The ternary operator works differently in sm6. See sm6-ternary.shader_test.
+shader model < 6.0
+
+
 [pixel shader]
 uniform float4 x;

@@ -8,12 +13,13 @@ float4 main() : sv_target

[test]
 uniform 0 float4 2.0 3.0 4.0 5.0
-todo(sm>=6) draw quad
+draw quad
 probe all rgba (2.0, 3.0, 4.0, 5.0)
 uniform 0 float4 0.0 10.0 11.0 12.0
-todo(sm>=6) draw quad
+draw quad
 probe all rgba (-1.0, 9.0, 10.0, 11.0)

+
 [pixel shader]
 uniform float4 x;

@@ -32,6 +38,7 @@ uniform 0 float4 1.1 3.0 4.0 5.0
 draw quad
 probe all rgba (1.1, 2.0, 0.0, 0.0)

+
 [pixel shader]
 float4 f;

@@ -42,37 +49,315 @@ float4 main() : sv_target
     return float4(f1, f2, f3, 0.0);
 }

-[require]
-shader model < 6.0

[test]
 uniform 0 float4 1.0 0.0 0.0 0.0
 draw quad
 probe all rgba (0.5, 0.6, 0.7, 0.0)

-[require]
-shader model >= 6.0
+
+[pixel shader]
+float4 x, y, z;
+
+float4 main() : sv_target
+{
+    return x ? y : z;
+}
+
+[test]
+uniform 0 float4 0.0 1.0 0.0 -3.0
+uniform 4 float4 1.0 2.0 3.0 4.0
+uniform 8 float4 5.0 6.0 7.0 8.0
+draw quad
+probe all rgba (5.0, 2.0, 7.0, 4.0)
+
+
+% The usual type conversion is applied to the first and second expression, as
+% long as they are numeric.
+
+[pixel shader fail]
+
+uniform float2x4 a;
+uniform float3x2 b;
+
+float4 main() : sv_target
+{
+    0 ? a : b;
+    return 0;
+}
+
+
+[pixel shader]
+
+static float3x3 a = {0, 1, 2, 3, 4, 5, 6, 7, 8};
+static float2x2 b = {.1, .2, .3, .4};
+
+float4 main() : sv_target
+{
+    return float4(1 ? a : b);
+}
+
+[test]
+draw quad
+probe all rgba (0.0, 1.0, 3.0, 4.0)
+
+
+[pixel shader]
+
+static float3 a = {0, 1, 2};
+static float4 b = {5, 6, 7, 8};
+
+float4 main() : sv_target
+{
+    return float4(0 ? a : b, 1.0);
+}
+
+[test]
+draw quad
+probe all rgba (5.0, 6.0, 7.0, 1.0)
+
+
+% More restrictions are placed on the type of the first (condition) operand,
+% relative to the common type computed from the other two (result). Either:
+% * the class and dimensions must match exactly;
+% * the condition operand is scalar;
+% * the result operands are scalar;
+% * one is a typeN and the other is a type1xN
+
+[pixel shader fail todo]
+
+uniform float4 cond;
+uniform float2x2 a, b;
+
+float4 main() : sv_target
+{
+    return float4(cond ? a : b);
+}
+
+
+[pixel shader fail todo]
+
+uniform float2x2 cond;
+uniform float4 a, b;
+
+float4 main() : sv_target
+{
+    return float4(cond ? a : b);
+}
+
+
+[pixel shader]
+
+static float2x2 cond = {1, 0, 0, 1};
+static float2x2 a = {1, 2, 3, 4};
+static float2x2 b = {5, 6, 7, 8};
+
+float4 main() : sv_target
+{
+    return float4(cond ? a : b);
+}
+
+[test]
+draw quad
+probe all rgba (1.0, 6.0, 7.0, 4.0)
+
+
+[pixel shader fail todo]
+
+uniform float3 cond;
+uniform float4 a, b;
+
+float4 main() : sv_target
+{
+    (cond ? a : b);
+    return 0;
+}
+
+
+[pixel shader fail todo]
+
+uniform float4 cond;
+uniform float4x1 a, b;
+
+float4 main() : sv_target
+{
+    (cond ? a : b);
+    return 0;
+}
+
+
+% As may be expected, this yields the type of the arguments, not the conditional.
+
+[pixel shader]
+
+static float4 cond = {1, 0, 0, 1};
+static float1x4 a = {1, 2, 3, 4};
+static float1x4 b = {5, 6, 7, 8};
+
+float4 main() : sv_target
+{
+    return (cond ? a : b)[0];
+}
+
+[test]
+draw quad
+probe all rgba (1.0, 6.0, 7.0, 4.0)
+
+
+[pixel shader todo]
+
+static float1x4 cond = {1, 0, 0, 1};
+static float4 a = {1, 2, 3, 4};
+static float4 b = {5, 6, 7, 8};
+
+float4 main() : sv_target
+{
+    return (cond ? a : b)[0];
+}
+
+[test]
+draw quad
+probe all rgba (1.0, 1.0, 1.0, 1.0)
+
+
+[pixel shader fail]
+
+uniform float1x4 cond;
+uniform float4 a, b;
+
+float4 main() : sv_target
+{
+    return (cond ? a : b)[0][0];
+}
+
+
+[pixel shader]
+
+static float1 cond = {0};
+static float1x1 a = {2};
+static float1x1 b = {3};
+
+float4 main() : sv_target
+{
+    return (cond ? a : b)[0][0];
+}
+
+[test]
+draw quad
+probe all rgba (3.0, 3.0, 3.0, 3.0)
+
+
+[pixel shader]
+
+uniform float cond;
+uniform float4 a, b;
+
+float4 main() : sv_target
+{
+    return float4(cond ? a : b);
+}

[test]
 uniform 0 float4 1.0 0.0 0.0 0.0
+uniform 4 float4 1.0 2.0 3.0 4.0
+uniform 8 float4 5.0 6.0 7.0 8.0
 draw quad
-probe all rgba (0.5, 0.2, 0.7, 0.0)
+probe all rgba (1.0, 2.0, 3.0, 4.0)

-[require]
-% reset requirements

+[pixel shader todo]

-[pixel shader fail(sm>=6)]
-float4 x, y, z;
+// "scalar" can mean any 1-component numeric type.
+static float1x1 cond = {0};
+static float4 a = {1, 2, 3, 4};
+static float4 b = {5, 6, 7, 8};

float4 main() : sv_target
 {
-    return x ? y : z;
+    return float4(cond ? a : b);
 }

[test]
-uniform 0 float4 0.0 1.0 0.0 -3.0
+draw quad
+probe all rgba (5.0, 6.0, 7.0, 8.0)
+
+
+[pixel shader todo]
+
+uniform float4 cond;
+uniform float4 a, b;
+
+float4 main() : sv_target
+{
+    return float4(cond ? a.x : b.x);
+}
+
+[test]
+uniform 0 float4 1.0 0.0 1.0 0.0
 uniform 4 float4 1.0 2.0 3.0 4.0
 uniform 8 float4 5.0 6.0 7.0 8.0
+todo draw quad
+probe all rgba (1.0, 5.0, 1.0, 5.0)
+
+
+[pixel shader todo]
+
+// "scalar" can mean any 1-component numeric type.
+static float4 cond = {1, 0, 0, 1};
+static float1x1 a = {2};
+static float1x1 b = {3};
+
+float4 main() : sv_target
+{
+    return float4(cond ? a : b);
+}
+
+[test]
 draw quad
-probe all rgba (5.0, 2.0, 7.0, 4.0)
+probe all rgba (2.0, 3.0, 3.0, 2.0)
+
+
+% Objects can be used, but their types have to be identical.
+
+[pixel shader todo]
+Texture2D t;
+
+float4 main() : sv_target
+{
+    Texture2D tmp = 0 ? t : t;
+    return 0;
+}
+
+
+[pixel shader fail]
+Texture2D t;
+float4 f;
+
+float4 main() : sv_target
+{
+    f ? t : t;
+    return 0;
+}
+
+
+[pixel shader fail]
+Texture2D t;
+Texture3D u;
+
+float4 main() : sv_target
+{
+    0 ? t : u;
+    return 0;
+}
+
+
+% Of course objects cannot be used as the condition.
+
+[pixel shader fail todo]
+Texture2D t;
+
+float4 main() : sv_target
+{
+    t ? 0 : 1;
+    return 0;
+}

[PATCH v5 4/5] vkd3d-shader/hlsl: Separate an add_ternary() helper.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl.y | 37 ++++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 5e0d48d3c..7927af592 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -3998,6 +3998,27 @@ static struct hlsl_block *add_constructor(struct hlsl_ctx *ctx, struct hlsl_type
     return params->instrs;
 }

+static bool add_ternary(struct hlsl_ctx *ctx, struct hlsl_block *block,
+        struct hlsl_ir_node *cond, struct hlsl_ir_node *first, struct hlsl_ir_node *second)
+{
+    struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {0};
+    struct hlsl_type *common_type;
+
+    if (!(common_type = get_common_numeric_type(ctx, first, second, &first->loc)))
+        return false;
+
+    if (!(first = add_implicit_conversion(ctx, block, first, common_type, &first->loc)))
+        return false;
+
+    if (!(second = add_implicit_conversion(ctx, block, second, common_type, &second->loc)))
+        return false;
+
+    args[0] = cond;
+    args[1] = first;
+    args[2] = second;
+    return add_expr(ctx, block, HLSL_OP3_TERNARY, args, common_type, &first->loc);
+}
+
 static unsigned int hlsl_offset_dim_count(enum hlsl_sampler_dim dim)
 {
     switch (dim)
@@ -7061,27 +7082,13 @@ conditional_expr:
             struct hlsl_ir_node *cond = node_from_block($1);
             struct hlsl_ir_node *first = node_from_block($3);
             struct hlsl_ir_node *second = node_from_block($5);
-            struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = { 0 };
-            struct hlsl_type *common_type;

hlsl_block_add_block($1, $3);
             hlsl_block_add_block($1, $5);
             destroy_block($3);
             destroy_block($5);

-            if (!(common_type = get_common_numeric_type(ctx, first, second, &@3)))
-                YYABORT;
-
-            if (!(first = add_implicit_conversion(ctx, $1, first, common_type, &@3)))
-                YYABORT;
-
-            if (!(second = add_implicit_conversion(ctx, $1, second, common_type, &@5)))
-                YYABORT;
-
-            args[0] = cond;
-            args[1] = first;
-            args[2] = second;
-            if (!add_expr(ctx, $1, HLSL_OP3_TERNARY, args, common_type, &@1))
+            if (!add_ternary(ctx, $1, cond, first, second))
                 YYABORT;
             $$ = $1;
         }

[PATCH v5 5/5] vkd3d-shader/hlsl: Allow non-numeric types in the ternary operator.

[Zebediah Figura]

From: Zebediah Figura

---
 libs/vkd3d-shader/hlsl.y       | 34 ++++++++++++++++++++++++++++------
 tests/hlsl/ternary.shader_test |  2 +-
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl.y b/libs/vkd3d-shader/hlsl.y
index 7927af592..13275e796 100644
--- a/libs/vkd3d-shader/hlsl.y
+++ b/libs/vkd3d-shader/hlsl.y
@@ -4004,14 +4004,36 @@ static bool add_ternary(struct hlsl_ctx *ctx, struct hlsl_block *block,
     struct hlsl_ir_node *args[HLSL_MAX_OPERANDS] = {0};
     struct hlsl_type *common_type;

-    if (!(common_type = get_common_numeric_type(ctx, first, second, &first->loc)))
-        return false;
+    if (first->data_type->class <= HLSL_CLASS_LAST_NUMERIC
+            && second->data_type->class <= HLSL_CLASS_LAST_NUMERIC)
+    {
+        if (!(common_type = get_common_numeric_type(ctx, first, second, &first->loc)))
+            return false;

-    if (!(first = add_implicit_conversion(ctx, block, first, common_type, &first->loc)))
-        return false;
+        if (!(first = add_implicit_conversion(ctx, block, first, common_type, &first->loc)))
+            return false;

-    if (!(second = add_implicit_conversion(ctx, block, second, common_type, &second->loc)))
-        return false;
+        if (!(second = add_implicit_conversion(ctx, block, second, common_type, &second->loc)))
+            return false;
+    }
+    else
+    {
+        struct vkd3d_string_buffer *first_string, *second_string;
+
+        if (!hlsl_types_are_equal(first->data_type, second->data_type))
+        {
+            first_string = hlsl_type_to_string(ctx, first->data_type);
+            second_string = hlsl_type_to_string(ctx, second->data_type);
+            if (first_string && second_string)
+                hlsl_error(ctx, &first->loc, VKD3D_SHADER_ERROR_HLSL_INVALID_TYPE,
+                        "Ternary argument types '%s' and '%s' do not match.",
+                        first_string->buffer, second_string->buffer);
+            hlsl_release_string_buffer(ctx, first_string);
+            hlsl_release_string_buffer(ctx, second_string);
+        }
+
+        common_type = first->data_type;
+    }

args[0] = cond;
     args[1] = first;
diff --git a/tests/hlsl/ternary.shader_test b/tests/hlsl/ternary.shader_test
index 70ae20f96..25899dd8c 100644
--- a/tests/hlsl/ternary.shader_test
+++ b/tests/hlsl/ternary.shader_test
@@ -329,7 +329,7 @@ float4 main() : sv_target
 }

-[pixel shader fail]
+[pixel shader fail todo]
 Texture2D t;
 float4 f;

[Zebediah Figura (＠zfigura)]

Sorry for the double push, I went and added [test] blocks anyway, then realized I had forgotten to address the other comment.

[Giovanni Mascellani (＠giomasce)]

The failure persisted after retrying the pipeline, and it also happens on my computer, so it doesn't look like an occasional hiccup. Can you reproduce it too?