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

[Jason Ekstrand]

On January 12, 2019 03:06:07 Lionel Landwerlin 
<lionel.g.landwerlin at intel.com> 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(&b->nb);
>> +      list_for_each_entry(struct vtn_case, other, &swtch->cases, link) {
>> +         if (cse->is_default)
>> +            continue;
>> +
>> +         any = nir_ior(&b->nb, any,
>> +                       vtn_switch_case_condition(b, swtch, sel, other));
>> +      }
>> +      return any;
>
>
> return nir_inot(&b->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(&b->nb);
>> +      util_dynarray_foreach(&cse->values, uint64_t, val) {
>> +         nir_ssa_def *imm = nir_imm_intN_t(&b->nb, *val, sel->bit_size);
>> +         cond = nir_ior(&b->nb, cond, nir_ieq(&b->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(&b->nb, fall_var, nir_imm_false(&b->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(&vtn_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, &vtn_switch->cases, link) {
>> -            if (cse->is_default) {
>> -               conditions[i++] = NULL;
>> -               continue;
>> -            }
>> -
>> -            nir_ssa_def *cond = NULL;
>> -            util_dynarray_foreach(&cse->values, uint64_t, val) {
>> -               nir_ssa_def *imm = nir_imm_intN_t(&b->nb, *val, sel->bit_size);
>> -               nir_ssa_def *is_val = nir_ieq(&b->nb, sel, imm);
>> -
>> -               cond = cond ? nir_ior(&b->nb, cond, is_val) : is_val;
>> -            }
>> -
>> -            any = any ? nir_ior(&b->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, &vtn_switch->cases, link) {
>>      /* Figure out the condition */
>> -            nir_ssa_def *cond = conditions[i++];
>> -            if (cse->is_default) {
>> -               vtn_assert(cond == NULL);
>> -               cond = nir_inot(&b->nb, any);
>> -            }
>> +            nir_ssa_def *cond =
>> +               vtn_switch_case_condition(b, vtn_switch, sel, cse);
>>      /* Take fallthrough into account */
>>      cond = nir_ior(&b->nb, cond, nir_load_var(&b->nb, fall_var));
>>
>> @@ -1030,7 +1021,6 @@ vtn_emit_cf_list(struct vtn_builder *b, struct 
>> list_head *cf_list,
>>
>>      nir_pop_if(&b->nb, case_if);
>>   }
>> -         vtn_assert(i == num_cases);
>>
>>   break;
>> }