Mesa (master): glsl: Use ir_rvalue to represent generic error_type values.

Kenneth Graunke kwg at kemper.freedesktop.org
Mon Apr 2 21:23:10 UTC 2012


Module: Mesa
Branch: master
Commit: 807e967c615dc80a264af5a89af7649f95481744
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=807e967c615dc80a264af5a89af7649f95481744

Author: Kenneth Graunke <kenneth at whitecape.org>
Date:   Thu Sep 22 15:04:56 2011 -0700

glsl: Use ir_rvalue to represent generic error_type values.

Currently, ir_call can be used as either a statement (for void
functions) or a value (for non-void functions).  This is rather awkward,
as it's the only class that can be used in both forms.

A number of places use ir_call::get_error_instruction() to construct a
generic value of error_type.  If ir_call is to become a statement, it
can no longer serve this purpose.

Unfortunately, none of our classes are particularly well suited for
this, and creating a new one would be rather aggrandizing.  So, this
patch introduces ir_rvalue::error_value(), a static method that creates
an instance of the base class, ir_rvalue.  This has the nice property
that you can't accidentally try and access uninitialized fields (as it
doesn't have any).  The downside is that the base class is no longer
abstract.

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

---

 src/glsl/ast_function.cpp            |   32 +++++++++++++++---------------
 src/glsl/ast_to_hir.cpp              |    2 +-
 src/glsl/hir_field_selection.cpp     |    2 +-
 src/glsl/ir.cpp                      |   10 ++++----
 src/glsl/ir.h                        |   35 +++++++++++++++++++--------------
 src/glsl/ir_clone.cpp                |   10 ++++++--
 src/glsl/ir_constant_expression.cpp  |    7 ++++++
 src/glsl/ir_hierarchical_visitor.cpp |    9 ++++++++
 src/glsl/ir_hierarchical_visitor.h   |    1 +
 src/glsl/ir_hv_accept.cpp            |    7 ++++++
 src/glsl/ir_print_visitor.cpp        |    4 +++
 src/glsl/ir_print_visitor.h          |    1 +
 src/glsl/ir_visitor.h                |    1 +
 13 files changed, 80 insertions(+), 41 deletions(-)

diff --git a/src/glsl/ast_function.cpp b/src/glsl/ast_function.cpp
index 9ffbce6..934bc5e 100644
--- a/src/glsl/ast_function.cpp
+++ b/src/glsl/ast_function.cpp
@@ -602,7 +602,7 @@ process_array_constructor(exec_list *instructions,
 		       "parameter%s",
 		       (constructor_type->length != 0) ? "at least" : "exactly",
 		       min_param, (min_param <= 1) ? "" : "s");
-      return ir_call::get_error_instruction(ctx);
+      return ir_rvalue::error_value(ctx);
    }
 
    if (constructor_type->length == 0) {
@@ -1239,7 +1239,7 @@ ast_function_expression::hir(exec_list *instructions,
 	 _mesa_glsl_error(& loc, state, "unknown type `%s' (structure name "
 			  "may be shadowed by a variable with the same name)",
 			  type->type_name);
-	 return ir_call::get_error_instruction(ctx);
+	 return ir_rvalue::error_value(ctx);
       }
 
 
@@ -1248,14 +1248,14 @@ ast_function_expression::hir(exec_list *instructions,
       if (constructor_type->is_sampler()) {
 	 _mesa_glsl_error(& loc, state, "cannot construct sampler type `%s'",
 			  constructor_type->name);
-	 return ir_call::get_error_instruction(ctx);
+	 return ir_rvalue::error_value(ctx);
       }
 
       if (constructor_type->is_array()) {
 	 if (state->language_version <= 110) {
 	    _mesa_glsl_error(& loc, state,
 			     "array constructors forbidden in GLSL 1.10");
-	    return ir_call::get_error_instruction(ctx);
+	    return ir_rvalue::error_value(ctx);
 	 }
 
 	 return process_array_constructor(instructions, constructor_type,
@@ -1286,7 +1286,7 @@ ast_function_expression::hir(exec_list *instructions,
 				"insufficient parameters to constructor "
 				"for `%s'",
 				constructor_type->name);
-	       return ir_call::get_error_instruction(ctx);
+	       return ir_rvalue::error_value(ctx);
 	    }
 
 	    if (apply_implicit_conversion(constructor_type->fields.structure[i].type,
@@ -1300,7 +1300,7 @@ ast_function_expression::hir(exec_list *instructions,
 				constructor_type->fields.structure[i].name,
 				ir->type->name,
 				constructor_type->fields.structure[i].type->name);
-	       return ir_call::get_error_instruction(ctx);;
+	       return ir_rvalue::error_value(ctx);;
 	    }
 
 	    node = node->next;
@@ -1309,7 +1309,7 @@ ast_function_expression::hir(exec_list *instructions,
 	 if (!node->is_tail_sentinel()) {
 	    _mesa_glsl_error(&loc, state, "too many parameters in constructor "
 			     "for `%s'", constructor_type->name);
-	    return ir_call::get_error_instruction(ctx);
+	    return ir_rvalue::error_value(ctx);
 	 }
 
 	 ir_rvalue *const constant =
@@ -1323,7 +1323,7 @@ ast_function_expression::hir(exec_list *instructions,
       }
 
       if (!constructor_type->is_numeric() && !constructor_type->is_boolean())
-	 return ir_call::get_error_instruction(ctx);
+	 return ir_rvalue::error_value(ctx);
 
       /* Total number of components of the type being constructed. */
       const unsigned type_components = constructor_type->components();
@@ -1350,14 +1350,14 @@ ast_function_expression::hir(exec_list *instructions,
 	    _mesa_glsl_error(& loc, state, "too many parameters to `%s' "
 			     "constructor",
 			     constructor_type->name);
-	    return ir_call::get_error_instruction(ctx);
+	    return ir_rvalue::error_value(ctx);
 	 }
 
 	 if (!result->type->is_numeric() && !result->type->is_boolean()) {
 	    _mesa_glsl_error(& loc, state, "cannot construct `%s' from a "
 			     "non-numeric data type",
 			     constructor_type->name);
-	    return ir_call::get_error_instruction(ctx);
+	    return ir_rvalue::error_value(ctx);
 	 }
 
 	 /* Count the number of matrix and nonmatrix parameters.  This
@@ -1382,7 +1382,7 @@ ast_function_expression::hir(exec_list *instructions,
 	 _mesa_glsl_error(& loc, state, "cannot construct `%s' from a "
 			  "matrix in GLSL 1.10",
 			  constructor_type->name);
-	 return ir_call::get_error_instruction(ctx);
+	 return ir_rvalue::error_value(ctx);
       }
 
       /* From page 50 (page 56 of the PDF) of the GLSL 1.50 spec:
@@ -1396,7 +1396,7 @@ ast_function_expression::hir(exec_list *instructions,
 	 _mesa_glsl_error(& loc, state, "for matrix `%s' constructor, "
 			  "matrix must be only parameter",
 			  constructor_type->name);
-	 return ir_call::get_error_instruction(ctx);
+	 return ir_rvalue::error_value(ctx);
       }
 
       /* From page 28 (page 34 of the PDF) of the GLSL 1.10 spec:
@@ -1410,7 +1410,7 @@ ast_function_expression::hir(exec_list *instructions,
 	 _mesa_glsl_error(& loc, state, "too few components to construct "
 			  "`%s'",
 			  constructor_type->name);
-	 return ir_call::get_error_instruction(ctx);
+	 return ir_rvalue::error_value(ctx);
       }
 
       /* Later, we cast each parameter to the same base type as the
@@ -1505,10 +1505,10 @@ ast_function_expression::hir(exec_list *instructions,
       ir_rvalue *value = NULL;
       if (sig == NULL) {
 	 no_matching_function_error(func_name, &loc, &actual_parameters, state);
-	 value = ir_call::get_error_instruction(ctx);
+	 value = ir_rvalue::error_value(ctx);
       } else if (!verify_parameter_modes(state, sig, actual_parameters, this->expressions)) {
 	 /* an error has already been emitted */
-	 value = ir_call::get_error_instruction(ctx);
+	 value = ir_rvalue::error_value(ctx);
       } else {
 	 value = generate_call(instructions, sig, &loc, &actual_parameters,
 			       &call, state);
@@ -1517,5 +1517,5 @@ ast_function_expression::hir(exec_list *instructions,
       return value;
    }
 
-   return ir_call::get_error_instruction(ctx);
+   return ir_rvalue::error_value(ctx);
 }
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index ff56e33..6210eb1 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -1699,7 +1699,7 @@ ast_expression::hir(exec_list *instructions,
 	 _mesa_glsl_error(& loc, state, "`%s' undeclared",
 			  this->primary_expression.identifier);
 
-	 result = ir_call::get_error_instruction(ctx);
+	 result = ir_rvalue::error_value(ctx);
 	 error_emitted = true;
       }
       break;
diff --git a/src/glsl/hir_field_selection.cpp b/src/glsl/hir_field_selection.cpp
index 3c33127..260b415 100644
--- a/src/glsl/hir_field_selection.cpp
+++ b/src/glsl/hir_field_selection.cpp
@@ -98,5 +98,5 @@ _mesa_ast_field_selection_to_hir(const ast_expression *expr,
 		       expr->primary_expression.identifier);
    }
 
-   return result ? result : ir_call::get_error_instruction(ctx);
+   return result ? result : ir_rvalue::error_value(ctx);
 }
diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
index 3c9d6e1..b497ff1 100644
--- a/src/glsl/ir.cpp
+++ b/src/glsl/ir.cpp
@@ -1458,13 +1458,13 @@ ir_function::has_user_signature()
 }
 
 
-ir_call *
-ir_call::get_error_instruction(void *ctx)
+ir_rvalue *
+ir_rvalue::error_value(void *mem_ctx)
 {
-   ir_call *call = new(ctx) ir_call;
+   ir_rvalue *v = new(mem_ctx) ir_rvalue;
 
-   call->type = glsl_type::error_type;
-   return call;
+   v->type = glsl_type::error_type;
+   return v;
 }
 
 void
diff --git a/src/glsl/ir.h b/src/glsl/ir.h
index 1faae3c..811eac0 100644
--- a/src/glsl/ir.h
+++ b/src/glsl/ir.h
@@ -132,11 +132,21 @@ protected:
 };
 
 
