[PATCH v8 2/2] vkd3d-shader/hlsl: Avoid infinite loop and invalid derefs in copy-prop.

[Giovanni Mascellani]

From: Giovanni Mascellani

Co-authored-by: Francisco Casas 
Co-authored-by: Zebediah Figura

Because copy_propagation_transform_object_load() replaces a deref
instead of an instruction, it is currently prone to two problems:

1- It can replace a deref with the same deref, returning true every
time and getting the compilation stuck in an endless loop of
copy-propagation iterations.

2- When performed multiple times in the same deref, the second time it
can replace the deref with a deref from a temp that is only valid in
another point of the program execution, resulting in an incorrect value.

This patch preempts this by avoiding replacing derefs when the new deref
doesn't point to a uniform variable. Because, uniform variables cannot
be written to.
---
 libs/vkd3d-shader/hlsl_codegen.c    | 20 +++++++++++++++
 tests/object-references.shader_test | 39 ++++++++++++++++++++++++++++-
 2 files changed, 58 insertions(+), 1 deletion(-)

diff --git a/libs/vkd3d-shader/hlsl_codegen.c b/libs/vkd3d-shader/hlsl_codegen.c
index 5e7cfff4..07ad8858 100644
--- a/libs/vkd3d-shader/hlsl_codegen.c
+++ b/libs/vkd3d-shader/hlsl_codegen.c
@@ -927,6 +927,26 @@ static bool copy_propagation_transform_object_load(struct hlsl_ctx *ctx,
     /* Only HLSL_IR_LOAD can produce an object. */
     load = hlsl_ir_load(value->node);

+    /* As we are replacing the instruction's deref (with the one in the hlsl_ir_load) and not the
+     * instruction itself, we won't be able to rely on the value retrieved by
+     * copy_propagation_get_value() for the new deref in subsequent iterations of copy propagation.
+     * This is because another value may be written to that deref between the hlsl_ir_load and
+     * this instruction.
+     *
+     * For this reason, we only replace the new deref when it corresponds to a uniform variable,
+     * which cannot be written to.
+     *
+     * In a valid shader, all object references must resolve statically to a single uniform object.
+     * If this is the case, we can expect copy propagation on regular store/loads and the other
+     * compilation passes to replace all hlsl_ir_loads with loads to uniform objects, so this
+     * implementation is complete, even with this restriction.
+     */
+    if (!load->src.var->is_uniform)
+    {
+        TRACE("Ignoring load from non-uniform object variable %s\n", load->src.var->name);
+        return false;
+    }
+
     hlsl_cleanup_deref(deref);
     hlsl_copy_deref(ctx, deref, &load->src);

diff --git a/tests/object-references.shader_test b/tests/object-references.shader_test
index 0c203d5a..bc74ccd4 100644
--- a/tests/object-references.shader_test
+++ b/tests/object-references.shader_test
@@ -1,3 +1,15 @@
+[pixel shader fail]
+sampler sam;
+
+float4 main() : sv_target
+{
+    Texture2D tex;
+
+    tex = tex; // Uninitialized assignment to self.
+    return tex.Sample(sam, float2(0, 0));
+}
+
+
 [require]
 shader model >= 4.0

@@ -35,7 +47,7 @@ float4 main() : sv_target

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

[texture 0]
@@ -201,3 +213,28 @@ float4 main(Texture2D tex2) : sv_target
     tex2 = tex1;
     return tex2.Load(int3(0, 0, 0));
 }
+
+
+[require]
+shader model >= 5.0
+
+
+[texture 0]
+size (1, 1)
+1.0 2.0 3.0 4.0
+
+[pixel shader todo]
+struct apple {
+    Texture2D tex;
+    float4 fo : COLOR;
+};
+
+float4 main(struct apple input) : sv_target
+{
+    input.tex = input.tex; // Assignment to self.
+    return input.tex.Load(int3(0, 0, 0));
+}
+
+[test]
+todo draw quad
+todo probe (0, 0) rgba (1.0, 2.0, 3.0, 4.0)