[Mesa-dev] [PATCH 18/78] i965: Take is_scalar_shader_stage() method out to allow reuse

[Jason Ekstrand]

On Fri, Jun 26, 2015 at 1:06 AM, Eduardo Lima Mitev <elima at igalia.com> wrote:
> This patch makes public the is_scalar_shader_stage() method in brw_shader, and
> renames it to brw_compiler_is_scalar_shader_stage(). The plan is to later reuse it
> in brw_nir, to enable/disable optimization passes depending on the type
> of shader stage.

I'm not so sure that this is a good plan.  It assumes that whether we
are doing a scalar or vec4 compile is based entirely on the shader
stage and some static information (possibly based on environment
variables).  Ken and I were talking around the office and we may want
to use both SIMD4x2 and SIMD8 mode for geometry shaders depending on
the number of inputs, etc.  This won't work in the given paradigm.

> The new method accepts a brw_compiler instead of a brw_context. This is done
> for consistency, since the actual info we need (scalar_vs) is in brw_compiler,
> and fetching in through brw_content->intelScreen->compiler seems like too
> much indirection.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89580
> ---
>  src/mesa/drivers/dri/i965/brw_shader.cpp | 22 ++++++----------------
>  src/mesa/drivers/dri/i965/brw_shader.h   | 13 +++++++++++++
>  2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 0b53647..3b99046 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -182,19 +182,6 @@ brw_shader_precompile(struct gl_context *ctx,
>     return true;
>  }
>
> -static inline bool
> -is_scalar_shader_stage(struct brw_context *brw, int stage)
> -{
> -   switch (stage) {
> -   case MESA_SHADER_FRAGMENT:
> -      return true;
> -   case MESA_SHADER_VERTEX:
> -      return brw->intelScreen->compiler->scalar_vs;
> -   default:
> -      return false;
> -   }
> -}
> -
>  static void
>  brw_lower_packing_builtins(struct brw_context *brw,
>                             gl_shader_stage shader_type,
> @@ -205,7 +192,8 @@ brw_lower_packing_builtins(struct brw_context *brw,
>             | LOWER_PACK_UNORM_2x16
>             | LOWER_UNPACK_UNORM_2x16;
>
> -   if (is_scalar_shader_stage(brw, shader_type)) {
> +   if (brw_compiler_is_scalar_shader_stage(brw->intelScreen->compiler,
> +                                           shader_type)) {
>        ops |= LOWER_UNPACK_UNORM_4x8
>             | LOWER_UNPACK_SNORM_4x8
>             | LOWER_PACK_UNORM_4x8
> @@ -218,7 +206,8 @@ brw_lower_packing_builtins(struct brw_context *brw,
>         * lowering is needed. For SOA code, the Half2x16 ops must be
>         * scalarized.
>         */
> -      if (is_scalar_shader_stage(brw, shader_type)) {
> +      if (brw_compiler_is_scalar_shader_stage(brw->intelScreen->compiler,
> +                                              shader_type)) {
>           ops |= LOWER_PACK_HALF_2x16_TO_SPLIT
>               |  LOWER_UNPACK_HALF_2x16_TO_SPLIT;
>        }
> @@ -294,7 +283,8 @@ process_glsl_ir(struct brw_context *brw,
>     do {
>        progress = false;
>
> -      if (is_scalar_shader_stage(brw, shader->Stage)) {
> +      if (brw_compiler_is_scalar_shader_stage(brw->intelScreen->compiler,
> +                                              shader->Stage)) {
>           brw_do_channel_expressions(shader->ir);
>           brw_do_vector_splitting(shader->ir);
>        }
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.h b/src/mesa/drivers/dri/i965/brw_shader.h
> index b2c1a0b..cef2226 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.h
> +++ b/src/mesa/drivers/dri/i965/brw_shader.h
> @@ -302,6 +302,19 @@ bool brw_cs_precompile(struct gl_context *ctx,
>                         struct gl_shader_program *shader_prog,
>                         struct gl_program *prog);
>
> +static inline bool
> +brw_compiler_is_scalar_shader_stage(struct brw_compiler *compiler, int stage)
> +{
> +   switch (stage) {
> +   case MESA_SHADER_FRAGMENT:

You need MESA_SHADER_COMPUTE here too.

> +      return true;
> +   case MESA_SHADER_VERTEX:
> +      return compiler->scalar_vs;
> +   default:
> +      return false;
> +   }
> +}
> +
>  #ifdef __cplusplus
>  }
>  #endif
> --
> 2.1.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev