[Mesa-dev] [PATCH 7/8] spirv: Restructure the case loop in OpSwitch handling

[Ian Romanick]

On 12/11/2017 02:50 PM, Jason Ekstrand wrote:
> On Mon, Dec 11, 2017 at 10:08 AM, Ian Romanick <idr at freedesktop.org
> <mailto:idr at freedesktop.org>> wrote:
> 
>     On 12/07/2017 08:12 AM, Jason Ekstrand wrote:
>     > Instead of calling vtn_add_case for the default case and then looping,
>     > add an is_default variable and do everything inside the loop. 
>     This will
>     > make the next commit easier.
>     > ---
>     >  src/compiler/spirv/vtn_cfg.c | 17 ++++++++++++++---
>     >  1 file changed, 14 insertions(+), 3 deletions(-)
>     >
>     > diff --git a/src/compiler/spirv/vtn_cfg.c
>     b/src/compiler/spirv/vtn_cfg.c
>     > index 25140ff..9d1ca84 100644
>     > --- a/src/compiler/spirv/vtn_cfg.c
>     > +++ b/src/compiler/spirv/vtn_cfg.c
>     > @@ -425,9 +425,20 @@ vtn_cfg_walk_blocks(struct vtn_builder *b,
>     struct list_head *cf_list,
>     >           const uint32_t *branch_end =
>     >              block->branch + (block->branch[0] >> SpvWordCountShift);
>     >
>     > -         vtn_add_case(b, swtch, break_block, block->branch[2], 0,
>     true);
>     > -         for (const uint32_t *w = block->branch + 3; w <
>     branch_end; w += 2)
>     > -            vtn_add_case(b, swtch, break_block, w[1], w[0], false);
>     > +         bool is_default = true;
>     > +         for (const uint32_t *w = block->branch + 2; w <
>     branch_end;) {
>     > +            uint32_t literal = 0;
>     > +            if (!is_default) {
>     > +               literal = *w;
>     > +               w++;
>     > +            }
>     > +
>     > +            uint32_t block_id = *w;
>     > +            w++;
> 
>     In other parts of Mesa, this would be
> 
>                 const uint32_t block_id = *(w++);
> 
>     Is that not the preferred style here too?  Having looked ahead at the
>     next patch, I can see why the other dereference of w is not like this.
> 
> 
> Yeah, that's cleaner.  I've switched to that and rebased patch 8 on it.

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>     > +
>     > +            vtn_add_case(b, swtch, break_block, block_id,
>     literal, is_default);
>     > +            is_default = false;
>     > +         }
>     >
>     >           /* Now, we go through and walk the blocks.  While we
>     walk through
>     >            * the blocks, we also gather the much-needed fall-through
>     >