[Mesa-dev] [PATCH v2] glsl: fix lowering outputs for early/nested returns

[Lars Hamre]

v2: limit lowerings to return statments in main()

Return statements in conditional blocks were not having their
output varyings lowered correctly.

This patch fixes the following piglit tests:
/spec/glsl-1.10/execution/vs-float-main-return
/spec/glsl-1.10/execution/vs-vec2-main-return
/spec/glsl-1.10/execution/vs-vec3-main-return

Signed-off-by: Lars Hamre <chemecse at gmail.com>

---

CC: Timothy Arceri <timothy.arceri at collabora.com>

Hi Timothy,

As it turns out, functions are inlined prior to
lower_packed_varyings() being called. I put in the check
to not visit other functions anyways in case that changes
at some point in the future.

I could not determine a way to test having the splicer
inserting instructions into a function that is not main()
within piglit. Inserting the lowering instructions before
a function's return statement just inserts "useless"
instructions, but does not produce incorrect results.
Please nudge me in the right direction if you have an idea.

There already exists piglit tests which check float, vec2,
and vec3 varyings for early returns (which this patch
fixes). Let me know if you have anything else to test for
in mind.

Regards,
Lars Hamre

NOTE: Someone with access will need to commit this after the review
      process

 src/compiler/glsl/lower_packed_varyings.cpp | 68 +++++++++++++++++++++++++++--
 1 file changed, 64 insertions(+), 4 deletions(-)

diff --git a/src/compiler/glsl/lower_packed_varyings.cpp b/src/compiler/glsl/lower_packed_varyings.cpp
index 825cc9e..c9197b7 100644
--- a/src/compiler/glsl/lower_packed_varyings.cpp
+++ b/src/compiler/glsl/lower_packed_varyings.cpp
@@ -724,6 +724,55 @@ lower_packed_varyings_gs_splicer::visit_leave(ir_emit_vertex *ev)
    return visit_continue;
 }

+/**
+ * Visitor that splices varying packing code before every return in main().
+ */
+class lower_packed_varyings_return_splicer : public ir_hierarchical_visitor
+{
+public:
+   explicit lower_packed_varyings_return_splicer(void *mem_ctx,
+                                                 const exec_list *instructions);
+
+   virtual ir_visitor_status visit_leave(ir_return *ret);
+   virtual ir_visitor_status visit_enter(ir_function_signature *sig);
+
+private:
+   /**
+    * Memory context used to allocate new instructions for the shader.
+    */
+   void * const mem_ctx;
+
+   /**
+    * Instructions that should be spliced into place before each return.
+    */
+   const exec_list *instructions;
+};
+
+
+lower_packed_varyings_return_splicer::lower_packed_varyings_return_splicer(
+      void *mem_ctx, const exec_list *instructions)
+   : mem_ctx(mem_ctx), instructions(instructions)
+{
+}
+
+ir_visitor_status
+lower_packed_varyings_return_splicer::visit_leave(ir_return *ret)
+{
+   foreach_in_list(ir_instruction, ir, this->instructions) {
+      ret->insert_before(ir->clone(this->mem_ctx, NULL));
+   }
+   return visit_continue;
+}
+
+ir_visitor_status
+lower_packed_varyings_return_splicer::visit_enter(ir_function_signature *sig)
+{
+   /* Only splice returns into main(). */
+   if (!strcmp(sig->function_name(), "main"))
+      return visit_continue;
+   return visit_continue_with_parent;
+}
+

 void
 lower_packed_varyings(void *mem_ctx, unsigned locations_used,
@@ -757,11 +806,22 @@ lower_packed_varyings(void *mem_ctx, unsigned locations_used,
          /* Now update all the EmitVertex instances */
          splicer.run(instructions);
       } else {
-         /* For other shader types, outputs need to be lowered at the end of
-          * main()
+         /* For other shader types, outputs need to be lowered before each
+          * return statement and at the end of main()
+          */
+
+         lower_packed_varyings_return_splicer splicer(mem_ctx, &new_instructions);
+
+         main_func_sig->body.head->insert_before(&new_variables);
+
+         splicer.run(instructions);
+
+         /* Lower outputs at the end of main() if the last instruction is not
+          * a return statement
           */
-         main_func_sig->body.append_list(&new_variables);
-         main_func_sig->body.append_list(&new_instructions);
+         if (((ir_instruction*)instructions->get_tail())->ir_type != ir_type_return) {
+            main_func_sig->body.append_list(&new_instructions);
+         }
       }
    } else {
       /* Shader inputs need to be lowered at the beginning of main() */
--
2.5.5