glsl: Don't trust loop analysis in the presence of function calls.

Function calls may have side effects that alter variables used inside
the loop.  In the fragment shader, they may even terminate the shader.
This means our analysis about loop-constant or induction variables may
be completely wrong.

In general it's impossible to determine whether they actually do or not
(due to the halting problem), so we'd need to perform conservative
static analysis.  For now, it's not worth the complexity: most functions
will be inlined, at which point we can unroll them successfully.

Fixes Piglit tests:
- shaders/glsl-fs-unroll-out-param
- shaders/glsl-fs-unroll-side-effect

NOTE: This is a candidate for release branches.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

diff --git a/src/glsl/loop_analysis.cpp b/src/glsl/loop_analysis.cpp
index 9bba6a97c488ac198482b7512ca198d71a3bbb61..6a0e4da51004cd1c67547f631f3e770c7204bbc5 100644 (file)
--- a/src/glsl/loop_analysis.cpp
+++ b/src/glsl/loop_analysis.cpp
@@ -110,6 +110,8 @@ public:
    virtual ir_visitor_status visit(ir_loop_jump *);
    virtual ir_visitor_status visit(ir_dereference_variable *);
 
+   virtual ir_visitor_status visit_enter(ir_call *);
+
    virtual ir_visitor_status visit_enter(ir_loop *);
    virtual ir_visitor_status visit_leave(ir_loop *);
    virtual ir_visitor_status visit_enter(ir_assignment *);
@@ -152,6 +154,21 @@ loop_analysis::visit(ir_loop_jump *ir)
 }
 
 
+ir_visitor_status
+loop_analysis::visit_enter(ir_call *ir)
+{
+   /* If we're not somewhere inside a loop, there's nothing to do. */
+   if (this->state.is_empty())
+      return visit_continue;
+
+   loop_variable_state *const ls =
+      (loop_variable_state *) this->state.get_head();
+
+   ls->contains_calls = true;
+   return visit_continue_with_parent;
+}
+
+
 ir_visitor_status
 loop_analysis::visit(ir_dereference_variable *ir)
 {
@@ -209,6 +226,17 @@ loop_analysis::visit_leave(ir_loop *ir)
    loop_variable_state *const ls =
       (loop_variable_state *) this->state.pop_head();
 
+   /* Function calls may contain side effects.  These could alter any of our
+    * variables in ways that cannot be known, and may even terminate shader
+    * execution (say, calling discard in the fragment shader).  So we can't
+    * rely on any of our analysis about assignments to variables.
+    *
+    * We could perform some conservative analysis (prove there's no statically
+    * possible assignment, etc.) but it isn't worth it for now; function
+    * inlining will allow us to unroll loops anyway.
+    */
+   if (ls->contains_calls)
+      return visit_continue;
 
    foreach_list(node, &ir->body_instructions) {
       /* Skip over declarations at the start of a loop.

diff --git a/src/glsl/loop_analysis.h b/src/glsl/loop_analysis.h
index 229730836a87039f4e805d339eb7f57671f55f56..8bed1db02106368e78d6c333bd8ae3fde6948a68 100644 (file)
--- a/src/glsl/loop_analysis.h
+++ b/src/glsl/loop_analysis.h
@@ -122,10 +122,16 @@ public:
     */
    unsigned num_loop_jumps;
 
+   /**
+    * Whether this loop contains any function calls.
+    */
+   bool contains_calls;
+
    loop_variable_state()
    {
       this->max_iterations = -1;
       this->num_loop_jumps = 0;
+      this->contains_calls = false;
       this->var_hash = hash_table_ctor(0, hash_table_pointer_hash,
                                       hash_table_pointer_compare);
    }