[Mesa-dev] [PATCH 1/2] i965: Convert scalar_* flags to a scalar_stage array.

Kenneth Graunke kenneth at whitecape.org
Mon Nov 16 16:06:37 PST 2015


On Monday, November 16, 2015 10:23:22 AM Pohjolainen, Topi wrote:
> On Fri, Nov 13, 2015 at 11:29:00AM -0800, Kenneth Graunke wrote:
> > On Friday, November 13, 2015 10:06:23 AM Pohjolainen, Topi wrote:
> > > On Thu, Nov 12, 2015 at 03:38:51PM -0800, Kenneth Graunke wrote:
> > > > I was going to add scalar_tcs and scalar_tes flags, and then thought
> > > > better of it and decided to convert this to an array.  Simpler.
> > > > 
> > > > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > > > ---
> > > >  src/mesa/drivers/dri/i965/brw_compiler.h          |  3 +--
> > > >  src/mesa/drivers/dri/i965/brw_context.c           |  2 +-
> > > >  src/mesa/drivers/dri/i965/brw_gs.c                |  3 ++-
> > > >  src/mesa/drivers/dri/i965/brw_link.cpp            | 11 +++++---
> > > >  src/mesa/drivers/dri/i965/brw_program.c           |  3 ++-
> > > >  src/mesa/drivers/dri/i965/brw_shader.cpp          | 31 ++++++-----------------
> > > >  src/mesa/drivers/dri/i965/brw_shader.h            |  2 --
> > > >  src/mesa/drivers/dri/i965/brw_vec4.cpp            |  4 +--
> > > >  src/mesa/drivers/dri/i965/brw_vec4_gs_visitor.cpp |  2 +-
> > > >  src/mesa/drivers/dri/i965/brw_vs.c                |  7 ++---
> > > >  10 files changed, 28 insertions(+), 40 deletions(-)
> > > > 
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_compiler.h b/src/mesa/drivers/dri/i965/brw_compiler.h
> > > > index e3a26d6..3f54616 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_compiler.h
> > > > +++ b/src/mesa/drivers/dri/i965/brw_compiler.h
> > > > @@ -89,8 +89,7 @@ struct brw_compiler {
> > > >     void (*shader_debug_log)(void *, const char *str, ...) PRINTFLIKE(2, 3);
> > > >     void (*shader_perf_log)(void *, const char *str, ...) PRINTFLIKE(2, 3);
> > > >  
> > > > -   bool scalar_vs;
> > > > -   bool scalar_gs;
> > > > +   bool scalar_stage[MESA_SHADER_STAGES];
> > > >     struct gl_shader_compiler_options glsl_compiler_options[MESA_SHADER_STAGES];
> > > >  };
> > > >  
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> > > > index ac6045d..2db99c7 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_context.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_context.c
> > > > @@ -525,7 +525,7 @@ brw_initialize_context_constants(struct brw_context *brw)
> > > >        ctx->Const.Program[MESA_SHADER_FRAGMENT].MaxImageUniforms =
> > > >           BRW_MAX_IMAGES;
> > > >        ctx->Const.Program[MESA_SHADER_VERTEX].MaxImageUniforms =
> > > > -         (brw->intelScreen->compiler->scalar_vs ? BRW_MAX_IMAGES : 0);
> > > > +         (brw->intelScreen->compiler->scalar_stage[MESA_SHADER_VERTEX] ? BRW_MAX_IMAGES : 0);
> > > >        ctx->Const.Program[MESA_SHADER_COMPUTE].MaxImageUniforms =
> > > >           BRW_MAX_IMAGES;
> > > >        ctx->Const.MaxImageUnits = MAX_IMAGE_UNITS;
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c
> > > > index ed0890f..ad5b242 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_gs.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_gs.c
> > > > @@ -87,7 +87,8 @@ brw_codegen_gs_prog(struct brw_context *brw,
> > > >     prog_data.base.base.nr_image_params = gs->NumImages;
> > > >  
> > > >     brw_nir_setup_glsl_uniforms(gp->program.Base.nir, prog, &gp->program.Base,
> > > > -                               &prog_data.base.base, compiler->scalar_gs);
> > > > +                               &prog_data.base.base,
> > > > +                               compiler->scalar_stage[MESA_SHADER_GEOMETRY]);
> > > >  
> > > >     GLbitfield64 outputs_written = gp->program.Base.OutputsWritten;
> > > >  
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
> > > > index 2991173..14421d4 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> > > > +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> > > > @@ -66,12 +66,14 @@ brw_lower_packing_builtins(struct brw_context *brw,
> > > >                             gl_shader_stage shader_type,
> > > >                             exec_list *ir)
> > > >  {
> > > > +   const struct brw_compiler *compiler = brw->intelScreen->compiler;
> > > > +
> > > >     int ops = LOWER_PACK_SNORM_2x16
> > > >             | LOWER_UNPACK_SNORM_2x16
> > > >             | LOWER_PACK_UNORM_2x16
> > > >             | LOWER_UNPACK_UNORM_2x16;
> > > >  
> > > > -   if (is_scalar_shader_stage(brw->intelScreen->compiler, shader_type)) {
> > > > +   if (compiler->scalar_stage[shader_type]) {
> > > >        ops |= LOWER_UNPACK_UNORM_4x8
> > > >             | LOWER_UNPACK_SNORM_4x8
> > > >             | LOWER_PACK_UNORM_4x8
> > > > @@ -84,7 +86,7 @@ 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->intelScreen->compiler, shader_type)) {
> > > > +      if (compiler->scalar_stage[shader_type]) {
> > > >           ops |= LOWER_PACK_HALF_2x16_TO_SPLIT
> > > >               |  LOWER_UNPACK_HALF_2x16_TO_SPLIT;
> > > >        }
> > > > @@ -103,6 +105,7 @@ process_glsl_ir(gl_shader_stage stage,
> > > >                  struct gl_shader *shader)
> > > >  {
> > > >     struct gl_context *ctx = &brw->ctx;
> > > > +   const struct brw_compiler *compiler = brw->intelScreen->compiler;
> > > >     const struct gl_shader_compiler_options *options =
> > > >        &ctx->Const.ShaderCompilerOptions[shader->Stage];
> > > >  
> > > > @@ -161,7 +164,7 @@ process_glsl_ir(gl_shader_stage stage,
> > > >     do {
> > > >        progress = false;
> > > >  
> > > > -      if (is_scalar_shader_stage(brw->intelScreen->compiler, shader->Stage)) {
> > > > +      if (compiler->scalar_stage[shader->Stage]) {
> > > >           brw_do_channel_expressions(shader->ir);
> > > >           brw_do_vector_splitting(shader->ir);
> > > >        }
> > > > @@ -252,7 +255,7 @@ brw_link_shader(struct gl_context *ctx, struct gl_shader_program *shProg)
> > > >        brw_add_texrect_params(prog);
> > > >  
> > > >        prog->nir = brw_create_nir(brw, shProg, prog, (gl_shader_stage) stage,
> > > > -                                 is_scalar_shader_stage(compiler, stage));
> > > > +                                 compiler->scalar_stage[stage]);
> > > >  
> > > >        _mesa_reference_program(ctx, &prog, NULL);
> > > >     }
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_program.c b/src/mesa/drivers/dri/i965/brw_program.c
> > > > index 1ccfa1b..2297fa6 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_program.c
> > > > +++ b/src/mesa/drivers/dri/i965/brw_program.c
> > > > @@ -126,6 +126,7 @@ brwProgramStringNotify(struct gl_context *ctx,
> > > >  		       struct gl_program *prog)
> > > >  {
> > > >     struct brw_context *brw = brw_context(ctx);
> > > > +   const struct brw_compiler *compiler = brw->intelScreen->compiler;
> > > >  
> > > >     switch (target) {
> > > >     case GL_FRAGMENT_PROGRAM_ARB: {
> > > > @@ -165,7 +166,7 @@ brwProgramStringNotify(struct gl_context *ctx,
> > > >        brw_add_texrect_params(prog);
> > > >  
> > > >        prog->nir = brw_create_nir(brw, NULL, prog, MESA_SHADER_VERTEX,
> > > > -                                 brw->intelScreen->compiler->scalar_vs);
> > > > +                                 compiler->scalar_stage[MESA_SHADER_VERTEX]);
> > > >  
> > > >        brw_vs_precompile(ctx, NULL, prog);
> > > >        break;
> > > > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > > > index 1cf00f4..04cd6a8 100644
> > > > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> > > > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > > > @@ -72,22 +72,6 @@ shader_perf_log_mesa(void *data, const char *fmt, ...)
> > > >     va_end(args);
> > > >  }
> > > >  
> > > > -bool
> > > > -is_scalar_shader_stage(const struct brw_compiler *compiler, int stage)
> > > > -{
> > > > -   switch (stage) {
> > > > -   case MESA_SHADER_FRAGMENT:
> > > > -   case MESA_SHADER_COMPUTE:
> > > > -      return true;
> > > > -   case MESA_SHADER_GEOMETRY:
> > > > -      return compiler->scalar_gs;
> > > > -   case MESA_SHADER_VERTEX:
> > > > -      return compiler->scalar_vs;
> > > > -   default:
> > > > -      return false;
> > > > -   }
> > > > -}
> > > > -
> > > >  struct brw_compiler *
> > > >  brw_compiler_create(void *mem_ctx, const struct brw_device_info *devinfo)
> > > >  {
> > > > @@ -100,11 +84,12 @@ brw_compiler_create(void *mem_ctx, const struct brw_device_info *devinfo)
> > > >     brw_fs_alloc_reg_sets(compiler);
> > > >     brw_vec4_alloc_reg_set(compiler);
> > > >  
> > > > -   if (devinfo->gen >= 8 && !(INTEL_DEBUG & DEBUG_VEC4VS))
> > > > -      compiler->scalar_vs = true;
> > > > -
> > > > -   if (devinfo->gen >= 8 && brw_env_var_as_boolean("INTEL_SCALAR_GS", false))
> > > > -      compiler->scalar_gs = true;
> > > > +   compiler->scalar_stage[MESA_SHADER_VERTEX] =
> > > > +      devinfo->gen >= 8 && !(INTEL_DEBUG & DEBUG_VEC4VS);
> > > > +   compiler->scalar_stage[MESA_SHADER_GEOMETRY] =
> > > > +      devinfo->gen >= 8 && brw_env_var_as_boolean("INTEL_SCALAR_GS", false);
> > > > +   compiler->scalar_stage[MESA_SHADER_FRAGMENT] = true;
> > > > +   compiler->scalar_stage[MESA_SHADER_COMPUTE] = true;
> > > >  
> > > >     nir_shader_compiler_options *nir_options =
> > > >        rzalloc(compiler, nir_shader_compiler_options);
> > > > @@ -137,7 +122,7 @@ brw_compiler_create(void *mem_ctx, const struct brw_device_info *devinfo)
> > > >        compiler->glsl_compiler_options[i].EmitNoIndirectUniform = false;
> > > >        compiler->glsl_compiler_options[i].LowerClipDistance = true;
> > > >  
> > > > -      bool is_scalar = is_scalar_shader_stage(compiler, i);
> > > > +      bool is_scalar = compiler->scalar_stage[i];
> > > 
> > > Okay this works because MESA_SHADER_VERTEX is defined as zero. Would it be
> > > clearer to write it out explicitly:
> > > 
> > >          bool is_scalar = compiler->scalar_stage[i] != MESA_SHADER_VERTEX;
> > 
> > I'm confused, sorry.  scalar_stage[] is an array of booleans.
> > 
> > (scalar_stage[MESA_SHADER_VERTEX] == true) means that we're using SIMD8
> > mode for vertex shaders.
> 
> Right, sorry, I have no idea what I was thinking.
> 
> Anyway, I went through also patch 2 by checking that every removed assignment
> had a new counterpart. Both patches and the one increasing
> MaxCombinedUniformBlocks to take tesselation stages into account are:
> 
> Reviewed-by: Topi Pohjolainen <topi.pohjolainen at intel.com>

Thanks!  Sorry it was such a pain to review - no real good way to move
that much code around.  I definitely appreciate it.

--Ken
-------------- 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: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151116/e6deca00/attachment.sig>


More information about the mesa-dev mailing list