+/**
+ * The base class for all "values"/expression trees.
+ */
 class ir_rvalue : public ir_instruction {
 public:
-   virtual ir_rvalue *clone(void *mem_ctx, struct hash_table *) const = 0;
+   virtual ir_rvalue *clone(void *mem_ctx, struct hash_table *) const;
 
-   virtual ir_constant *constant_expression_value() = 0;
+   virtual void accept(ir_visitor *v)
+   {
+      v->visit(this);
+   }
+
+   virtual ir_visitor_status accept(ir_hierarchical_visitor *);
+
+   virtual ir_constant *constant_expression_value();
 
    virtual ir_rvalue * as_rvalue()
    {
@@ -209,6 +219,14 @@ public:
     */
    virtual bool is_negative_one() const;
 
+
+   /**
+    * Return a generic value of error_type.
+    *
+    * Allocation will be performed with 'mem_ctx' as ralloc owner.
+    */
+   static ir_rvalue *error_value(void *mem_ctx);
+
 protected:
    ir_rvalue();
 };
@@ -1030,13 +1048,6 @@ public:
    virtual ir_visitor_status accept(ir_hierarchical_visitor *);
 
    /**
-    * Get a generic ir_call object when an error occurs
-    *
-    * Any allocation will be performed with 'ctx' as ralloc owner.
-    */
-   static ir_call *get_error_instruction(void *ctx);
-
-   /**
     * Get an iterator for the set of acutal parameters
     */
    exec_list_iterator iterator()
@@ -1078,12 +1089,6 @@ public:
    bool use_builtin;
 
 private:
-   ir_call()
-      : callee(NULL)
-   {
-      this->ir_type = ir_type_call;
-   }
-
    ir_function_signature *callee;
 };
 
diff --git a/src/glsl/ir_clone.cpp b/src/glsl/ir_clone.cpp
index c63615c..b6d0e8a 100644
--- a/src/glsl/ir_clone.cpp
+++ b/src/glsl/ir_clone.cpp
@@ -27,6 +27,13 @@
 #include "glsl_types.h"
 #include "program/hash_table.h"
 
+ir_rvalue *
+ir_rvalue::clone(void *mem_ctx, struct hash_table *ht) const
+{
+   /* The only possible instantiation is the generic error value. */
+   return error_value(mem_ctx);
+}
+
 /**
  * Duplicate an IR variable
  *
@@ -160,9 +167,6 @@ ir_loop::clone(void *mem_ctx, struct hash_table *ht) const
 ir_call *
 ir_call::clone(void *mem_ctx, struct hash_table *ht) const
 {
-   if (this->type == glsl_type::error_type)
-      return ir_call::get_error_instruction(mem_ctx);
-
    exec_list new_parameters;
 
    foreach_iter(exec_list_iterator, iter, this->actual_parameters) {
diff --git a/src/glsl/ir_constant_expression.cpp b/src/glsl/ir_constant_expression.cpp
index adca62e..2910b2e 100644
--- a/src/glsl/ir_constant_expression.cpp
+++ b/src/glsl/ir_constant_expression.cpp
@@ -71,6 +71,13 @@ dot(ir_constant *op0, ir_constant *op1)
 }
 
 ir_constant *
+ir_rvalue::constant_expression_value()
+{
+   assert(this->type->is_error());
+   return NULL;
+}
+
+ir_constant *
 ir_expression::constant_expression_value()
 {
    if (this->type->is_error())
diff --git a/src/glsl/ir_hierarchical_visitor.cpp b/src/glsl/ir_hierarchical_visitor.cpp
index b5eacd6..f244140 100644
--- a/src/glsl/ir_hierarchical_visitor.cpp
+++ b/src/glsl/ir_hierarchical_visitor.cpp
@@ -33,6 +33,15 @@ ir_hierarchical_visitor::ir_hierarchical_visitor()
 }
 
 ir_visitor_status
+ir_hierarchical_visitor::visit(ir_rvalue *ir)
+{
+   if (this->callback != NULL)
+      this->callback(ir, this->data);
+
+   return visit_continue;
+}
+
+ir_visitor_status
 ir_hierarchical_visitor::visit(ir_variable *ir)
 {
    if (this->callback != NULL)
diff --git a/src/glsl/ir_hierarchical_visitor.h b/src/glsl/ir_hierarchical_visitor.h
index bba046d..143eb7c 100644
--- a/src/glsl/ir_hierarchical_visitor.h
+++ b/src/glsl/ir_hierarchical_visitor.h
@@ -82,6 +82,7 @@ public:
     * \name Visit methods for leaf-node classes
     */
    /*@{*/
+   virtual ir_visitor_status visit(class ir_rvalue *);
    virtual ir_visitor_status visit(class ir_variable *);
    virtual ir_visitor_status visit(class ir_constant *);
    virtual ir_visitor_status visit(class ir_loop_jump *);
diff --git a/src/glsl/ir_hv_accept.cpp b/src/glsl/ir_hv_accept.cpp
index 0e78fda..c3be052 100644
--- a/src/glsl/ir_hv_accept.cpp
+++ b/src/glsl/ir_hv_accept.cpp
@@ -66,6 +66,13 @@ visit_list_elements(ir_hierarchical_visitor *v, exec_list *l,
 
 
 ir_visitor_status
+ir_rvalue::accept(ir_hierarchical_visitor *v)
+{
+   return v->visit(this);
+}
+
+
+ir_visitor_status
 ir_variable::accept(ir_hierarchical_visitor *v)
 {
    return v->visit(this);
diff --git a/src/glsl/ir_print_visitor.cpp b/src/glsl/ir_print_visitor.cpp
index 1471355..78ba0ba 100644
--- a/src/glsl/ir_print_visitor.cpp
+++ b/src/glsl/ir_print_visitor.cpp
@@ -135,6 +135,10 @@ print_type(const glsl_type *t)
    }
 }
 
+void ir_print_visitor::visit(ir_rvalue *ir)
+{
+   printf("error");
+}
 
 void ir_print_visitor::visit(ir_variable *ir)
 {
diff --git a/src/glsl/ir_print_visitor.h b/src/glsl/ir_print_visitor.h
index c7136f1..6c308f3 100644
--- a/src/glsl/ir_print_visitor.h
+++ b/src/glsl/ir_print_visitor.h
@@ -54,6 +54,7 @@ public:
     * the hierarchy should not have \c visit methods.
     */
    /*@{*/
+   virtual void visit(ir_rvalue *);
    virtual void visit(ir_variable *);
    virtual void visit(ir_function_signature *);
    virtual void visit(ir_function *);
diff --git a/src/glsl/ir_visitor.h b/src/glsl/ir_visitor.h
index 7dd35fe..4a00155 100644
--- a/src/glsl/ir_visitor.h
+++ b/src/glsl/ir_visitor.h
@@ -44,6 +44,7 @@ public:
     * the hierarchy should not have \c visit methods.
     */
    /*@{*/
+   virtual void visit(class ir_rvalue *) { assert(!"unhandled error_type"); }
    virtual void visit(class ir_variable *) = 0;
    virtual void visit(class ir_function_signature *) = 0;
    virtual void visit(class ir_function *) = 0;




More information about the mesa-commit mailing list