[Mesa-dev] [PATCH 3/5] i965: Move TCS output indirect_offset.file check out a level.

[Pohjolainen, Topi]

On Thu, Apr 21, 2016 at 10:32:07PM -0700, Kenneth Graunke wrote:
> I want to add another condition.  Moving the indirect_offset.file
> check out a level should make this a little easier.
> 
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp | 88 ++++++++++++++++--------------
>  1 file changed, 46 insertions(+), 42 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp b/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp
> index 8e2713a..8a4ae2e 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_tcs.cpp
> @@ -393,52 +393,56 @@ vec4_tcs_visitor::nir_emit_intrinsic(nir_intrinsic_instr *instr)
>        src_reg indirect_offset = get_indirect_offset(instr);
>        unsigned imm_offset = instr->const_index[0];
>  
> -      if (imm_offset == 0 && indirect_offset.file == BAD_FILE) {
> -         value.type = BRW_REGISTER_TYPE_F;

I checked this mechanically without understanding the context too well. As
such it looks good. I was just wondering if it would have been cleaner to
keep the indentation and just:

         if (indirect_offset.file == BAD_FILE) {
            emit_urb_write(swizzle(value, swiz), mask,
                           imm_offset, indirect_offset);
            break;
         }

         if (imm_offset == 0) {

> +      if (indirect_offset.file == BAD_FILE) {
> +         if (imm_offset == 0) {
> +            value.type = BRW_REGISTER_TYPE_F;
>  
> -         mask &= (1 << tesslevel_inner_components(key->tes_primitive_mode)) - 1;
> +            mask &=
> +               (1 << tesslevel_inner_components(key->tes_primitive_mode)) - 1;
>  
> -         /* This is a write to gl_TessLevelInner[], which lives in the
> -          * Patch URB header.  The layout depends on the domain.
> -          */
> -         switch (key->tes_primitive_mode) {
> -         case GL_QUADS:
> -            /* gl_TessLevelInner[].xy lives at DWords 3-2 (reversed).
> -             * We use an XXYX swizzle to reverse put .xy in the .wz
> -             * channels, and use a .zw writemask.
> +            /* This is a write to gl_TessLevelInner[], which lives in the
> +             * Patch URB header.  The layout depends on the domain.
>               */
> -            swiz = BRW_SWIZZLE4(0, 0, 1, 0);
> -            mask = writemask_for_backwards_vector(mask);
> -            break;
> -         case GL_TRIANGLES:
> -            /* gl_TessLevelInner[].x lives at DWord 4, so we set the
> -             * writemask to X and bump the URB offset by 1.
> +            switch (key->tes_primitive_mode) {
> +            case GL_QUADS:
> +               /* gl_TessLevelInner[].xy lives at DWords 3-2 (reversed).
> +                * We use an XXYX swizzle to reverse put .xy in the .wz
> +                * channels, and use a .zw writemask.
> +                */
> +               swiz = BRW_SWIZZLE4(0, 0, 1, 0);
> +               mask = writemask_for_backwards_vector(mask);
> +               break;
> +            case GL_TRIANGLES:
> +               /* gl_TessLevelInner[].x lives at DWord 4, so we set the
> +                * writemask to X and bump the URB offset by 1.
> +                */
> +               imm_offset = 1;
> +               break;
> +            case GL_ISOLINES:
> +               /* Skip; gl_TessLevelInner[] doesn't exist for isolines. */
> +               return;
> +            default:
> +               unreachable("Bogus tessellation domain");
> +            }
> +         } else if (imm_offset == 1) {
> +            value.type = BRW_REGISTER_TYPE_F;
> +
> +            mask &=
> +               (1 << tesslevel_outer_components(key->tes_primitive_mode)) - 1;
> +
> +            /* This is a write to gl_TessLevelOuter[] which lives in the
> +             * Patch URB Header at DWords 4-7.  However, it's reversed, so
> +             * instead of .xyzw we have .wzyx.
>               */
> -            imm_offset = 1;
> -            break;
> -         case GL_ISOLINES:
> -            /* Skip; gl_TessLevelInner[] doesn't exist for isolines. */
> -            return;
> -         default:
> -            unreachable("Bogus tessellation domain");
> -         }
> -      } else if (imm_offset == 1 && indirect_offset.file == BAD_FILE) {
> -         value.type = BRW_REGISTER_TYPE_F;
> -
> -         mask &= (1 << tesslevel_outer_components(key->tes_primitive_mode)) - 1;
> -
> -         /* This is a write to gl_TessLevelOuter[] which lives in the
> -          * Patch URB Header at DWords 4-7.  However, it's reversed, so
> -          * instead of .xyzw we have .wzyx.
> -          */
> -         if (key->tes_primitive_mode == GL_ISOLINES) {
> -            /* Isolines .xy should be stored in .zw, in order. */
> -            swiz = BRW_SWIZZLE4(0, 0, 0, 1);
> -            mask <<= 2;
> -         } else {
> -            /* Other domains are reversed; store .wzyx instead of .xyzw. */
> -            swiz = BRW_SWIZZLE_WZYX;
> -            mask = writemask_for_backwards_vector(mask);
> +            if (key->tes_primitive_mode == GL_ISOLINES) {
> +               /* Isolines .xy should be stored in .zw, in order. */
> +               swiz = BRW_SWIZZLE4(0, 0, 0, 1);
> +               mask <<= 2;
> +            } else {
> +               /* Other domains are reversed; store .wzyx instead of .xyzw. */
> +               swiz = BRW_SWIZZLE_WZYX;
> +               mask = writemask_for_backwards_vector(mask);
> +            }
>           }
>        }
>  
> -- 
> 2.8.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev