glsl: Convert ir_call to be a statement rather than a value.

Aside from ir_call, our IR is cleanly split into two classes:
- Statements (typeless; used for side effects, control flow)
- Values (deeply nestable, pure, typed expression trees)

Unfortunately, ir_call confused all this:
- For void functions, we placed ir_call directly in the instruction
  stream, treating it as an untyped statement.  Yet, it was a subclass
  of ir_rvalue, and no other ir_rvalue could be used in this way.
- For functions with a return value, ir_call could be placed in
  arbitrary expression trees.  While this fit naturally with the source
  language, it meant that expressions might not be pure, making it
  difficult to transform and optimize them.  To combat this, we always
  emitted ir_call directly in the RHS of an ir_assignment, only using
  a temporary variable in expression trees.  Many passes relied on this
  assumption; the acos and atan built-ins violated it.

This patch makes ir_call a statement (ir_instruction) rather than a
value (ir_rvalue).  Non-void calls now take a ir_dereference of a
variable, and store the return value there---effectively a call and
assignment rolled into one.  They cannot be embedded in expressions.

All expression trees are now pure, without exception.

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

1 files changed, 5 insertions, 57 deletions

diff --git a/src/glsl/opt_function_inlining.cpp b/src/glsl/opt_function_inlining.cpp
index ec8b72c..4ff4d97 100644
--- a/src/glsl/opt_function_inlining.cpp
+++ b/src/glsl/opt_function_inlining.cpp
@@ -53,7 +53,6 @@ public:
 
    virtual ir_visitor_status visit_enter(ir_expression *);
    virtual ir_visitor_status visit_enter(ir_call *);
-   virtual ir_visitor_status visit_enter(ir_assignment *);
    virtual ir_visitor_status visit_enter(ir_return *);
    virtual ir_visitor_status visit_enter(ir_texture *);
    virtual ir_visitor_status visit_enter(ir_swizzle *);
@@ -63,23 +62,10 @@ public:
 
 
 bool
-automatic_inlining_predicate(ir_instruction *ir)
-{
-   ir_call *call = ir->as_call();
-
-   if (call && can_inline(call))
-      return true;
-
-   return false;
-}
-
-bool
 do_function_inlining(exec_list *instructions)
 {
    ir_function_inlining_visitor v;
 
-   do_expression_flattening(instructions, automatic_inlining_predicate);
-
    v.run(instructions);
 
    return v.progress;
@@ -89,12 +75,12 @@ static void
 replace_return_with_assignment(ir_instruction *ir, void *data)
 {
    void *ctx = ralloc_parent(ir);
-   ir_variable *retval = (ir_variable *)data;
+   ir_dereference *orig_deref = (ir_dereference *) data;
    ir_return *ret = ir->as_return();
 
    if (ret) {
       if (ret->value) {
-	 ir_rvalue *lhs = new(ctx) ir_dereference_variable(retval);
+	 ir_rvalue *lhs = orig_deref->clone(ctx, NULL);
 	 ret->replace_with(new(ctx) ir_assignment(lhs, ret->value, NULL));
       } else {
 	 /* un-valued return has to be the last return, or we shouldn't
@@ -106,14 +92,13 @@ replace_return_with_assignment(ir_instruction *ir, void *data)
    }
 }
 
-ir_rvalue *
+void
 ir_call::generate_inline(ir_instruction *next_ir)
 {
    void *ctx = ralloc_parent(this);
    ir_variable **parameters;
    int num_parameters;
    int i;
-   ir_variable *retval = NULL;
    struct hash_table *ht;
 
    ht = hash_table_ctor(0, hash_table_pointer_hash, hash_table_pointer_compare);
@@ -124,13 +109,6 @@ ir_call::generate_inline(ir_instruction *next_ir)
 
    parameters = new ir_variable *[num_parameters];
 
-   /* Generate storage for the return value. */
-   if (!this->callee->return_type->is_void()) {
-      retval = new(ctx) ir_variable(this->callee->return_type, "_ret_val",
-				    ir_var_auto);
-      next_ir->insert_before(retval);
-   }
-
    /* Generate the declarations for the parameters to our inlined code,
     * and set up the mapping of real function body variables to ours.
     */
@@ -185,7 +163,7 @@ ir_call::generate_inline(ir_instruction *next_ir)
       ir_instruction *new_ir = ir->clone(ctx, ht);
 
       new_instructions.push_tail(new_ir);
-      visit_tree(new_ir, replace_return_with_assignment, retval);
+      visit_tree(new_ir, replace_return_with_assignment, this->return_deref);
    }
 
    /* If any samplers were passed in, replace any deref of the sampler
@@ -238,11 +216,6 @@ ir_call::generate_inline(ir_instruction *next_ir)
    delete [] parameters;
 
    hash_table_dtor(ht);
-
-   if (retval)
-      return new(ctx) ir_dereference_variable(retval);
-   else
-      return NULL;
 }
 
 
@@ -282,13 +255,7 @@ ir_visitor_status
 ir_function_inlining_visitor::visit_enter(ir_call *ir)
 {
    if (can_inline(ir)) {
-      /* If the call was part of some tree, then it should have been
-       * flattened out or we shouldn't have seen it because of a
-       * visit_continue_with_parent in this visitor.
-       */
-      assert(ir == base_ir);
-
-      (void) ir->generate_inline(ir);
+      ir->generate_inline(ir);
       ir->remove();
       this->progress = true;
    }
@@ -297,25 +264,6 @@ ir_function_inlining_visitor::visit_enter(ir_call *ir)
 }
 
 
-ir_visitor_status
-ir_function_inlining_visitor::visit_enter(ir_assignment *ir)
-{
-   ir_call *call = ir->rhs->as_call();
-   if (!call || !can_inline(call))
-      return visit_continue;
-
-   /* generates the parameter setup, function body, and returns the return
-    * value of the function
-    */
-   ir_rvalue *rhs = call->generate_inline(ir);
-   assert(rhs);
-
-   ir->rhs = rhs;
-   this->progress = true;
-
-   return visit_continue;
-}
-
 /**
  * Replaces references to the "sampler" variable with a clone of "deref."
  *