[Mesa-dev] [PATCH] i965: remove type_size_vec4_times_4()

[Kenneth Graunke]

On Tuesday, June 14, 2016 4:53:22 PM PDT Timothy Arceri wrote:
> type_size_vec4_times_4() was introduced as a fix in 8dcf807cb43383
> however since 3810c1561 we can just use type_size_scalar() and
> get the actual number of outputs we need.
> 
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  Hi Ken,
> 
>  I'm looking into the other suggestions you made on IRC so this may all just
>  go away but seems like a good idea to clean this up in the meantime.
> 
>  src/mesa/drivers/dri/i965/brw_fs.cpp     | 13 -------------
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp |  2 +-
>  src/mesa/drivers/dri/i965/brw_nir.c      |  4 ++--
>  src/mesa/drivers/dri/i965/brw_shader.h   |  1 -
>  4 files changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 0347b0a..8774f25 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -505,19 +505,6 @@ type_size_scalar(const struct glsl_type *type)
>     return 0;
>  }
>  
> -/**
> - * Returns the number of scalar components needed to store type, assuming
> - * that vectors are padded out to vec4.
> - *
> - * This has the packing rules of type_size_vec4(), but counts components
> - * similar to type_size_scalar().
> - */
> -extern "C" int
> -type_size_vec4_times_4(const struct glsl_type *type)
> -{
> -   return 4 * type_size_vec4(type);
> -}
> -
>  /* Attribute arrays are loaded as one vec4 per element (or matrix column),
>   * except for double-precision types, which are loaded as one dvec4.
>   */
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> index a956f9d..b811953 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
> @@ -108,7 +108,7 @@ fs_visitor::nir_setup_single_output_varying(fs_reg *reg,
>        for (unsigned count = 0; count < num_elements; count += 4) {
>           this->outputs[*location] = *reg;
>           this->output_components[*location] = MIN2(4, num_elements - count);
> -         *reg = offset(*reg, bld, 4);
> +         *reg = offset(*reg, bld, this->output_components[*location]);
>           (*location)++;
>        }
>     }
> diff --git a/src/mesa/drivers/dri/i965/brw_nir.c b/src/mesa/drivers/dri/i965/brw_nir.c
> index e01f160..d8cf12d 100644
> --- a/src/mesa/drivers/dri/i965/brw_nir.c
> +++ b/src/mesa/drivers/dri/i965/brw_nir.c
> @@ -292,8 +292,8 @@ brw_nir_lower_vue_outputs(nir_shader *nir,
>  {
>     if (is_scalar) {
>        nir_assign_var_locations(&nir->outputs, &nir->num_outputs,
> -                               type_size_vec4_times_4);
> -      nir_lower_io(nir, nir_var_shader_out, type_size_vec4_times_4);
> +                               type_size_scalar);
> +      nir_lower_io(nir, nir_var_shader_out, type_size_scalar);
>     } else {
>        nir_foreach_variable(var, &nir->outputs)
>           var->data.driver_location = var->data.location;
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h
> index 656dc89..9300f20 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.h
> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
> @@ -294,7 +294,6 @@ struct gl_shader *brw_new_shader(struct gl_context *ctx, GLuint name, GLuint typ
>  int type_size_scalar(const struct glsl_type *type);
>  int type_size_vec4(const struct glsl_type *type);
>  int type_size_dvec4(const struct glsl_type *type);
> -int type_size_vec4_times_4(const struct glsl_type *type);
>  int type_size_vs_input(const struct glsl_type *type);
>  
>  unsigned tesslevel_outer_components(GLenum tes_primitive_mode);
> 

I was skeptical, but this looks correct.  This only applies to shadowed
outputs, and just controls the packing within the fs_reg we allocate for
those outputs.  The URB layout remains the same.

It appears that we only needed this prior to the commit you referenced
because the old code was buggy.  Now that it's fixed, it doesn't matter.

I think this is fine, then.  Presumably you've run it through Jenkins
and everything was happy?

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160614/2cdf3cfa/attachment.sig>