Re: [Mesa-dev] [PATCH v2] glsl: handle a switch where default is in the middle of cases

[Anuj Phogat]

On Thu, Jun 19, 2014 at 6:11 AM, Tapani Pälli tapani.pa...@intel.com wrote:
 This fixes following tests in es3conform:

shaders.switch.default_not_last_dynamic_vertex
shaders.switch.default_not_last_dynamic_fragment

 and makes following tests in Piglit pass:

glsl-1.30/execution/switch/fs-default-notlast-fallthrough
glsl-1.30/execution/switch/fs-default_notlast

 No Piglit regressions.

 v2: take away unnecessary ir_if, just use conditional assignment

Tried verifying es3conform fixes. Patch doesn't compile anymore with
mesa master.

 Signed-off-by: Tapani Pälli tapani.pa...@intel.com
 ---
  src/glsl/ast_to_hir.cpp   | 84 
 +--
  src/glsl/glsl_parser_extras.h |  3 ++
  2 files changed, 84 insertions(+), 3 deletions(-)

 diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
 index 132a955..2a105c6 100644
 --- a/src/glsl/ast_to_hir.cpp
 +++ b/src/glsl/ast_to_hir.cpp
 @@ -4503,6 +4503,12 @@ ast_switch_statement::hir(exec_list *instructions,
 instructions-push_tail(new(ctx) ir_assignment(deref_is_break_var,
is_break_val));

 +   state-switch_state.run_default =
 +  new(ctx) ir_variable(glsl_type::bool_type,
 + run_default_tmp,
 + ir_var_temporary);
 +   instructions-push_tail(state-switch_state.run_default);
 +
 /* Cache test expression.
  */
 test_to_hir(instructions, state);
 @@ -4557,8 +4563,72 @@ ir_rvalue *
  ast_case_statement_list::hir(exec_list *instructions,
   struct _mesa_glsl_parse_state *state)
  {
 -   foreach_list_typed (ast_case_statement, case_stmt, link,  this-cases)
 -  case_stmt-hir(instructions, state);
 +   exec_list default_case, after_default, tmp;
 +
 +   foreach_list_typed (ast_case_statement, case_stmt, link,  this-cases) {
 +  case_stmt-hir(tmp, state);
 +
 +  /* Default case. */
 +  if (state-switch_state.previous_default  default_case.is_empty()) {
 + default_case.append_list(tmp);
 + continue;
 +  }
 +
 +  /* If default case found, append 'after_default' list. */
 +  if (!default_case.is_empty())
 + after_default.append_list(tmp);
 +  else
 + instructions-append_list(tmp);
 +   }
 +
 +   /* Handle the default case. This is done here because default might not be
 +* the last case. We need to add checks against following cases first to 
 see
 +* if default should be chosen or not.
 +*/
 +   if (!default_case.is_empty()) {
 +
 +  /* Default case was the last one, no checks required. */
 +  if (after_default.is_empty()) {
 + instructions-append_list(default_case);
 + return NULL;
 +  }
 +
 +  ir_rvalue *const true_val = new (state) ir_constant(true);
 +  ir_dereference_variable *deref_run_default_var =
 + new(state) ir_dereference_variable(state-switch_state.run_default);
 +
 +  /* Choose to run default case initially, following conditional
 +   * assignments might change this.
 +   */
 +  ir_assignment *const init_var =
 + new(state) ir_assignment(deref_run_default_var, true_val);
 +  instructions-push_tail(init_var);
 +
 +  foreach_list(n, after_default) {
 + ir_instruction *ir = (ir_instruction *) n;
 + ir_assignment *assign = ir-as_assignment();
 +
 + if (!assign)
 +continue;
 +
 + /* Clone the check between case label and init expression. */
 + ir_expression *exp = (ir_expression*) assign-condition;
 + ir_expression *clone = exp-clone(state, NULL);
 +
 + ir_dereference_variable *deref_var =
 +new(state) 
 ir_dereference_variable(state-switch_state.run_default);
 + ir_rvalue *const false_val = new (state) ir_constant(false);
 +
 + ir_assignment *const set_false =
 +new(state) ir_assignment(deref_var, false_val, clone);
 +
 + instructions-push_tail(set_false);
 +  }
 +
 +  /* Append default case and all cases after it. */
 +  instructions-append_list(default_case);
 +  instructions-append_list(after_default);
 +   }

 /* Case statements do not have r-values. */
 return NULL;
 @@ -4718,9 +4788,17 @@ ast_case_label::hir(exec_list *instructions,
}
state-switch_state.previous_default = this;

 +  /* Set fallthru condition on 'run_default' bool. */
 +  ir_dereference_variable *deref_run_default =
 + new(ctx) ir_dereference_variable(state-switch_state.run_default);
 +  ir_rvalue *const cond_true = new(ctx) ir_constant(true);
 +  ir_expression *test_cond = new(ctx) ir_expression(ir_binop_all_equal,
 +cond_true,
 +deref_run_default);
 +
/* Set falltrhu state. */
ir_assignment *set_fallthru =
 - new(ctx) 

[Mesa-dev] [PATCH v2] glsl: handle a switch where default is in the middle of cases

[Tapani Pälli]

This fixes following tests in es3conform:

   shaders.switch.default_not_last_dynamic_vertex
   shaders.switch.default_not_last_dynamic_fragment

and makes following tests in Piglit pass:

   glsl-1.30/execution/switch/fs-default-notlast-fallthrough
   glsl-1.30/execution/switch/fs-default_notlast

No Piglit regressions.

v2: take away unnecessary ir_if, just use conditional assignment

Signed-off-by: Tapani Pälli tapani.pa...@intel.com
---
 src/glsl/ast_to_hir.cpp   | 84 +--
 src/glsl/glsl_parser_extras.h |  3 ++
 2 files changed, 84 insertions(+), 3 deletions(-)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 132a955..2a105c6 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -4503,6 +4503,12 @@ ast_switch_statement::hir(exec_list *instructions,
instructions-push_tail(new(ctx) ir_assignment(deref_is_break_var,
   is_break_val));
 
+   state-switch_state.run_default =
+  new(ctx) ir_variable(glsl_type::bool_type,
+ run_default_tmp,
+ ir_var_temporary);
+   instructions-push_tail(state-switch_state.run_default);
+
/* Cache test expression.
 */
test_to_hir(instructions, state);
@@ -4557,8 +4563,72 @@ ir_rvalue *
 ast_case_statement_list::hir(exec_list *instructions,
  struct _mesa_glsl_parse_state *state)
 {
-   foreach_list_typed (ast_case_statement, case_stmt, link,  this-cases)
-  case_stmt-hir(instructions, state);
+   exec_list default_case, after_default, tmp;
+
+   foreach_list_typed (ast_case_statement, case_stmt, link,  this-cases) {
+  case_stmt-hir(tmp, state);
+
+  /* Default case. */
+  if (state-switch_state.previous_default  default_case.is_empty()) {
+ default_case.append_list(tmp);
+ continue;
+  }
+
+  /* If default case found, append 'after_default' list. */
+  if (!default_case.is_empty())
+ after_default.append_list(tmp);
+  else
+ instructions-append_list(tmp);
+   }
+
+   /* Handle the default case. This is done here because default might not be
+* the last case. We need to add checks against following cases first to see
+* if default should be chosen or not.
+*/
+   if (!default_case.is_empty()) {
+
+  /* Default case was the last one, no checks required. */
+  if (after_default.is_empty()) {
+ instructions-append_list(default_case);
+ return NULL;
+  }
+
+  ir_rvalue *const true_val = new (state) ir_constant(true);
+  ir_dereference_variable *deref_run_default_var =
+ new(state) ir_dereference_variable(state-switch_state.run_default);
+
+  /* Choose to run default case initially, following conditional
+   * assignments might change this.
+   */
+  ir_assignment *const init_var =
+ new(state) ir_assignment(deref_run_default_var, true_val);
+  instructions-push_tail(init_var);
+
+  foreach_list(n, after_default) {
+ ir_instruction *ir = (ir_instruction *) n;
+ ir_assignment *assign = ir-as_assignment();
+
+ if (!assign)
+continue;
+
+ /* Clone the check between case label and init expression. */
+ ir_expression *exp = (ir_expression*) assign-condition;
+ ir_expression *clone = exp-clone(state, NULL);
+
+ ir_dereference_variable *deref_var =
+new(state) 
ir_dereference_variable(state-switch_state.run_default);
+ ir_rvalue *const false_val = new (state) ir_constant(false);
+
+ ir_assignment *const set_false =
+new(state) ir_assignment(deref_var, false_val, clone);
+
+ instructions-push_tail(set_false);
+  }
+
+  /* Append default case and all cases after it. */
+  instructions-append_list(default_case);
+  instructions-append_list(after_default);
+   }
 
/* Case statements do not have r-values. */
return NULL;
@@ -4718,9 +4788,17 @@ ast_case_label::hir(exec_list *instructions,
   }
   state-switch_state.previous_default = this;
 
+  /* Set fallthru condition on 'run_default' bool. */
+  ir_dereference_variable *deref_run_default =
+ new(ctx) ir_dereference_variable(state-switch_state.run_default);
+  ir_rvalue *const cond_true = new(ctx) ir_constant(true);
+  ir_expression *test_cond = new(ctx) ir_expression(ir_binop_all_equal,
+cond_true,
+deref_run_default);
+
   /* Set falltrhu state. */
   ir_assignment *set_fallthru =
- new(ctx) ir_assignment(deref_fallthru_var, true_val);
+ new(ctx) ir_assignment(deref_fallthru_var, true_val, test_cond);
 
   instructions-push_tail(set_fallthru);
}
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index a59090f..de23d33 100644
--- 

Re: [Mesa-dev] [PATCH v2] glsl: handle a switch where default is in the middle of cases

[Roland Scheidegger]

Am 19.06.2014 15:11, schrieb Tapani Pälli:
 This fixes following tests in es3conform:
 
shaders.switch.default_not_last_dynamic_vertex
shaders.switch.default_not_last_dynamic_fragment
 
 and makes following tests in Piglit pass:
 
glsl-1.30/execution/switch/fs-default-notlast-fallthrough
glsl-1.30/execution/switch/fs-default_notlast
 
 No Piglit regressions.
 
 v2: take away unnecessary ir_if, just use conditional assignment
 
 Signed-off-by: Tapani Pälli tapani.pa...@intel.com
 ---
  src/glsl/ast_to_hir.cpp   | 84 
 +--
  src/glsl/glsl_parser_extras.h |  3 ++
  2 files changed, 84 insertions(+), 3 deletions(-)
 
 diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
 index 132a955..2a105c6 100644
 --- a/src/glsl/ast_to_hir.cpp
 +++ b/src/glsl/ast_to_hir.cpp
 @@ -4503,6 +4503,12 @@ ast_switch_statement::hir(exec_list *instructions,
 instructions-push_tail(new(ctx) ir_assignment(deref_is_break_var,
is_break_val));
  
 +   state-switch_state.run_default =
 +  new(ctx) ir_variable(glsl_type::bool_type,
 + run_default_tmp,
 + ir_var_temporary);
 +   instructions-push_tail(state-switch_state.run_default);
 +
 /* Cache test expression.
  */
 test_to_hir(instructions, state);
 @@ -4557,8 +4563,72 @@ ir_rvalue *
  ast_case_statement_list::hir(exec_list *instructions,
   struct _mesa_glsl_parse_state *state)
  {
 -   foreach_list_typed (ast_case_statement, case_stmt, link,  this-cases)
 -  case_stmt-hir(instructions, state);
 +   exec_list default_case, after_default, tmp;
 +
 +   foreach_list_typed (ast_case_statement, case_stmt, link,  this-cases) {
 +  case_stmt-hir(tmp, state);
 +
 +  /* Default case. */
 +  if (state-switch_state.previous_default  default_case.is_empty()) {
 + default_case.append_list(tmp);
 + continue;
 +  }
 +
 +  /* If default case found, append 'after_default' list. */
 +  if (!default_case.is_empty())
 + after_default.append_list(tmp);
 +  else
 + instructions-append_list(tmp);
 +   }
 +
 +   /* Handle the default case. This is done here because default might not be
 +* the last case. We need to add checks against following cases first to 
 see
 +* if default should be chosen or not.
 +*/
 +   if (!default_case.is_empty()) {
 +
 +  /* Default case was the last one, no checks required. */
 +  if (after_default.is_empty()) {
 + instructions-append_list(default_case);
 + return NULL;
 +  }
 +
 +  ir_rvalue *const true_val = new (state) ir_constant(true);
 +  ir_dereference_variable *deref_run_default_var =
 + new(state) ir_dereference_variable(state-switch_state.run_default);
 +
 +  /* Choose to run default case initially, following conditional
 +   * assignments might change this.
 +   */
 +  ir_assignment *const init_var =
 + new(state) ir_assignment(deref_run_default_var, true_val);
 +  instructions-push_tail(init_var);
 +
 +  foreach_list(n, after_default) {
 + ir_instruction *ir = (ir_instruction *) n;
 + ir_assignment *assign = ir-as_assignment();
 +
 + if (!assign)
 +continue;
 +
 + /* Clone the check between case label and init expression. */
 + ir_expression *exp = (ir_expression*) assign-condition;
 + ir_expression *clone = exp-clone(state, NULL);
 +
 + ir_dereference_variable *deref_var =
 +new(state) 
 ir_dereference_variable(state-switch_state.run_default);
 + ir_rvalue *const false_val = new (state) ir_constant(false);
 +
 + ir_assignment *const set_false =
 +new(state) ir_assignment(deref_var, false_val, clone);
 +
 + instructions-push_tail(set_false);
 +  }
 +
 +  /* Append default case and all cases after it. */
 +  instructions-append_list(default_case);
 +  instructions-append_list(after_default);
 +   }
  
 /* Case statements do not have r-values. */
 return NULL;
 @@ -4718,9 +4788,17 @@ ast_case_label::hir(exec_list *instructions,
}
state-switch_state.previous_default = this;
  
 +  /* Set fallthru condition on 'run_default' bool. */
 +  ir_dereference_variable *deref_run_default =
 + new(ctx) ir_dereference_variable(state-switch_state.run_default);
 +  ir_rvalue *const cond_true = new(ctx) ir_constant(true);
 +  ir_expression *test_cond = new(ctx) ir_expression(ir_binop_all_equal,
 +cond_true,
 +deref_run_default);
 +
/* Set falltrhu state. */
ir_assignment *set_fallthru =
 - new(ctx) ir_assignment(deref_fallthru_var, true_val);
 + new(ctx) ir_assignment(deref_fallthru_var,