On 06/15/2011 09:33 AM, Dan McCabe wrote:
Data structures for switch statement and case label are created that parallel
the structure of other AST data.
---
  src/glsl/ast.h |   27 +++++++++++++++++++++++----
  1 files changed, 23 insertions(+), 4 deletions(-)

Dan, thanks for sending this out!  A few comments below...

diff --git a/src/glsl/ast.h b/src/glsl/ast.h
index 878f48b..48d1795 100644
--- a/src/glsl/ast.h
+++ b/src/glsl/ast.h
@@ -628,13 +628,19 @@ public:

  class ast_case_label : public ast_node {
  public:
+   ast_case_label(ast_expression *test_value);
+   virtual void print(void) const;
+
+   virtual ir_rvalue *hir(exec_list *instructions,
+                         struct _mesa_glsl_parse_state *state);

     /**
-    * An expression of NULL means 'default'.
+    * An test value of NULL means 'default'.
      */
-   ast_expression *expression;
+   ast_expression *test_value;
  };

+
  class ast_selection_statement : public ast_node {
  public:
     ast_selection_statement(ast_expression *condition,
@@ -653,8 +659,21 @@ public:

  class ast_switch_statement : public ast_node {
  public:
-   ast_expression *expression;
-   exec_list statements;
+   ast_switch_statement(ast_expression *test_expression,
+                       ast_node *body);
+   virtual void print(void) const;
+
+   virtual ir_rvalue *hir(exec_list *instructions,
+                         struct _mesa_glsl_parse_state *state);
+
+   ast_expression *test_expression;
+   ast_node *body;
+
+   ir_variable *test_var;

Presumably this is "x" in "switch (x) { ... }"? That can be an arbitrary scalar integer expression, not just a variable.

+   ir_variable *fallthru_var;
+
+protected:
+   void test_to_hir(class ir_loop *, struct _mesa_glsl_parse_state *);
  };

I don't think we should be putting ir_variables (or any HIR code!) in the AST structures. The AST should really just be an abstract syntax tree, representing the text and structure of the program, but independent of any IR code we might generate.

I had imagined creating some kind of IR structure along the lines of:

class ir_switch : public ir_instruction
{
        ir_rvalue *switch_value; // switch (...) - arbitrary expression

        exec_list cases; // list of ir_case
};

class ir_case : public exec_node
{
        ir_rvalue *test_value; // case X: ... NULL means "default:"
        exec_list body; // statements inside this case
        // possibly a bool has_break or falls_through, if helpful
};

This seems pretty straightforward to generate at AST->HIR time.

Then there would be a lowering pass (see lower_*.cpp) based on an ir_hierarchical_visitor that visits ir_switch, replacing it with the loop/if structure. ir_variable *fallthru_var would fit nicely as a member of that visitor. visit(ir_switch *) would create/emit it, and visit(ir_case *) would reference it.

Of course, there may be another solution; it's just an idea.

--Kenneth
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to