[PATCH 0/2] MR331: vkd3d-shader/hlsl: Lower matrix swizzles.

[Francisco Casas (＠fcasas)]

As @nsivov pointed out, we are currently parsing matrix swizzles (e.g. mat._m12_m32) and creating an hlsl_ir_swizzle for the matrix itself. We are currently allowing shaders that use these to compile even though they are not working.

While our compilation passes don't expect hlsl_ir_swizzles with matrix values, turns out we need them as a parse-time construct because we don't know if they will be used as the lhs of an assignment during parsing, similarly to hlsl_ir_indexes.

This patch adds tests and a pass to lower these matrix swizzles.

Implementing assignments with matrix swizzles is still pending.

[PATCH 1/2] tests: Test matrix swizzles.

[Francisco Casas]

From: Francisco Casas

---
 Makefile.am                           |   1 +
 tests/hlsl/swizzle-matrix.shader_test | 202 ++++++++++++++++++++++++++
 2 files changed, 203 insertions(+)
 create mode 100644 tests/hlsl/swizzle-matrix.shader_test

diff --git a/Makefile.am b/Makefile.am
index b36358b2..e869df1f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -164,6 +164,7 @@ vkd3d_shader_tests = \
    tests/hlsl/struct-assignment.shader_test \
    tests/hlsl/struct-semantics.shader_test \
    tests/hlsl/swizzle-constant-prop.shader_test \
+	tests/hlsl/swizzle-matrix.shader_test \
    tests/hlsl/swizzles.shader_test \
    tests/hlsl/ternary.shader_test \
    tests/hlsl/texture-load-offset.shader_test \
diff --git a/tests/hlsl/swizzle-matrix.shader_test b/tests/hlsl/swizzle-matrix.shader_test
new file mode 100644
index 00000000..2602d9dd
--- /dev/null
+++ b/tests/hlsl/swizzle-matrix.shader_test
@@ -0,0 +1,202 @@
+[pixel shader]
+float3x2 mat;
+
+float4 main() : sv_target
+{
+    return mat._21_31_11_12;
+}
+
+[test]
+uniform 0 float4 11 21 31 -1
+uniform 4 float4 12 22 32 -1
+draw quad
+todo probe all rgba (21.0, 31.0, 11.0, 12.0)
+
+
+[pixel shader]
+float3x2 mat;
+
+float4 main() : sv_target
+{
+    return mat._m00_m20_m01_m21;
+}
+
+[test]
+uniform 0 float4 11 21 31 -1
+uniform 4 float4 12 22 32 -1
+draw quad
+todo probe all rgba (11.0, 31.0, 12.0, 32.0)
+
+
+[pixel shader]
+row_major float3x2 mat;
+
+float4 main() : sv_target
+{
+    return mat._11_31_12_32;
+}
+
+[test]
+uniform 0 float4 11 12 -1 -1
+uniform 4 float4 21 22 -1 -1
+uniform 8 float4 31 32 -1 -1
+draw quad
+todo probe all rgba (11.0, 31.0, 12.0, 32.0)
+
+
+[pixel shader]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+    float2 p = mat._11_32;
+
+    return float4(p, p);
+}
+
+[test]
+draw quad
+todo probe all rgba (1.0, 6.0, 1.0, 6.0)
+
+
+% zero-based and one-based subscripts cannot be used in the same swizzle.
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+    float2 p = mat._11_m00;
+
+    return 0.0;
+}
+
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+    float p = mat._41;
+
+    return 0.0;
+}
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+    float p = mat._33;
+
+    return 0.0;
+}
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+    float p = mat._m30;
+
+    return 0.0;
+}
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+    float p = mat._m22;
+
+    return 0.0;
+}
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+    float p = mat._00;
+
+    return 0.0;
+}
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+
+    return mat._11_11_11_11_11;
+}
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3x2 mat = {1, 2, 3, 4, 5, 6};
+    float p = mat.x
+
+    return 0;
+}
+
+[pixel shader fail]
+float4 main() : sv_target
+{
+    float3 vec = {1, 2, 3};
+    float p = vec._11;
+
+    return 0;
+}
+
+% Like with vectors, a single-component swizzle is scalar.
+[pixel shader]
+float4 main() : SV_TARGET
+{
+    float4 vec = {10, 20, 30, 40};
+    int2x3 idx = {0, 1, 2, 3, 4, 5};
+
+    return vec[idx._21];
+}
+
+[test]
+draw quad
+todo probe all rgba (40.0, 40.0, 40.0, 40.0)
+
+
+[pixel shader todo]
+float3 a;
+
+float4 main() : sv_target
+{
+    float2x2 mat = {1, 2, 3, 4};
+
+    mat._11 = 10;
+    mat._m01_m10_m11 = a;
+    return float4(mat);
+}
+
+[test]
+uniform 0 float4 20 30 40 -1
+todo draw quad
+todo probe all rgba (10.0, 20.0, 30.0, 40.0)
+
+
+[pixel shader todo]
+float3 a;
+
+float4 main() : sv_target
+{
+    row_major float4x2 mat = {1, 2, 3, 4, 5, 6, 7, 8};
+
+    mat._11 = 10;
+    mat._m01_m10_m31 = a;
+    return float4(mat._m31_m10_m01_m00);
+}
+
+[test]
+uniform 0 float4 20 30 80 -1
+todo draw quad
+todo probe all rgba (80.0, 30.0, 20.0, 10.0)
+
+
+% Cannot repeat components when assigning to a swizzle.
+[pixel shader fail todo]
+float4 main() : sv_target
+{
+    float2x2 mat = {1, 2, 3, 4};
+
+    mat._m01_m01 = float2(20, 20);
+    return 0;
+}

[PATCH 2/2] vkd3d-shader/hlsl: Lower matrix swizzles.

[Francisco Casas]

From: Francisco Casas

---
 libs/vkd3d-shader/hlsl_codegen.c      | 54 +++++++++++++++++++++++++++
 tests/hlsl/swizzle-matrix.shader_test | 10 ++---
 2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 710d2908..1822bef6 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -934,6 +934,59 @@ static struct hlsl_ir_node *add_zero_mipmap_level(struct hlsl_ctx *ctx, struct h
     return &coords_load->node;
 }

+/* hlsl_ir_swizzle nodes that directly point to a matrix value are only a parse-time construct that
+ * represents matrix swizzles (e.g. mat._m01_m23) before we know if they will be used in the lhs of
+ * an assignment or as a value made from different components of the matrix. The former cases should
+ * have already been split into several separate assignments, but the latter are lowered by this
+ * pass. */
+static bool lower_matrix_swizzles(struct hlsl_ctx *ctx, struct hlsl_ir_node *instr, void *context)
+{
+    struct hlsl_ir_swizzle *swizzle;
+    struct hlsl_ir_load *var_load;
+    struct hlsl_deref var_deref;
+    struct hlsl_type *matrix_type;
+    struct hlsl_ir_var *var;
+    unsigned int x, y, k, i;
+
+    if (instr->type != HLSL_IR_SWIZZLE)
+        return false;
+    swizzle = hlsl_ir_swizzle(instr);
+    matrix_type = swizzle->val.node->data_type;
+    if (matrix_type->class != HLSL_CLASS_MATRIX)
+        return false;
+
+    if (!(var = hlsl_new_synthetic_var(ctx, "matrix-swizzle", instr->data_type, &instr->loc)))
+        return false;
+    hlsl_init_simple_deref_from_var(&var_deref, var);
+
+    for (i = 0; i < instr->data_type->dimx; ++i)
+    {
+        struct hlsl_block block, store_block;
+        struct hlsl_ir_node *load;
+
+        y = (swizzle->swizzle >> (8 * i + 4)) & 0xf;
+        x = (swizzle->swizzle >> 8 * i) & 0xf;
+        k = y * matrix_type->dimx + x;
+
+        hlsl_block_init(&block);
+        if (!(load = hlsl_add_load_component(ctx, &block,  swizzle->val.node, k, &instr->loc)))
+            return false;
+
+        if (!hlsl_new_store_component(ctx, &store_block, &var_deref, i, load))
+            return false;
+        hlsl_block_add_block(&block, &store_block);
+
+        list_move_before(&instr->entry, &block.instrs);
+    }
+
+    if (!(var_load = hlsl_new_var_load(ctx, var, &instr->loc)))
+        return false;
+    list_add_before(&instr->entry, &var_load->node.entry);
+    hlsl_replace_node(instr, &var_load->node);
+
+    return true;
+}
+
 /* hlsl_ir_index nodes are a parse-time construct used to represent array indexing and struct
  * record access before knowing if they will be used in the lhs of an assignment --in which case
  * they are lowered into a deref-- or as the load of an element within a larger value.
@@ -4247,6 +4300,7 @@ int hlsl_emit_bytecode(struct hlsl_ctx *ctx, struct hlsl_ir_function_decl *entry

while (hlsl_transform_ir(ctx, lower_calls, body, NULL));

+    hlsl_transform_ir(ctx, lower_matrix_swizzles, body, NULL);
     hlsl_transform_ir(ctx, lower_index_loads, body, NULL);

LIST_FOR_EACH_ENTRY(var, &ctx->globals->vars, struct hlsl_ir_var, scope_entry)
diff --git a/tests/hlsl/swizzle-matrix.shader_test b/tests/hlsl/swizzle-matrix.shader_test
index 2602d9dd..bc2814eb 100644
--- a/tests/hlsl/swizzle-matrix.shader_test
+++ b/tests/hlsl/swizzle-matrix.shader_test
@@ -10,7 +10,7 @@ float4 main() : sv_target
 uniform 0 float4 11 21 31 -1
 uniform 4 float4 12 22 32 -1
 draw quad
-todo probe all rgba (21.0, 31.0, 11.0, 12.0)
+probe all rgba (21.0, 31.0, 11.0, 12.0)

[pixel shader]
@@ -25,7 +25,7 @@ float4 main() : sv_target
 uniform 0 float4 11 21 31 -1
 uniform 4 float4 12 22 32 -1
 draw quad
-todo probe all rgba (11.0, 31.0, 12.0, 32.0)
+probe all rgba (11.0, 31.0, 12.0, 32.0)

[pixel shader]
@@ -41,7 +41,7 @@ uniform 0 float4 11 12 -1 -1
 uniform 4 float4 21 22 -1 -1
 uniform 8 float4 31 32 -1 -1
 draw quad
-todo probe all rgba (11.0, 31.0, 12.0, 32.0)
+probe all rgba (11.0, 31.0, 12.0, 32.0)

[pixel shader]
@@ -55,7 +55,7 @@ float4 main() : sv_target

[test]
 draw quad
-todo probe all rgba (1.0, 6.0, 1.0, 6.0)
+probe all rgba (1.0, 6.0, 1.0, 6.0)

% zero-based and one-based subscripts cannot be used in the same swizzle.
@@ -152,7 +152,7 @@ float4 main() : SV_TARGET

[test]
 draw quad
-todo probe all rgba (40.0, 40.0, 40.0, 40.0)
+probe all rgba (40.0, 40.0, 40.0, 40.0)

[pixel shader todo]

[PATCH 0/2] MR331: vkd3d-shader/hlsl: Lower matrix swizzles. - approved

[Giovanni Mascellani (＠giomasce)]

This merge request was approved by Giovanni Mascellani.

[PATCH 0/2] MR331: vkd3d-shader/hlsl: Lower matrix swizzles. - approved

[Zebediah Figura (＠zfigura)]

This merge request was approved by Zebediah Figura.

[Francisco Casas (＠fcasas)]

Note: Got to remember that lower_matrix_swizzles() could use lower_ir() like in !334, if that gets upstreamed first.

[Zebediah Figura (＠zfigura)]

> Note: Got to remember that lower_matrix_swizzles() could use lower_ir() like in !334, if that gets upstreamed first.

IIRC lower_ir() already exists, so you could rebase this now; I just forgot about it since it's new :-)