[Mesa-dev] [PATCH 2/3] glsl: Lower UBO and SSBO access in glsl linker

[Iago Toral]

On Mon, 2015-11-09 at 16:52 +0100, Iago Toral wrote:
> On Wed, 2015-11-04 at 15:33 -0800, Kristian Høgsberg Kristensen wrote:
> > All GLSL IR consumers run this lowering pass so we can move it to the
> > linker. This moves the pass up quite a bit, but that's the point: it
> > needs to run before we throw away information about per-component vector
> > access.
> > 
> > Signed-off-by: Kristian Høgsberg Kristensen <krh at bitplanet.net>
> > ---
> >  src/glsl/linker.cpp                        | 8 ++++++++
> >  src/mesa/drivers/dri/i965/brw_link.cpp     | 2 --
> >  src/mesa/drivers/dri/i965/brw_shader.cpp   | 2 ++
> >  src/mesa/main/mtypes.h                     | 2 ++
> >  src/mesa/state_tracker/st_extensions.c     | 1 +
> >  src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 -
> >  6 files changed, 13 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> > index c35d87a..ea6a3f3 100644
> > --- a/src/glsl/linker.cpp
> > +++ b/src/glsl/linker.cpp
> > @@ -4449,6 +4449,14 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
> >  
> >     /* FINISHME: Assign fragment shader output locations. */
> >  
> > +   for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> > +      if (prog->_LinkedShaders[i] == NULL)
> > +	 continue;
> > +
> > +      if (ctx->Const.ShaderCompilerOptions[i].LowerBufferInterfaceBlocks)
> > +         lower_ubo_reference(prog->_LinkedShaders[i]);
> > +   }
> > +
> 
> It probably makes more sense to rewrite this loop as:
> 
> if (ctx->Const.ShaderCompilerOptions[i].LowerBufferInterfaceBlocks) {
>    for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
>       if (prog->_LinkedShaders[i] != NULL)
>          lower_ubo_reference(prog->_LinkedShaders[i]);
>    }
> }
> 
> With that change, and assuming that this change is not responsible for
> the shader-db regressions posted by Jason:

Forget about that, I did not notice that LowerBufferInterfaceBlocks is
set by stage. You can keep the Rb for the original version.

Iago

> Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
> 
> >  done:
> >     for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
> >        free(shader_list[i]);
> > diff --git a/src/mesa/drivers/dri/i965/brw_link.cpp b/src/mesa/drivers/dri/i965/brw_link.cpp
> > index f1e3860..2991173 100644
> > --- a/src/mesa/drivers/dri/i965/brw_link.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_link.cpp
> > @@ -157,8 +157,6 @@ process_glsl_ir(gl_shader_stage stage,
> >                   _mesa_shader_stage_to_abbrev(shader->Stage));
> >     }
> >  
> > -   lower_ubo_reference(shader);
> > -
> >     bool progress;
> >     do {
> >        progress = false;
> > diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > index 4ea297a..5adc986 100644
> > --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> > @@ -148,6 +148,8 @@ brw_compiler_create(void *mem_ctx, const struct brw_device_info *devinfo)
> >           compiler->glsl_compiler_options[i].EmitNoIndirectSampler = true;
> >  
> >        compiler->glsl_compiler_options[i].NirOptions = nir_options;
> > +
> > +      compiler->glsl_compiler_options[i].LowerBufferInterfaceBlocks = true;
> >     }
> >  
> >     return compiler;
> > diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> > index d6c1eb8..800ad81 100644
> > --- a/src/mesa/main/mtypes.h
> > +++ b/src/mesa/main/mtypes.h
> > @@ -2874,6 +2874,8 @@ struct gl_shader_compiler_options
> >      */
> >     GLboolean OptimizeForAOS;
> >  
> > +   GLboolean LowerBufferInterfaceBlocks; /**< Lower UBO and SSBO access to intrinsics. */
> > +
> >     const struct nir_shader_compiler_options *NirOptions;
> >  };
> >  
> > diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
> > index bd7cbcc..bbb9027 100644
> > --- a/src/mesa/state_tracker/st_extensions.c
> > +++ b/src/mesa/state_tracker/st_extensions.c
> > @@ -254,6 +254,7 @@ void st_init_limits(struct pipe_screen *screen,
> >                                        PIPE_SHADER_CAP_MAX_UNROLL_ITERATIONS_HINT);
> >  
> >        options->LowerClipDistance = true;
> > +      options->LowerBufferInterfaceBlocks = true;
> >     }
> >  
> >     c->LowerTessLevel = true;
> > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > index ca00930..9ee6f8f 100644
> > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > @@ -5822,7 +5822,6 @@ st_link_shader(struct gl_context *ctx, struct gl_shader_program *prog)
> >                           (!ctx->Const.NativeIntegers ? INT_DIV_TO_MUL_RCP : 0) |
> >                           (options->EmitNoSat ? SAT_TO_CLAMP : 0));
> >  
> > -      lower_ubo_reference(prog->_LinkedShaders[i]);
> >        do_vec_index_to_cond_assign(ir);
> >        lower_vector_insert(ir, true);
> >        lower_quadop_vector(ir, false);
>