On Thu, Nov 5, 2015 at 5:39 PM, Timothy Arceri <timothy.arc...@collabora.com> 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 <k...@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]); >> + } >> + >> 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. */ >> + > > Does it even matter if this pass was to run for drivers that don't support > UBO/SSBO? > > I assume all other drivers would want to run this pass, in which case is it > really helpful to a this flag?
I think it was made optional in case some driver did not want UBO access lowered to intrinsics, or wanted to run a custom lowering pass. For example, if a driver wanted to push some UBO contents instead of pulling, it could make that decision there. I don't know. There are a lot of similar options that are also always enabled and I didn't want to change whether or not GLSL IR consumers can decide to run the lowering pass with this series. Kristian >> 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); _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev