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

[Lionel Landwerlin]

On 12/01/2019 15:29, Jason Ekstrand wrote:
> 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.


With that fixed and CI happy:


Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>


>
>>
>>
>>> +   } 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;
>>> }
>
>
>
>