[Mesa-dev] [PATCH 3/5] glsl: Convert ir_call to be a statement rather than an rvalue.
Paul Berry
stereotype441 at gmail.com
Wed Sep 21 13:47:27 PDT 2011
On 20 September 2011 18:28, Kenneth Graunke <kenneth at whitecape.org> wrote:
> This begins the process of cleaning up and un-muddling our IR.
>
> 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 at whitecape.org>
> ---
> src/glsl/ast_function.cpp | 49 ++++++++------------
> src/glsl/builtins/ir/acos | 23 ++++++---
> src/glsl/builtins/ir/atan | 80
> ++++++++++++++++++---------------
> src/glsl/ir.cpp | 2 -
> src/glsl/ir.h | 21 ++++++---
> src/glsl/ir_basic_block.cpp | 46 +------------------
> src/glsl/ir_clone.cpp | 6 ++-
> src/glsl/ir_constant_expression.cpp | 3 -
> src/glsl/ir_expression_flattening.cpp | 6 +--
> src/glsl/ir_hv_accept.cpp | 6 +++
> src/glsl/ir_print_visitor.cpp | 5 ++-
> src/glsl/ir_reader.cpp | 32 ++++++++++---
> src/glsl/ir_validate.cpp | 12 +++++
> src/glsl/linker.cpp | 1 +
> src/glsl/opt_constant_folding.cpp | 10 ++++
> src/glsl/opt_constant_variable.cpp | 12 +++++
> src/glsl/opt_dead_code.cpp | 3 +-
> src/glsl/opt_dead_code_local.cpp | 7 +---
> src/glsl/opt_function_inlining.cpp | 62 ++-----------------------
> 19 files changed, 179 insertions(+), 207 deletions(-)
>
> diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
> index 392b7f9..c7d9c06 100644
> --- a/src/glsl/ast_function.cpp
> +++ b/src/glsl/ast_function.cpp
> @@ -258,31 +258,24 @@ match_function_by_name(exec_list *instructions, const
> char *name,
> formal_iter.next();
> }
>
> - /* Always insert the call in the instruction stream, and return a
> deref
> - * of its return val if it returns a value, since we don't know if
> - * the rvalue is going to be assigned to anything or not.
> + /* If the function call is a constant expression, don't
> + * generate the instructions to call it; just generate an
> + * ir_constant representing the constant value.
> *
> - * Also insert any out parameter conversions after the call.
> + * Function calls can only be constant expressions starting
> + * in GLSL 1.20.
> */
> - ir_call *call = new(ctx) ir_call(sig, actual_parameters);
> - ir_dereference_variable *deref;
> - if (!sig->return_type->is_void()) {
> - /* If the function call is a constant expression, don't
> - * generate the instructions to call it; just generate an
> - * ir_constant representing the constant value.
> - *
> - * Function calls can only be constant expressions starting
> - * in GLSL 1.20.
> - */
> - if (state->language_version >= 120) {
> - ir_constant *const_val = call->constant_expression_value();
> - if (const_val) {
> - return const_val;
> - }
> - }
> + if (state->language_version >= 120) {
> + ir_constant *value =
> sig->constant_expression_value(actual_parameters);
> + if (value != NULL) {
> + return value;
> + }
> + }
>
> + ir_dereference_variable *deref = NULL;
> + if (!sig->return_type->is_void()) {
> + /* Create a new temporary to hold the return value */
> ir_variable *var;
> -
> var = new(ctx) ir_variable(sig->return_type,
> ralloc_asprintf(ctx, "%s_retval",
> sig->function_name()),
> @@ -290,16 +283,14 @@ match_function_by_name(exec_list *instructions, const
> char *name,
> instructions->push_tail(var);
>
> deref = new(ctx) ir_dereference_variable(var);
> - ir_assignment *assign = new(ctx) ir_assignment(deref, call, NULL);
> - instructions->push_tail(assign);
> -
> - deref = new(ctx) ir_dereference_variable(var);
> - } else {
> - instructions->push_tail(call);
> - deref = NULL;
> }
> + ir_call *call = new(ctx) ir_call(sig, deref, actual_parameters);
> + instructions->push_tail(call);
> +
> + /* Also emit any necessary out-parameter conversions. */
> instructions->append_list(&post_call_conversions);
> - return deref;
> +
> + return deref ? deref->clone(ctx, NULL) : NULL;
> } else {
> char *str = prototype_string(NULL, name, actual_parameters);
>
> diff --git a/src/glsl/builtins/ir/acos b/src/glsl/builtins/ir/acos
> index d1cfebe..f0078f8 100644
> --- a/src/glsl/builtins/ir/acos
> +++ b/src/glsl/builtins/ir/acos
> @@ -2,21 +2,28 @@
> (signature float
> (parameters
> (declare (in) float x))
> - ((return (expression float - (constant float (1.5707963))
> - (call asin ((var_ref x)))))))
> + ((declare () float s)
> + (call asin (var_ref s) ((var_ref x)))
> + (return (expression float - (constant float (1.5707963)) (var_ref
> s)))))
> +
> (signature vec2
> (parameters
> (declare (in) vec2 x))
> - ((return (expression vec2 - (constant float (1.5707963))
> - (call asin ((var_ref x)))))))
> + ((declare () vec2 s)
> + (call asin (var_ref s) ((var_ref x)))
> + (return (expression vec2 - (constant float (1.5707963)) (var_ref
> s)))))
> +
> (signature vec3
> (parameters
> (declare (in) vec3 x))
> - ((return (expression vec3 - (constant float (1.5707963))
> - (call asin ((var_ref x)))))))
> + ((declare () vec3 s)
> + (call asin (var_ref s) ((var_ref x)))
> + (return (expression vec3 - (constant float (1.5707963)) (var_ref
> s)))))
> +
> (signature vec4
> (parameters
> (declare (in) vec4 x))
> - ((return (expression vec4 - (constant float (1.5707963))
> - (call asin ((var_ref x)))))))
> + ((declare () vec4 s)
> + (call asin (var_ref s) ((var_ref x)))
> + (return (expression vec4 - (constant float (1.5707963)) (var_ref
> s)))))
> ))
> diff --git a/src/glsl/builtins/ir/atan b/src/glsl/builtins/ir/atan
> index 7b5ea13..a9dc08e 100644
> --- a/src/glsl/builtins/ir/atan
> +++ b/src/glsl/builtins/ir/atan
> @@ -2,50 +2,62 @@
> (signature float
> (parameters
> (declare (in) float y_over_x))
> - ((return (call asin ((expression float *
> + ((declare () float s)
> + (call asin (var_ref s)
> + ((expression float *
> (var_ref y_over_x)
> (expression float rsq
> (expression float +
> (expression float *
> (var_ref y_over_x)
> (var_ref y_over_x))
> - (constant float (1.0))))))))))
> + (constant float (1.0)))))))
> + (return (var_ref s))))
>
> (signature vec2
> (parameters
> (declare (in) vec2 y_over_x))
> - ((return (call asin ((expression vec2 *
> + ((declare () vec2 s)
> + (call asin (var_ref s)
> + ((expression vec2 *
> (var_ref y_over_x)
> (expression vec2 rsq
> (expression vec2 +
> (expression vec2 *
> (var_ref y_over_x)
> (var_ref y_over_x))
> - (constant float (1.0))))))))))
> + (constant float (1.0)))))))
> + (return (var_ref s))))
>
> (signature vec3
> (parameters
> (declare (in) vec3 y_over_x))
> - ((return (call asin ((expression vec3 *
> + ((declare () vec3 s)
> + (call asin (var_ref s)
> + ((expression vec3 *
> (var_ref y_over_x)
> (expression vec3 rsq
> (expression vec3 +
> (expression vec3 *
> (var_ref y_over_x)
> (var_ref y_over_x))
> - (constant float (1.0))))))))))
> + (constant float (1.0)))))))
> + (return (var_ref s))))
>
> (signature vec4
> (parameters
> (declare (in) vec4 y_over_x))
> - ((return (call asin ((expression vec4 *
> + ((declare () vec4 s)
> + (call asin (var_ref s)
> + ((expression vec4 *
> (var_ref y_over_x)
> (expression vec4 rsq
> (expression vec4 +
> (expression vec4 *
> (var_ref y_over_x)
> (var_ref y_over_x))
> - (constant float (1.0))))))))))
> + (constant float (1.0)))))))
> + (return (var_ref s))))
>
> (signature float
> (parameters
> @@ -57,7 +69,7 @@
> (if (expression bool >
> (expression float abs (var_ref x))
> (expression float * (constant float (1.0e-8)) (expression float
> abs (var_ref y)))) (
> - (assign (x) (var_ref r) (call atan ((expression float / (var_ref
> y) (var_ref x)))))
> + (call atan (var_ref r) ((expression float / (var_ref y) (var_ref
> x))))
> (if (expression bool < (var_ref x) (constant float (0.000000)) ) (
> (if (expression bool >= (var_ref y) (constant float (0.000000)) )
> ((assign (x) (var_ref r) (expression float + (var_ref r)
> (constant float (3.141593)))))
> @@ -82,12 +94,11 @@
> (declare (in) vec2 y)
> (declare (in) vec2 x))
> ((declare () vec2 r)
> - (assign (x) (var_ref r)
> - (call atan ((swiz x (var_ref y))
> - (swiz x (var_ref x)))))
> - (assign (y) (var_ref r)
> - (call atan ((swiz y (var_ref y))
> - (swiz y (var_ref x)))))
> + (declare () float temp)
> + (call atan (var_ref temp) ((swiz x (var_ref y)) (swiz x (var_ref
> x))))
> + (assign (x) (var_ref r) (var_ref temp))
> + (call atan (var_ref temp) ((swiz y (var_ref y)) (swiz y (var_ref
> x))))
> + (assign (y) (var_ref r) (var_ref temp))
> (return (var_ref r))))
>
> (signature vec3
> @@ -95,15 +106,13 @@
> (declare (in) vec3 y)
> (declare (in) vec3 x))
> ((declare () vec3 r)
> - (assign (x) (var_ref r)
> - (call atan ((swiz x (var_ref y))
> - (swiz x (var_ref x)))))
> - (assign (y) (var_ref r)
> - (call atan ((swiz y (var_ref y))
> - (swiz y (var_ref x)))))
> - (assign (z) (var_ref r)
> - (call atan ((swiz z (var_ref y))
> - (swiz z (var_ref x)))))
> + (declare () float temp)
> + (call atan (var_ref temp) ((swiz x (var_ref y)) (swiz x (var_ref
> x))))
> + (assign (x) (var_ref r) (var_ref temp))
> + (call atan (var_ref temp) ((swiz y (var_ref y)) (swiz y (var_ref
> x))))
> + (assign (y) (var_ref r) (var_ref temp))
> + (call atan (var_ref temp) ((swiz z (var_ref y)) (swiz z (var_ref
> x))))
> + (assign (z) (var_ref r) (var_ref temp))
> (return (var_ref r))))
>
> (signature vec4
> @@ -111,18 +120,15 @@
> (declare (in) vec4 y)
> (declare (in) vec4 x))
> ((declare () vec4 r)
> - (assign (x) (var_ref r)
> - (call atan ((swiz x (var_ref y))
> - (swiz x (var_ref x)))))
> - (assign (y) (var_ref r)
> - (call atan ((swiz y (var_ref y))
> - (swiz y (var_ref x)))))
> - (assign (z) (var_ref r)
> - (call atan ((swiz z (var_ref y))
> - (swiz z (var_ref x)))))
> - (assign (w) (var_ref r)
> - (call atan ((swiz w (var_ref y))
> - (swiz w (var_ref x)))))
> - (return (var_ref r)))))
> + (declare () float temp)
> + (call atan (var_ref temp) ((swiz x (var_ref y)) (swiz x (var_ref
> x))))
> + (assign (x) (var_ref r) (var_ref temp))
> + (call atan (var_ref temp) ((swiz y (var_ref y)) (swiz y (var_ref
> x))))
> + (assign (y) (var_ref r) (var_ref temp))
> + (call atan (var_ref temp) ((swiz z (var_ref y)) (swiz z (var_ref
> x))))
> + (assign (z) (var_ref r) (var_ref temp))
> + (call atan (var_ref temp) ((swiz w (var_ref y)) (swiz w (var_ref
> x))))
> + (assign (w) (var_ref r) (var_ref temp))
> + (return (var_ref r))))
>
> ))
> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> index 70d0ae2..92a7601 100644
> --- a/src/glsl/ir.cpp
> +++ b/src/glsl/ir.cpp
> @@ -1458,8 +1458,6 @@ ir_function::has_user_signature()
> void
> ir_call::set_callee(ir_function_signature *sig)
> {
> - assert((this->type == NULL) || (this->type == sig->return_type));
> -
> this->callee = sig;
> }
>
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index 9bf3b70..11deaf6 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -978,16 +978,17 @@ public:
>
>
> /**
> - * IR instruction representing a function call
> + * HIR instruction representing a high-level function call, containing a
> + * list of parameters, and returning a value in the supplied temporary.
> */
> -class ir_call : public ir_rvalue {
> +class ir_call : public ir_instruction {
> public:
> - ir_call(ir_function_signature *callee, exec_list *actual_parameters)
> - : callee(callee)
> + ir_call(ir_function_signature *callee, ir_dereference *return_deref,
> + exec_list *actual_parameters)
> + : return_deref(return_deref), callee(callee)
> {
> ir_type = ir_type_call;
> assert(callee->return_type != NULL);
> - type = callee->return_type;
> actual_parameters->move_nodes_to(& this->actual_parameters);
> this->use_builtin = callee->is_builtin;
> }
> @@ -1039,9 +1040,15 @@ public:
>
> /**
> * Generates an inline version of the function before @ir,
> - * returning the return value of the function.
> + * storing the return value in return_deref.
> */
> - ir_rvalue *generate_inline(ir_instruction *ir);
> + void generate_inline(ir_instruction *ir);
> +
> + /**
> + * Storage for the function's return value.
> + * This should be NULL if the return type is void.
> + */
> + ir_dereference *return_deref;
>
> /* List of ir_rvalue of paramaters passed in this call. */
> exec_list actual_parameters;
> diff --git a/src/glsl/ir_basic_block.cpp b/src/glsl/ir_basic_block.cpp
> index a833825..5ebbf6f 100644
> --- a/src/glsl/ir_basic_block.cpp
> +++ b/src/glsl/ir_basic_block.cpp
> @@ -32,31 +32,6 @@
> #include "ir_basic_block.h"
> #include "glsl_types.h"
>
> -class ir_has_call_visitor : public ir_hierarchical_visitor {
> -public:
> - ir_has_call_visitor()
> - {
> - has_call = false;
> - }
> -
> - virtual ir_visitor_status visit_enter(ir_call *ir)
> - {
> - (void) ir;
> - has_call = true;
> - return visit_stop;
> - }
> -
> - bool has_call;
> -};
> -
> -bool
> -ir_has_call(ir_instruction *ir)
> -{
> - ir_has_call_visitor v;
> - ir->accept(&v);
> - return v.has_call;
> -}
> -
> /**
> * Calls a user function for every basic block in the instruction stream.
> *
> @@ -122,24 +97,9 @@ void call_for_basic_blocks(exec_list *instructions,
>
> call_for_basic_blocks(&ir_sig->body, callback, data);
> }
> - } else if (ir->as_assignment()) {
> - /* If there's a call in the expression tree being assigned,
> - * then that ends the BB too.
> - *
> - * The assumption is that any consumer of the basic block
> - * walker is fine with the fact that the call is somewhere in
> - * the tree even if portions of the tree may be evaluated
> - * after the call.
> - *
> - * A consumer that has an issue with this could not process
> - * the last instruction of the basic block. If doing so,
> - * expression flattener may be useful before using the basic
> - * block finder to get more maximal basic blocks out.
> - */
> - if (ir_has_call(ir)) {
> - callback(leader, ir, data);
> - leader = NULL;
> - }
> + } else if (ir->as_call()) {
> + callback(leader, ir, data);
> + leader = NULL;
> }
> last = ir;
> }
> diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
> index e8b946a..3993ff3 100644
> --- a/src/glsl/ir_clone.cpp
> +++ b/src/glsl/ir_clone.cpp
> @@ -156,6 +156,10 @@ ir_loop::clone(void *mem_ctx, struct hash_table *ht)
> const
> ir_call *
> ir_call::clone(void *mem_ctx, struct hash_table *ht) const
> {
> + ir_dereference *new_return_ref = NULL;
> + if (this->return_deref != NULL)
> + new_return_ref = this->return_deref->clone(mem_ctx, ht);
> +
> exec_list new_parameters;
>
> foreach_iter(exec_list_iterator, iter, this->actual_parameters) {
> @@ -163,7 +167,7 @@ ir_call::clone(void *mem_ctx, struct hash_table *ht)
> const
> new_parameters.push_tail(ir->clone(mem_ctx, ht));
> }
>
> - return new(mem_ctx) ir_call(this->callee, &new_parameters);
> + return new(mem_ctx) ir_call(this->callee, new_return_ref,
> &new_parameters);
> }
>
> ir_expression *
> diff --git a/src/glsl/ir_constant_expression.cpp
> b/src/glsl/ir_constant_expression.cpp
> index 4264847..bc0b589 100644
> --- a/src/glsl/ir_constant_expression.cpp
> +++ b/src/glsl/ir_constant_expression.cpp
> @@ -983,9 +983,6 @@ ir_constant::constant_expression_value()
> ir_constant *
> ir_call::constant_expression_value()
> {
> - if (this->type == glsl_type::error_type)
> - return NULL;
> -
> return
> this->callee->constant_expression_value(&this->actual_parameters);
> }
>
I believe the deleted if-test needs to be moved into
ir_constant::constant_expression_value().
>
> diff --git a/src/glsl/ir_expression_flattening.cpp
> b/src/glsl/ir_expression_flattening.cpp
> index 0b7c537..3922da8 100644
> --- a/src/glsl/ir_expression_flattening.cpp
> +++ b/src/glsl/ir_expression_flattening.cpp
> @@ -27,10 +27,8 @@
> * Takes the leaves of expression trees and makes them dereferences of
> * assignments of the leaves to temporaries, according to a predicate.
> *
> - * This is used for automatic function inlining, where we want to take
> - * an expression containing a call and move the call out to its own
> - * assignment so that we can inline it at the appropriate place in the
> - * instruction stream.
> + * This is used for breaking down matrix operations, where it's easier to
> + * create a temporary and work on each of its vectory components
> individually.
> */
>
> #include "ir.h"
> diff --git a/src/glsl/ir_hv_accept.cpp b/src/glsl/ir_hv_accept.cpp
> index d33fc85..f8cd776 100644
> --- a/src/glsl/ir_hv_accept.cpp
> +++ b/src/glsl/ir_hv_accept.cpp
> @@ -317,6 +317,12 @@ ir_call::accept(ir_hierarchical_visitor *v)
> if (s != visit_continue)
> return (s == visit_continue_with_parent) ? visit_continue : s;
>
> + if (this->return_deref != NULL) {
> + s = this->return_deref->accept(v);
> + if (s != visit_continue)
> + return (s == visit_continue_with_parent) ? visit_continue : s;
> + }
> +
>
I think we need to set in_assignee here (in much the same way that
ir_assignment::accept() does for the LHS of an assignment). But I'm not
100% certain about that, since we don't do it for out and inout parameters
(I wouldn't be surprised if there are some lurking bugs because of this).
> s = visit_list_elements(v, &this->actual_parameters);
> if (s == visit_stop)
> return s;
> diff --git a/src/glsl/ir_print_visitor.cpp b/src/glsl/ir_print_visitor.cpp
> index ea78582..b0a9c58 100644
> --- a/src/glsl/ir_print_visitor.cpp
> +++ b/src/glsl/ir_print_visitor.cpp
> @@ -409,7 +409,10 @@ void ir_print_visitor::visit(ir_constant *ir)
> void
> ir_print_visitor::visit(ir_call *ir)
> {
> - printf("(call %s (", ir->callee_name());
> + printf("(call %s ", ir->callee_name());
> + if (ir->return_deref)
> + ir->return_deref->accept(this);
> + printf(" (");
> foreach_iter(exec_list_iterator, iter, *ir) {
> ir_instruction *const inst = (ir_instruction *) iter.get();
>
> diff --git a/src/glsl/ir_reader.cpp b/src/glsl/ir_reader.cpp
> index 2d0bccb..bd341fe 100644
> --- a/src/glsl/ir_reader.cpp
> +++ b/src/glsl/ir_reader.cpp
> @@ -51,11 +51,11 @@ private:
> ir_variable *read_declaration(s_expression *);
> ir_if *read_if(s_expression *, ir_loop *);
> ir_loop *read_loop(s_expression *);
> + ir_call *read_call(s_expression *);
> ir_return *read_return(s_expression *);
> ir_rvalue *read_rvalue(s_expression *);
> ir_assignment *read_assignment(s_expression *);
> ir_expression *read_expression(s_expression *);
> - ir_call *read_call(s_expression *);
> ir_swizzle *read_swizzle(s_expression *);
> ir_constant *read_constant(s_expression *);
> ir_texture *read_texture(s_expression *);
> @@ -347,6 +347,8 @@ ir_reader::read_instruction(s_expression *expr, ir_loop
> *loop_ctx)
> inst = read_if(list, loop_ctx);
> } else if (strcmp(tag->value(), "loop") == 0) {
> inst = read_loop(list);
> + } else if (strcmp(tag->value(), "call") == 0) {
> + inst = read_call(list);
> } else if (strcmp(tag->value(), "return") == 0) {
> inst = read_return(list);
> } else if (strcmp(tag->value(), "function") == 0) {
> @@ -520,8 +522,6 @@ ir_reader::read_rvalue(s_expression *expr)
> rvalue = read_swizzle(list);
> } else if (strcmp(tag->value(), "expression") == 0) {
> rvalue = read_expression(list);
> - } else if (strcmp(tag->value(), "call") == 0) {
> - rvalue = read_call(list);
> } else if (strcmp(tag->value(), "constant") == 0) {
> rvalue = read_constant(list);
> } else {
> @@ -609,10 +609,20 @@ ir_reader::read_call(s_expression *expr)
> {
> s_symbol *name;
> s_list *params;
> + s_list *s_return = NULL;
>
> - s_pattern pat[] = { "call", name, params };
> - if (!MATCH(expr, pat)) {
> - ir_read_error(expr, "expected (call <name> (<param> ...))");
> + ir_dereference *return_deref = NULL;
> +
> + s_pattern void_pat[] = { "call", name, params };
> + s_pattern non_void_pat[] = { "call", name, s_return, params };
> + if (MATCH(expr, non_void_pat)) {
> + return_deref = read_dereference(s_return);
> + if (return_deref == NULL) {
> + ir_read_error(s_return, "when reading return deref of call");
> + return NULL;
> + }
> + } else if (!MATCH(expr, void_pat)) {
> + ir_read_error(expr, "expected (call <name> [<deref>] (<param>
> ...))");
> return NULL;
> }
>
> @@ -642,7 +652,15 @@ ir_reader::read_call(s_expression *expr)
> return NULL;
> }
>
> - return new(mem_ctx) ir_call(callee, ¶meters);
> + if (callee->return_type == glsl_type::void_type && return_deref) {
> + ir_read_error(expr, "call has return value storage but void type");
> + return NULL;
> + } else if (callee->return_type != glsl_type::void_type &&
> !return_deref) {
> + ir_read_error(expr, "call has non-void type but no return value
> storage");
> + return NULL;
> + }
> +
> + return new(mem_ctx) ir_call(callee, return_deref, ¶meters);
> }
>
> ir_expression *
> diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
> index 2d1c609..848305c 100644
> --- a/src/glsl/ir_validate.cpp
> +++ b/src/glsl/ir_validate.cpp
> @@ -537,6 +537,18 @@ ir_validate::visit_enter(ir_call *ir)
> {
> ir_function_signature *const callee = ir->get_callee();
>
> + if (callee->return_type != glsl_type::void_type) {
> + if (ir->return_deref == NULL) {
> + printf("ir_call has non-void callee but no return storage\n");
> + abort();
> + }
> + if (callee->return_type != ir->return_deref->type) {
> + printf("callee type %s does not match return storage type %s\n",
> + callee->return_type->name, ir->return_deref->type->name);
> + abort();
> + }
> + }
> +
>
Validation should also fail if callee->return_type == glsl_type::void_type
but ir->return_deref != NULL.
> if (callee->ir_type != ir_type_function_signature) {
> printf("IR called by ir_call is not ir_function_signature!\n");
> abort();
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 195f58f..1606a67 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -754,6 +754,7 @@ move_non_declarations(exec_list *instructions,
> exec_node *last,
> continue;
>
> assert(inst->as_assignment()
> + || inst->as_call()
> || ((var != NULL) && (var->mode == ir_var_temporary)));
>
> if (make_copies) {
> diff --git a/src/glsl/opt_constant_folding.cpp
> b/src/glsl/opt_constant_folding.cpp
> index 599b215..db8cd5c 100644
> --- a/src/glsl/opt_constant_folding.cpp
> +++ b/src/glsl/opt_constant_folding.cpp
> @@ -117,6 +117,7 @@ ir_constant_folding_visitor::visit_enter(ir_assignment
> *ir)
> ir_visitor_status
> ir_constant_folding_visitor::visit_enter(ir_call *ir)
> {
> + /* Attempt to constant fold parameters */
> exec_list_iterator sig_iter = ir->get_callee()->parameters.iterator();
> foreach_iter(exec_list_iterator, iter, *ir) {
> ir_rvalue *param_rval = (ir_rvalue *)iter.get();
> @@ -133,6 +134,15 @@ ir_constant_folding_visitor::visit_enter(ir_call *ir)
> sig_iter.next();
> }
>
> + /* Next, see if the call can be replaced with an assignment of a
> constant */
> + ir_constant *const_val = ir->constant_expression_value();
> +
> + if (const_val != NULL) {
> + ir_assignment *assignment =
> + new(ralloc_parent(ir)) ir_assignment(ir->return_deref, const_val);
> + ir->replace_with(assignment);
> + }
> +
> return visit_continue_with_parent;
> }
>
> diff --git a/src/glsl/opt_constant_variable.cpp
> b/src/glsl/opt_constant_variable.cpp
> index 3fa7c3b..18c2801 100644
> --- a/src/glsl/opt_constant_variable.cpp
> +++ b/src/glsl/opt_constant_variable.cpp
> @@ -127,6 +127,7 @@ ir_constant_variable_visitor::visit_enter(ir_assignment
> *ir)
> ir_visitor_status
> ir_constant_variable_visitor::visit_enter(ir_call *ir)
> {
> + /* Mark any out parameters as assigned to */
> exec_list_iterator sig_iter = ir->get_callee()->parameters.iterator();
> foreach_iter(exec_list_iterator, iter, *ir) {
> ir_rvalue *param_rval = (ir_rvalue *)iter.get();
> @@ -143,6 +144,17 @@ ir_constant_variable_visitor::visit_enter(ir_call *ir)
> }
> sig_iter.next();
> }
> +
> + /* Mark the return storage as having been assigned to */
> + if (ir->return_deref != NULL) {
> + ir_variable *var = ir->return_deref->variable_referenced();
> + struct assignment_entry *entry;
> +
> + assert(var);
> + entry = get_assignment_entry(var, &this->list);
> + entry->assignment_count++;
> + }
> +
> return visit_continue;
> }
>
> diff --git a/src/glsl/opt_dead_code.cpp b/src/glsl/opt_dead_code.cpp
> index cb500d2..35609e9 100644
> --- a/src/glsl/opt_dead_code.cpp
> +++ b/src/glsl/opt_dead_code.cpp
> @@ -78,8 +78,7 @@ do_dead_code(exec_list *instructions)
> * Don't do so if it's a shader output, though.
> */
> if (entry->var->mode != ir_var_out &&
> - entry->var->mode != ir_var_inout &&
> - !ir_has_call(entry->assign)) {
> + entry->var->mode != ir_var_inout) {
> entry->assign->remove();
> progress = true;
>
> diff --git a/src/glsl/opt_dead_code_local.cpp
> b/src/glsl/opt_dead_code_local.cpp
> index 39962bd..a81a38f 100644
> --- a/src/glsl/opt_dead_code_local.cpp
> +++ b/src/glsl/opt_dead_code_local.cpp
> @@ -149,12 +149,7 @@ process_assignment(void *ctx, ir_assignment *ir,
> exec_list *assignments)
> }
> }
>
> - /* Add this instruction to the assignment list available to be removed.
> - * But not if the assignment has other side effects.
> - */
> - if (ir_has_call(ir))
> - return progress;
> -
> + /* Add this instruction to the assignment list available to be removed.
> */
> assignment_entry *entry = new(ctx) assignment_entry(var, ir);
> assignments->push_tail(entry);
>
> diff --git a/src/glsl/opt_function_inlining.cpp
> b/src/glsl/opt_function_inlining.cpp
> index 8fef358..4138dbe 100644
> --- a/src/glsl/opt_function_inlining.cpp
> +++ b/src/glsl/opt_function_inlining.cpp
> @@ -54,7 +54,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 *);
> @@ -64,23 +63,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;
> @@ -90,12 +76,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
> @@ -107,14 +93,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);
> @@ -125,13 +110,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.
> */
> @@ -186,7 +164,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
> @@ -239,11 +217,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;
> }
>
>
> @@ -283,13 +256,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;
> }
> @@ -298,25 +265,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."
> *
> --
> 1.7.6.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Are we making it a rule that an ir_call's return_deref needs to always be an
ir_dereference_variable? If so, we should probably use the more specific
type in the class declaration (or at the very least add code to
ir_validate.cpp to check this). If not, then a few other visitors need to
be updated:
- ir_vec_index_to_cond_assign_visitor: do the appropriate lowering on an
ir_call's return_deref.
- ir_vec_index_to_swizzle_visitor: do the appropriate lowering on an
ir_call's return_deref.
Also, regardless of the type of return_deref, I think these visitors need to
be updated:
- find_assignment_visitor (in linker.cpp): needs to check ir_call's
return_deref to see if it is the variable being sought.
- ir_tree_grafting_visitor: kill the graft if an ir_call's return_deref
refers to a variable that's present in the expression being grafted.
- loop_analysis: in the visitor for ir_dereference_variable, handle the case
where in_assignee is true but we're inside a return_deref.
- discard_simplifier, ir_if_simplification_visitor, and
redundant_jumps_visitor: add a visit_enter(ir_call *) function that returns
visit_continue_with_parent, just like those visitors do for ir_assignment.
Finally, I was kind of surprised to discover that the visitor classes
ir_to_mesa_visitor and glsl_to_tgsi_visitor have visit methods for ir_call.
I thought we inlined everything before converting to Mesa IR or TGSI. So
maybe these visit methods are dead code? If they are, we should probably
replace the dead code with assert(!"not reached"). If they aren't, then
they need to be updated to handle return_deref properly.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20110921/20995ce2/attachment-0001.htm>
More information about the mesa-dev
mailing list