Re: [Mesa-dev] [PATCH] spirv: Emit switch conditions on-the-fly

2019-01-12 Thread Lionel Landwerlin

On 12/01/2019 15:29, Jason Ekstrand wrote:
On January 12, 2019 03:06:07 Lionel Landwerlin 
 wrote:



On 12/01/2019 03:45, Jason Ekstrand wrote:

Instead of emitting all of the conditions for the cases of a switch
statement up-front, emit them on-the-fly as we emit the code for each
case.  The original justification for this was that we were going to
have to build a default case anyway which would need them all.  
However,

we can just trust CSE to clean up the mess in that case. Emitting each
condition right before the if statement that uses it reduces register
pressure and, in one customer benchmark, gets rid of spilling and
improves performance by about 2x.
---
src/compiler/spirv/vtn_cfg.c | 62 +++-
1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/src/compiler/spirv/vtn_cfg.c 
b/src/compiler/spirv/vtn_cfg.c

index aef1b7e18fb..34a910accc6 100644
--- a/src/compiler/spirv/vtn_cfg.c
+++ b/src/compiler/spirv/vtn_cfg.c
@@ -852,6 +852,30 @@ vtn_emit_branch(struct vtn_builder *b, enum 
vtn_branch_type branch_type,

}
}

+static nir_ssa_def *
+vtn_switch_case_condition(struct vtn_builder *b, struct vtn_switch 
*swtch,

+  nir_ssa_def *sel, struct vtn_case *cse)
+{
+   if (cse->is_default) {
+  nir_ssa_def *any = nir_imm_false(>nb);
+  list_for_each_entry(struct vtn_case, other, >cases, 
link) {

+ if (cse->is_default)
+    continue;
+
+ any = nir_ior(>nb, any,
+   vtn_switch_case_condition(b, swtch, sel, 
other));

+  }
+  return any;



return nir_inot(>nb, any); here?


Yup. As you can probably guess, I was lazy and didn't send it through 
Jenkins before sending.  I'll get that fixed.



With that fixed and CI happy:


Reviewed-by: Lionel Landwerlin 








+   } else {
+  nir_ssa_def *cond = nir_imm_false(>nb);
+  util_dynarray_foreach(>values, uint64_t, val) {
+ nir_ssa_def *imm = nir_imm_intN_t(>nb, *val, 
sel->bit_size);

+ cond = nir_ior(>nb, cond, nir_ieq(>nb, sel, imm));
+  }
+  return cond;
+   }
+}
+
static void
vtn_emit_cf_list(struct vtn_builder *b, struct list_head *cf_list,
  nir_variable *switch_fall_var, bool *has_switch_break,
@@ -978,46 +1002,13 @@ vtn_emit_cf_list(struct vtn_builder *b, 
struct list_head *cf_list,

 nir_local_variable_create(b->nb.impl, glsl_bool_type(), "fall");
  nir_store_var(>nb, fall_var, nir_imm_false(>nb), 1);

- /* Next, we gather up all of the conditions.  We have to 
do this
-  * up-front because we also need to build an "any" 
condition so

-  * that we can use !any for default.
-  */
- const int num_cases = list_length(_switch->cases);
- NIR_VLA(nir_ssa_def *, conditions, num_cases);
-
  nir_ssa_def *sel = vtn_ssa_value(b, vtn_switch->selector)->def;
- /* An accumulation of all conditions.  Used for the 
default */

- nir_ssa_def *any = NULL;
-
- int i = 0;
- list_for_each_entry(struct vtn_case, cse, 
_switch->cases, link) {

-    if (cse->is_default) {
-   conditions[i++] = NULL;
-   continue;
-    }
-
-    nir_ssa_def *cond = NULL;
-    util_dynarray_foreach(>values, uint64_t, val) {
-   nir_ssa_def *imm = nir_imm_intN_t(>nb, *val, 
sel->bit_size);

-   nir_ssa_def *is_val = nir_ieq(>nb, sel, imm);
-
-   cond = cond ? nir_ior(>nb, cond, is_val) : is_val;
-    }
-
-    any = any ? nir_ior(>nb, any, cond) : cond;
-    conditions[i++] = cond;
- }
- vtn_assert(i == num_cases);

  /* Now we can walk the list of cases and actually emit code */
- i = 0;
  list_for_each_entry(struct vtn_case, cse, _switch->cases, link) {
 /* Figure out the condition */
-    nir_ssa_def *cond = conditions[i++];
-    if (cse->is_default) {
-   vtn_assert(cond == NULL);
-   cond = nir_inot(>nb, any);
-    }
+    nir_ssa_def *cond =
+   vtn_switch_case_condition(b, vtn_switch, sel, cse);
 /* Take fallthrough into account */
 cond = nir_ior(>nb, cond, nir_load_var(>nb, fall_var));

@@ -1030,7 +1021,6 @@ vtn_emit_cf_list(struct vtn_builder *b, struct 
list_head *cf_list,


 nir_pop_if(>nb, case_if);
  }
- vtn_assert(i == num_cases);

  break;
}







___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] spirv: Emit switch conditions on-the-fly

2019-01-12 Thread Jason Ekstrand
On January 12, 2019 03:06:07 Lionel Landwerlin 
 wrote:



On 12/01/2019 03:45, Jason Ekstrand wrote:

Instead of emitting all of the conditions for the cases of a switch
statement up-front, emit them on-the-fly as we emit the code for each
case.  The original justification for this was that we were going to
have to build a default case anyway which would need them all.  However,
we can just trust CSE to clean up the mess in that case.  Emitting each
condition right before the if statement that uses it reduces register
pressure and, in one customer benchmark, gets rid of spilling and
improves performance by about 2x.
---
src/compiler/spirv/vtn_cfg.c | 62 +++-
1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
index aef1b7e18fb..34a910accc6 100644
--- a/src/compiler/spirv/vtn_cfg.c
+++ b/src/compiler/spirv/vtn_cfg.c
@@ -852,6 +852,30 @@ vtn_emit_branch(struct vtn_builder *b, enum 
vtn_branch_type branch_type,

}
}

+static nir_ssa_def *
+vtn_switch_case_condition(struct vtn_builder *b, struct vtn_switch *swtch,
+  nir_ssa_def *sel, struct vtn_case *cse)
+{
+   if (cse->is_default) {
+  nir_ssa_def *any = nir_imm_false(>nb);
+  list_for_each_entry(struct vtn_case, other, >cases, link) {
+ if (cse->is_default)
+continue;
+
+ any = nir_ior(>nb, any,
+   vtn_switch_case_condition(b, swtch, sel, other));
+  }
+  return any;



return nir_inot(>nb, any); here?


Yup. As you can probably guess, I was lazy and didn't send it through 
Jenkins before sending.  I'll get that fixed.






+   } else {
+  nir_ssa_def *cond = nir_imm_false(>nb);
+  util_dynarray_foreach(>values, uint64_t, val) {
+ nir_ssa_def *imm = nir_imm_intN_t(>nb, *val, sel->bit_size);
+ cond = nir_ior(>nb, cond, nir_ieq(>nb, sel, imm));
+  }
+  return cond;
+   }
+}
+
static void
vtn_emit_cf_list(struct vtn_builder *b, struct list_head *cf_list,
  nir_variable *switch_fall_var, bool *has_switch_break,
@@ -978,46 +1002,13 @@ vtn_emit_cf_list(struct vtn_builder *b, struct 
list_head *cf_list,

 nir_local_variable_create(b->nb.impl, glsl_bool_type(), "fall");
  nir_store_var(>nb, fall_var, nir_imm_false(>nb), 1);

- /* Next, we gather up all of the conditions.  We have to do this
-  * up-front because we also need to build an "any" condition so
-  * that we can use !any for default.
-  */
- const int num_cases = list_length(_switch->cases);
- NIR_VLA(nir_ssa_def *, conditions, num_cases);
-
  nir_ssa_def *sel = vtn_ssa_value(b, vtn_switch->selector)->def;
- /* An accumulation of all conditions.  Used for the default */
- nir_ssa_def *any = NULL;
-
- int i = 0;
- list_for_each_entry(struct vtn_case, cse, _switch->cases, link) {
-if (cse->is_default) {
-   conditions[i++] = NULL;
-   continue;
-}
-
-nir_ssa_def *cond = NULL;
-util_dynarray_foreach(>values, uint64_t, val) {
-   nir_ssa_def *imm = nir_imm_intN_t(>nb, *val, sel->bit_size);
-   nir_ssa_def *is_val = nir_ieq(>nb, sel, imm);
-
-   cond = cond ? nir_ior(>nb, cond, is_val) : is_val;
-}
-
-any = any ? nir_ior(>nb, any, cond) : cond;
-conditions[i++] = cond;
- }
- vtn_assert(i == num_cases);

  /* Now we can walk the list of cases and actually emit code */
- i = 0;
  list_for_each_entry(struct vtn_case, cse, _switch->cases, link) {
 /* Figure out the condition */
-nir_ssa_def *cond = conditions[i++];
-if (cse->is_default) {
-   vtn_assert(cond == NULL);
-   cond = nir_inot(>nb, any);
-}
+nir_ssa_def *cond =
+   vtn_switch_case_condition(b, vtn_switch, sel, cse);
 /* Take fallthrough into account */
 cond = nir_ior(>nb, cond, nir_load_var(>nb, fall_var));

@@ -1030,7 +1021,6 @@ vtn_emit_cf_list(struct vtn_builder *b, struct 
list_head *cf_list,


 nir_pop_if(>nb, case_if);
  }
- vtn_assert(i == num_cases);

  break;
}




___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] spirv: Emit switch conditions on-the-fly

2019-01-12 Thread Lionel Landwerlin

On 12/01/2019 03:45, Jason Ekstrand wrote:

Instead of emitting all of the conditions for the cases of a switch
statement up-front, emit them on-the-fly as we emit the code for each
case.  The original justification for this was that we were going to
have to build a default case anyway which would need them all.  However,
we can just trust CSE to clean up the mess in that case.  Emitting each
condition right before the if statement that uses it reduces register
pressure and, in one customer benchmark, gets rid of spilling and
improves performance by about 2x.
---
  src/compiler/spirv/vtn_cfg.c | 62 +++-
  1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
index aef1b7e18fb..34a910accc6 100644
--- a/src/compiler/spirv/vtn_cfg.c
+++ b/src/compiler/spirv/vtn_cfg.c
@@ -852,6 +852,30 @@ vtn_emit_branch(struct vtn_builder *b, enum 
vtn_branch_type branch_type,
 }
  }
  
+static nir_ssa_def *

+vtn_switch_case_condition(struct vtn_builder *b, struct vtn_switch *swtch,
+  nir_ssa_def *sel, struct vtn_case *cse)
+{
+   if (cse->is_default) {
+  nir_ssa_def *any = nir_imm_false(>nb);
+  list_for_each_entry(struct vtn_case, other, >cases, link) {
+ if (cse->is_default)
+continue;
+
+ any = nir_ior(>nb, any,
+   vtn_switch_case_condition(b, swtch, sel, other));
+  }
+  return any;



return nir_inot(>nb, any); here?



+   } else {
+  nir_ssa_def *cond = nir_imm_false(>nb);
+  util_dynarray_foreach(>values, uint64_t, val) {
+ nir_ssa_def *imm = nir_imm_intN_t(>nb, *val, sel->bit_size);
+ cond = nir_ior(>nb, cond, nir_ieq(>nb, sel, imm));
+  }
+  return cond;
+   }
+}
+
  static void
  vtn_emit_cf_list(struct vtn_builder *b, struct list_head *cf_list,
   nir_variable *switch_fall_var, bool *has_switch_break,
@@ -978,46 +1002,13 @@ vtn_emit_cf_list(struct vtn_builder *b, struct list_head 
*cf_list,
  nir_local_variable_create(b->nb.impl, glsl_bool_type(), "fall");
   nir_store_var(>nb, fall_var, nir_imm_false(>nb), 1);
  
- /* Next, we gather up all of the conditions.  We have to do this

-  * up-front because we also need to build an "any" condition so
-  * that we can use !any for default.
-  */
- const int num_cases = list_length(_switch->cases);
- NIR_VLA(nir_ssa_def *, conditions, num_cases);
-
   nir_ssa_def *sel = vtn_ssa_value(b, vtn_switch->selector)->def;
- /* An accumulation of all conditions.  Used for the default */
- nir_ssa_def *any = NULL;
-
- int i = 0;
- list_for_each_entry(struct vtn_case, cse, _switch->cases, link) {
-if (cse->is_default) {
-   conditions[i++] = NULL;
-   continue;
-}
-
-nir_ssa_def *cond = NULL;
-util_dynarray_foreach(>values, uint64_t, val) {
-   nir_ssa_def *imm = nir_imm_intN_t(>nb, *val, sel->bit_size);
-   nir_ssa_def *is_val = nir_ieq(>nb, sel, imm);
-
-   cond = cond ? nir_ior(>nb, cond, is_val) : is_val;
-}
-
-any = any ? nir_ior(>nb, any, cond) : cond;
-conditions[i++] = cond;
- }
- vtn_assert(i == num_cases);
  
   /* Now we can walk the list of cases and actually emit code */

- i = 0;
   list_for_each_entry(struct vtn_case, cse, _switch->cases, link) {
  /* Figure out the condition */
-nir_ssa_def *cond = conditions[i++];
-if (cse->is_default) {
-   vtn_assert(cond == NULL);
-   cond = nir_inot(>nb, any);
-}
+nir_ssa_def *cond =
+   vtn_switch_case_condition(b, vtn_switch, sel, cse);
  /* Take fallthrough into account */
  cond = nir_ior(>nb, cond, nir_load_var(>nb, fall_var));
  
@@ -1030,7 +1021,6 @@ vtn_emit_cf_list(struct vtn_builder *b, struct list_head *cf_list,
  
  nir_pop_if(>nb, case_if);

   }
- vtn_assert(i == num_cases);
  
   break;

}



___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] [PATCH] spirv: Emit switch conditions on-the-fly

2019-01-11 Thread Jason Ekstrand
Instead of emitting all of the conditions for the cases of a switch
statement up-front, emit them on-the-fly as we emit the code for each
case.  The original justification for this was that we were going to
have to build a default case anyway which would need them all.  However,
we can just trust CSE to clean up the mess in that case.  Emitting each
condition right before the if statement that uses it reduces register
pressure and, in one customer benchmark, gets rid of spilling and
improves performance by about 2x.
---
 src/compiler/spirv/vtn_cfg.c | 62 +++-
 1 file changed, 26 insertions(+), 36 deletions(-)

diff --git a/src/compiler/spirv/vtn_cfg.c b/src/compiler/spirv/vtn_cfg.c
index aef1b7e18fb..34a910accc6 100644
--- a/src/compiler/spirv/vtn_cfg.c
+++ b/src/compiler/spirv/vtn_cfg.c
@@ -852,6 +852,30 @@ vtn_emit_branch(struct vtn_builder *b, enum 
vtn_branch_type branch_type,
}
 }
 
+static nir_ssa_def *
+vtn_switch_case_condition(struct vtn_builder *b, struct vtn_switch *swtch,
+  nir_ssa_def *sel, struct vtn_case *cse)
+{
+   if (cse->is_default) {
+  nir_ssa_def *any = nir_imm_false(>nb);
+  list_for_each_entry(struct vtn_case, other, >cases, link) {
+ if (cse->is_default)
+continue;
+
+ any = nir_ior(>nb, any,
+   vtn_switch_case_condition(b, swtch, sel, other));
+  }
+  return any;
+   } else {
+  nir_ssa_def *cond = nir_imm_false(>nb);
+  util_dynarray_foreach(>values, uint64_t, val) {
+ nir_ssa_def *imm = nir_imm_intN_t(>nb, *val, sel->bit_size);
+ cond = nir_ior(>nb, cond, nir_ieq(>nb, sel, imm));
+  }
+  return cond;
+   }
+}
+
 static void
 vtn_emit_cf_list(struct vtn_builder *b, struct list_head *cf_list,
  nir_variable *switch_fall_var, bool *has_switch_break,
@@ -978,46 +1002,13 @@ vtn_emit_cf_list(struct vtn_builder *b, struct list_head 
*cf_list,
 nir_local_variable_create(b->nb.impl, glsl_bool_type(), "fall");
  nir_store_var(>nb, fall_var, nir_imm_false(>nb), 1);
 
- /* Next, we gather up all of the conditions.  We have to do this
-  * up-front because we also need to build an "any" condition so
-  * that we can use !any for default.
-  */
- const int num_cases = list_length(_switch->cases);
- NIR_VLA(nir_ssa_def *, conditions, num_cases);
-
  nir_ssa_def *sel = vtn_ssa_value(b, vtn_switch->selector)->def;
- /* An accumulation of all conditions.  Used for the default */
- nir_ssa_def *any = NULL;
-
- int i = 0;
- list_for_each_entry(struct vtn_case, cse, _switch->cases, link) {
-if (cse->is_default) {
-   conditions[i++] = NULL;
-   continue;
-}
-
-nir_ssa_def *cond = NULL;
-util_dynarray_foreach(>values, uint64_t, val) {
-   nir_ssa_def *imm = nir_imm_intN_t(>nb, *val, sel->bit_size);
-   nir_ssa_def *is_val = nir_ieq(>nb, sel, imm);
-
-   cond = cond ? nir_ior(>nb, cond, is_val) : is_val;
-}
-
-any = any ? nir_ior(>nb, any, cond) : cond;
-conditions[i++] = cond;
- }
- vtn_assert(i == num_cases);
 
  /* Now we can walk the list of cases and actually emit code */
- i = 0;
  list_for_each_entry(struct vtn_case, cse, _switch->cases, link) {
 /* Figure out the condition */
-nir_ssa_def *cond = conditions[i++];
-if (cse->is_default) {
-   vtn_assert(cond == NULL);
-   cond = nir_inot(>nb, any);
-}
+nir_ssa_def *cond =
+   vtn_switch_case_condition(b, vtn_switch, sel, cse);
 /* Take fallthrough into account */
 cond = nir_ior(>nb, cond, nir_load_var(>nb, fall_var));
 
@@ -1030,7 +1021,6 @@ vtn_emit_cf_list(struct vtn_builder *b, struct list_head 
*cf_list,
 
 nir_pop_if(>nb, case_if);
  }
- vtn_assert(i == num_cases);
 
  break;
   }
-- 
2.20.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev