[Mesa-dev] [PATCH 05/53] st/mesa/glsl/i965: move ImageUnits and ImageAccess fields to gl_program
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Tue Jan 3 16:25:15 UTC 2017
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
On 03/01/17 02:43, Timothy Arceri wrote:
> Having it here rather than in gl_linked_shader allows us to simplify
> the code.
>
> Also it is error prone to depend on the gl_linked_shader for programs
> in current use because a failed linking attempt will free infomation
> about the current program. In i965 we could be trying to recompile
> a shader variant but may have lost some required fields.
>
> We drop the memset on ImageUnits because gl_program is already
> created using rzalloc().
> ---
> src/compiler/glsl/link_uniform_initializers.cpp | 5 +--
> src/compiler/glsl/link_uniforms.cpp | 3 +-
> src/mesa/drivers/dri/i965/brw_context.c | 3 +-
> src/mesa/drivers/dri/i965/brw_context.h | 1 -
> src/mesa/drivers/dri/i965/brw_gs_surface_state.c | 9 ++---
> src/mesa/drivers/dri/i965/brw_tcs_surface_state.c | 9 ++---
> src/mesa/drivers/dri/i965/brw_tes_surface_state.c | 9 ++---
> src/mesa/drivers/dri/i965/brw_vs_surface_state.c | 9 ++---
> src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 24 +++++---------
> src/mesa/main/mtypes.h | 40 +++++++++++------------
> src/mesa/main/uniform_query.cpp | 2 +-
> src/mesa/state_tracker/st_atom_image.c | 3 +-
> 12 files changed, 50 insertions(+), 67 deletions(-)
>
> diff --git a/src/compiler/glsl/link_uniform_initializers.cpp b/src/compiler/glsl/link_uniform_initializers.cpp
> index 93340ba..6f05a3a 100644
> --- a/src/compiler/glsl/link_uniform_initializers.cpp
> +++ b/src/compiler/glsl/link_uniform_initializers.cpp
> @@ -141,9 +141,10 @@ set_opaque_binding(void *mem_ctx, gl_shader_program *prog,
> storage->opaque[sh].active) {
> for (unsigned i = 0; i < elements; i++) {
> const unsigned index = storage->opaque[sh].index + i;
> - if (index >= ARRAY_SIZE(shader->ImageUnits))
> + if (index >= ARRAY_SIZE(shader->Program->sh.ImageUnits))
> break;
> - shader->ImageUnits[index] = storage->storage[i].i;
> + shader->Program->sh.ImageUnits[index] =
> + storage->storage[i].i;
> }
> }
> }
> diff --git a/src/compiler/glsl/link_uniforms.cpp b/src/compiler/glsl/link_uniforms.cpp
> index 256afa3..cd00837 100644
> --- a/src/compiler/glsl/link_uniforms.cpp
> +++ b/src/compiler/glsl/link_uniforms.cpp
> @@ -613,7 +613,7 @@ private:
> this->next_image += MAX2(1, uniform->array_elements);
>
> for (unsigned i = first; i < MIN2(next_image, MAX_IMAGE_UNIFORMS); i++)
> - prog->_LinkedShaders[shader_type]->ImageAccess[i] = access;
> + prog->_LinkedShaders[shader_type]->Program->sh.ImageAccess[i] = access;
> }
> }
>
> @@ -1308,7 +1308,6 @@ link_assign_uniform_locations(struct gl_shader_program *prog,
> * types cannot have initializers."
> */
> memset(sh->SamplerUnits, 0, sizeof(sh->SamplerUnits));
> - memset(sh->ImageUnits, 0, sizeof(sh->ImageUnits));
>
> link_update_uniform_buffer_variables(sh, i);
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 4ca77c7..e53aefd 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -280,7 +280,8 @@ intel_update_state(struct gl_context * ctx, GLuint new_state)
>
> if (unlikely(shader && shader->Program->info.num_images)) {
> for (unsigned j = 0; j < shader->Program->info.num_images; j++) {
> - struct gl_image_unit *u = &ctx->ImageUnits[shader->ImageUnits[j]];
> + struct gl_image_unit *u =
> + &ctx->ImageUnits[shader->Program->sh.ImageUnits[j]];
> tex_obj = intel_texture_object(u->TexObj);
>
> if (tex_obj && tex_obj->mt) {
> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
> index a281713..ad65a22 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.h
> +++ b/src/mesa/drivers/dri/i965/brw_context.h
> @@ -1512,7 +1512,6 @@ void brw_upload_abo_surfaces(struct brw_context *brw,
> struct brw_stage_state *stage_state,
> struct brw_stage_prog_data *prog_data);
> void brw_upload_image_surfaces(struct brw_context *brw,
> - struct gl_linked_shader *shader,
> const struct gl_program *prog,
> struct brw_stage_state *stage_state,
> struct brw_stage_prog_data *prog_data);
> diff --git a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> index cf56acf..e2ef222 100644
> --- a/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_gs_surface_state.c
> @@ -122,16 +122,13 @@ const struct brw_tracked_state brw_gs_abo_surfaces = {
> static void
> brw_upload_gs_image_surfaces(struct brw_context *brw)
> {
> - struct gl_context *ctx = &brw->ctx;
> /* BRW_NEW_GEOMETRY_PROGRAM */
> - struct gl_shader_program *prog =
> - ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY];
> const struct gl_program *gp = brw->geometry_program;
>
> - if (gp && prog) {
> + if (gp) {
> /* BRW_NEW_GS_PROG_DATA, BRW_NEW_IMAGE_UNITS, _NEW_TEXTURE */
> - brw_upload_image_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_GEOMETRY],
> - gp, &brw->gs.base, brw->gs.base.prog_data);
> + brw_upload_image_surfaces(brw, gp, &brw->gs.base,
> + brw->gs.base.prog_data);
> }
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c b/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c
> index b2d075f..16f0bd2 100644
> --- a/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_tcs_surface_state.c
> @@ -123,16 +123,13 @@ const struct brw_tracked_state brw_tcs_abo_surfaces = {
> static void
> brw_upload_tcs_image_surfaces(struct brw_context *brw)
> {
> - struct gl_context *ctx = &brw->ctx;
> /* BRW_NEW_TESS_PROGRAMS */
> - struct gl_shader_program *prog =
> - ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_CTRL];
> const struct gl_program *tcp = brw->tess_ctrl_program;
>
> - if (tcp && prog) {
> + if (tcp) {
> /* BRW_NEW_TCS_PROG_DATA, BRW_NEW_IMAGE_UNITS */
> - brw_upload_image_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_TESS_CTRL],
> - tcp, &brw->tcs.base, brw->tcs.base.prog_data);
> + brw_upload_image_surfaces(brw, tcp, &brw->tcs.base,
> + brw->tcs.base.prog_data);
> }
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_tes_surface_state.c b/src/mesa/drivers/dri/i965/brw_tes_surface_state.c
> index a351683..f74d869 100644
> --- a/src/mesa/drivers/dri/i965/brw_tes_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_tes_surface_state.c
> @@ -123,16 +123,13 @@ const struct brw_tracked_state brw_tes_abo_surfaces = {
> static void
> brw_upload_tes_image_surfaces(struct brw_context *brw)
> {
> - struct gl_context *ctx = &brw->ctx;
> /* BRW_NEW_TESS_PROGRAMS */
> - struct gl_shader_program *prog =
> - ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL];
> const struct gl_program *tep = brw->tess_eval_program;
>
> - if (tep && prog) {
> + if (tep) {
> /* BRW_NEW_TES_PROG_DATA, BRW_NEW_IMAGE_UNITS */
> - brw_upload_image_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_TESS_EVAL],
> - tep, &brw->tes.base, brw->tes.base.prog_data);
> + brw_upload_image_surfaces(brw, tep, &brw->tes.base,
> + brw->tes.base.prog_data);
> }
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> index 7bc7b77..6c349f4 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_vs_surface_state.c
> @@ -188,16 +188,13 @@ const struct brw_tracked_state brw_vs_abo_surfaces = {
> static void
> brw_upload_vs_image_surfaces(struct brw_context *brw)
> {
> - struct gl_context *ctx = &brw->ctx;
> /* BRW_NEW_VERTEX_PROGRAM */
> - struct gl_shader_program *prog =
> - ctx->_Shader->CurrentProgram[MESA_SHADER_VERTEX];
> const struct gl_program *vp = brw->vertex_program;
>
> - if (vp && prog) {
> + if (vp) {
> /* BRW_NEW_VS_PROG_DATA, BRW_NEW_IMAGE_UNITS, _NEW_TEXTURE */
> - brw_upload_image_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_VERTEX],
> - vp, &brw->vs.base, brw->vs.base.prog_data);
> + brw_upload_image_surfaces(brw, vp, &brw->vs.base,
> + brw->vs.base.prog_data);
> }
> }
>
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index 6c44381..34fec7e 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -1564,16 +1564,13 @@ const struct brw_tracked_state brw_cs_abo_surfaces = {
> static void
> brw_upload_cs_image_surfaces(struct brw_context *brw)
> {
> - struct gl_context *ctx = &brw->ctx;
> /* _NEW_PROGRAM */
> - struct gl_shader_program *prog =
> - ctx->_Shader->CurrentProgram[MESA_SHADER_COMPUTE];
> const struct gl_program *cp = brw->compute_program;
>
> - if (cp && prog) {
> + if (cp) {
> /* BRW_NEW_CS_PROG_DATA, BRW_NEW_IMAGE_UNITS, _NEW_TEXTURE */
> - brw_upload_image_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_COMPUTE],
> - cp, &brw->cs.base, brw->cs.base.prog_data);
> + brw_upload_image_surfaces(brw, cp, &brw->cs.base,
> + brw->cs.base.prog_data);
> }
> }
>
> @@ -1780,7 +1777,6 @@ update_image_surface(struct brw_context *brw,
>
> void
> brw_upload_image_surfaces(struct brw_context *brw,
> - struct gl_linked_shader *shader,
> const struct gl_program *prog,
> struct brw_stage_state *stage_state,
> struct brw_stage_prog_data *prog_data)
> @@ -1788,12 +1784,12 @@ brw_upload_image_surfaces(struct brw_context *brw,
> assert(prog);
> struct gl_context *ctx = &brw->ctx;
>
> - if (prog->info.num_images && shader) {
> + if (prog->info.num_images) {
> for (unsigned i = 0; i < prog->info.num_images; i++) {
> - struct gl_image_unit *u = &ctx->ImageUnits[shader->ImageUnits[i]];
> + struct gl_image_unit *u = &ctx->ImageUnits[prog->sh.ImageUnits[i]];
> const unsigned surf_idx = prog_data->binding_table.image_start + i;
>
> - update_image_surface(brw, u, shader->ImageAccess[i],
> + update_image_surface(brw, u, prog->sh.ImageAccess[i],
> surf_idx,
> &stage_state->surf_offset[surf_idx],
> &prog_data->image_param[i]);
> @@ -1811,15 +1807,13 @@ brw_upload_image_surfaces(struct brw_context *brw,
> static void
> brw_upload_wm_image_surfaces(struct brw_context *brw)
> {
> - struct gl_context *ctx = &brw->ctx;
> /* BRW_NEW_FRAGMENT_PROGRAM */
> - struct gl_shader_program *prog = ctx->_Shader->_CurrentFragmentProgram;
> const struct gl_program *wm = brw->fragment_program;
>
> - if (wm && prog) {
> + if (wm) {
> /* BRW_NEW_FS_PROG_DATA, BRW_NEW_IMAGE_UNITS, _NEW_TEXTURE */
> - brw_upload_image_surfaces(brw, prog->_LinkedShaders[MESA_SHADER_FRAGMENT],
> - wm, &brw->wm.base, brw->wm.base.prog_data);
> + brw_upload_image_surfaces(brw, wm, &brw->wm.base,
> + brw->wm.base.prog_data);
> }
> }
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index c7535a3..162627f 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -1980,6 +1980,25 @@ struct gl_program
> GLuint MaxSubroutineFunctionIndex;
> struct gl_subroutine_function *SubroutineFunctions;
>
> + /**
> + * Map from image uniform index to image unit (set by glUniform1i())
> + *
> + * An image uniform index is associated with each image uniform by
> + * the linker. The image index associated with each uniform is
> + * stored in the \c gl_uniform_storage::image field.
> + */
> + GLubyte ImageUnits[MAX_IMAGE_UNIFORMS];
> +
> + /**
> + * Access qualifier specified in the shader for each image uniform
> + * index. Either \c GL_READ_ONLY, \c GL_WRITE_ONLY or \c
> + * GL_READ_WRITE.
> + *
> + * It may be different, though only more strict than the value of
> + * \c gl_image_unit::Access for the corresponding image unit.
> + */
> + GLenum ImageAccess[MAX_IMAGE_UNIFORMS];
> +
> union {
> struct {
> /**
> @@ -2375,28 +2394,9 @@ struct gl_linked_shader
> struct glsl_symbol_table *symbols;
>
> /**
> - * Map from image uniform index to image unit (set by glUniform1i())
> - *
> - * An image uniform index is associated with each image uniform by
> - * the linker. The image index associated with each uniform is
> - * stored in the \c gl_uniform_storage::image field.
> - */
> - GLubyte ImageUnits[MAX_IMAGE_UNIFORMS];
> -
> - /**
> - * Access qualifier specified in the shader for each image uniform
> - * index. Either \c GL_READ_ONLY, \c GL_WRITE_ONLY or \c
> - * GL_READ_WRITE.
> - *
> - * It may be different, though only more strict than the value of
> - * \c gl_image_unit::Access for the corresponding image unit.
> - */
> - GLenum ImageAccess[MAX_IMAGE_UNIFORMS];
> -
> - /**
> * Number of image uniforms defined in the shader. It specifies
> * the number of valid elements in the \c ImageUnits and \c
> - * ImageAccess arrays above.
> + * ImageAccess arrays.
> */
> GLuint NumImages;
>
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index 3108d34..ec6d277 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -908,7 +908,7 @@ _mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shProg,
> struct gl_linked_shader *sh = shProg->_LinkedShaders[i];
>
> for (int j = 0; j < count; j++)
> - sh->ImageUnits[uni->opaque[i].index + offset + j] =
> + sh->Program->sh.ImageUnits[uni->opaque[i].index + offset + j] =
> ((GLint *) values)[j];
> }
> }
> diff --git a/src/mesa/state_tracker/st_atom_image.c b/src/mesa/state_tracker/st_atom_image.c
> index a7b7371..b30006a 100644
> --- a/src/mesa/state_tracker/st_atom_image.c
> +++ b/src/mesa/state_tracker/st_atom_image.c
> @@ -58,7 +58,8 @@ st_bind_images(struct st_context *st, struct gl_linked_shader *shader,
> c = &st->ctx->Const.Program[shader->Stage];
>
> for (i = 0; i < shader->NumImages; i++) {
> - struct gl_image_unit *u = &st->ctx->ImageUnits[shader->ImageUnits[i]];
> + struct gl_image_unit *u =
> + &st->ctx->ImageUnits[shader->Program->sh.ImageUnits[i]];
> struct st_texture_object *stObj = st_texture_object(u->TexObj);
> struct pipe_image_view *img = &images[i];
>
More information about the mesa-dev
mailing list