[Mesa-dev] [PATCH 2/2] glsl/mesa: stop duplicating geom and tcs layout values
Iago Toral
itoral at igalia.com
Wed Jun 22 14:54:03 UTC 2016
On Wed, 2016-06-22 at 12:41 +1000, Timothy Arceri wrote:
> We already store these in gl_shader and gl_program here we
> remove it from gl_shader_program and just use the values
> from gl_shader.
>
> This will allow us to keep the shader cache restore code as
> simple as it can be while making it somewhat clearer where these
> values originate from.
> ---
> src/compiler/glsl/glsl_to_nir.cpp | 2 +-
> src/compiler/glsl/linker.cpp | 18 ++++++----------
> src/mesa/drivers/dri/i965/brw_tcs.c | 6 ++++--
> src/mesa/main/api_validate.c | 6 ++++--
> src/mesa/main/mtypes.h | 20 +----------------
> src/mesa/main/shaderapi.c | 43 ++++++++++++++++++++++++-------------
> src/mesa/main/shaderobj.c | 6 +++---
> 7 files changed, 48 insertions(+), 53 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp
> index daf237e..16d0c1d 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -166,7 +166,7 @@ glsl_to_nir(const struct gl_shader_program *shader_prog,
>
> switch (stage) {
> case MESA_SHADER_TESS_CTRL:
> - shader->info.tcs.vertices_out = shader_prog->TessCtrl.VerticesOut;
> + shader->info.tcs.vertices_out = sh->TessCtrl.VerticesOut;
> break;
>
> case MESA_SHADER_GEOMETRY:
> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
> index ec71bfe..9a3c326 100644
> --- a/src/compiler/glsl/linker.cpp
> +++ b/src/compiler/glsl/linker.cpp
> @@ -861,7 +861,7 @@ validate_geometry_shader_executable(struct gl_shader_program *prog,
> if (shader == NULL)
> return;
>
> - unsigned num_vertices = vertices_per_prim(prog->Geom.InputType);
> + unsigned num_vertices = vertices_per_prim(shader->Geom.InputType);
> prog->Geom.VerticesIn = num_vertices;
>
> analyze_clip_cull_usage(prog, shader, ctx,
> @@ -877,9 +877,11 @@ static void
> validate_geometry_shader_emissions(struct gl_context *ctx,
> struct gl_shader_program *prog)
> {
> - if (prog->_LinkedShaders[MESA_SHADER_GEOMETRY] != NULL) {
> + struct gl_shader *sh = prog->_LinkedShaders[MESA_SHADER_GEOMETRY];
> +
> + if (sh != NULL) {
> find_emit_vertex_visitor emit_vertex(ctx->Const.MaxVertexStreams - 1);
> - emit_vertex.run(prog->_LinkedShaders[MESA_SHADER_GEOMETRY]->ir);
> + emit_vertex.run(sh->ir);
> if (emit_vertex.error()) {
> linker_error(prog, "Invalid call %s(%d). Accepted values for the "
> "stream parameter are in the range [0, %d].\n",
> @@ -914,7 +916,7 @@ validate_geometry_shader_emissions(struct gl_context *ctx,
> * EmitStreamVertex() or EmitEndPrimitive() are called with a non-zero
> * stream.
> */
> - if (prog->Geom.UsesStreams && prog->Geom.OutputType != GL_POINTS) {
> + if (prog->Geom.UsesStreams && sh->Geom.OutputType != GL_POINTS) {
> linker_error(prog, "EmitStreamVertex(n) and EndStreamPrimitive(n) "
> "with n>0 requires point output\n");
> }
> @@ -1797,7 +1799,6 @@ link_tcs_out_layout_qualifiers(struct gl_shader_program *prog,
> "vertices out layout qualifier\n");
> return;
> }
> - prog->TessCtrl.VerticesOut = linked_shader->TessCtrl.VerticesOut;
> }
>
>
> @@ -2059,26 +2060,21 @@ link_gs_inout_layout_qualifiers(struct gl_shader_program *prog,
> "geometry shader didn't declare primitive input type\n");
> return;
> }
> - prog->Geom.InputType = linked_shader->Geom.InputType;
>
> if (linked_shader->Geom.OutputType == PRIM_UNKNOWN) {
> linker_error(prog,
> "geometry shader didn't declare primitive output type\n");
> return;
> }
> - prog->Geom.OutputType = linked_shader->Geom.OutputType;
>
> if (linked_shader->Geom.VerticesOut == -1) {
> linker_error(prog,
> "geometry shader didn't declare max_vertices\n");
> return;
> }
> - prog->Geom.VerticesOut = linked_shader->Geom.VerticesOut;
>
> if (linked_shader->Geom.Invocations == 0)
> linked_shader->Geom.Invocations = 1;
> -
> - prog->Geom.Invocations = linked_shader->Geom.Invocations;
> }
>
>
> @@ -2353,7 +2349,7 @@ link_intrastage_shaders(void *mem_ctx,
>
> /* Set the size of geometry shader input arrays */
> if (linked->Stage == MESA_SHADER_GEOMETRY) {
> - unsigned num_vertices = vertices_per_prim(prog->Geom.InputType);
> + unsigned num_vertices = vertices_per_prim(linked->Geom.InputType);
> geom_array_resize_visitor input_resize_visitor(num_vertices, prog);
> foreach_in_list(ir_instruction, ir, linked->ir) {
> ir->accept(&input_resize_visitor);
> diff --git a/src/mesa/drivers/dri/i965/brw_tcs.c b/src/mesa/drivers/dri/i965/brw_tcs.c
> index a133fef..ff08c0a 100644
> --- a/src/mesa/drivers/dri/i965/brw_tcs.c
> +++ b/src/mesa/drivers/dri/i965/brw_tcs.c
> @@ -422,8 +422,10 @@ brw_tcs_precompile(struct gl_context *ctx,
> brw_setup_tex_for_precompile(brw, &key.tex, prog);
>
> /* Guess that the input and output patches have the same dimensionality. */
> - if (brw->gen < 8)
> - key.input_vertices = shader_prog->TessCtrl.VerticesOut;
> + if (brw->gen < 8) {
> + key.input_vertices = shader_prog->
> + _LinkedShaders[MESA_SHADER_TESS_CTRL]->TessCtrl.VerticesOut;
> + }
>
> key.tes_primitive_mode = brw->tess_eval_program ?
> brw->tess_eval_program->PrimitiveMode : GL_TRIANGLES;
> diff --git a/src/mesa/main/api_validate.c b/src/mesa/main/api_validate.c
> index ab34d99..8efbf50 100644
> --- a/src/mesa/main/api_validate.c
> +++ b/src/mesa/main/api_validate.c
> @@ -200,7 +200,8 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode, const char *name)
> */
> if (ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]) {
> const GLenum geom_mode =
> - ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]->Geom.InputType;
> + ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]->
> + _LinkedShaders[MESA_SHADER_GEOMETRY]->Geom.InputType;
> struct gl_shader_program *tes =
> ctx->_Shader->CurrentProgram[MESA_SHADER_TESS_EVAL];
> GLenum mode_before_gs = mode;
> @@ -305,7 +306,8 @@ _mesa_valid_prim_mode(struct gl_context *ctx, GLenum mode, const char *name)
> GLboolean pass = GL_TRUE;
>
> if(ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]) {
> - switch (ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]->Geom.OutputType) {
> + switch (ctx->_Shader->CurrentProgram[MESA_SHADER_GEOMETRY]->
> + _LinkedShaders[MESA_SHADER_GEOMETRY]->Geom.OutputType) {
> case GL_POINTS:
> pass = ctx->TransformFeedback.Mode == GL_POINTS;
> break;
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 168e2ae..0cd420e 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2718,17 +2718,6 @@ struct gl_shader_program
> enum gl_frag_depth_layout FragDepthLayout;
>
> /**
> - * Tessellation Control shader state from layout qualifiers.
> - */
> - struct {
> - /**
> - * 0 - vertices not declared in shader, or
> - * 1 .. GL_MAX_PATCH_VERTICES
> - */
> - GLint VerticesOut;
> - } TessCtrl;
> -
> - /**
> * Tessellation Evaluation shader state from layout qualifiers.
> */
> struct {
> @@ -2748,14 +2737,7 @@ struct gl_shader_program
> */
> struct {
> GLint VerticesIn;
ah, VerticesIn is not available in gl_shader, that is annoying :(
Seeing the result, I am not sure whether this is actually better or not.
Either way, the patch is:
Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>
Feel free to drop it if you are not convinced that this is not a good
idea though.
Iago
> - GLint VerticesOut;
> - /**
> - * 1 .. MAX_GEOMETRY_SHADER_INVOCATIONS
> - */
> - GLint Invocations;
> - GLenum InputType; /**< GL_POINTS, GL_LINES, GL_LINES_ADJACENCY_ARB,
> - GL_TRIANGLES, or GL_TRIANGLES_ADJACENCY_ARB */
> - GLenum OutputType; /**< GL_POINTS, GL_LINE_STRIP or GL_TRIANGLE_STRIP */
> +
> /**
> * True if gl_ClipDistance is written to. Copied into
> * gl_geometry_program by _mesa_copy_linked_program_data().
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index 7f8b296..16835bc 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -731,26 +731,34 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname,
> case GL_GEOMETRY_VERTICES_OUT:
> if (!has_core_gs)
> break;
> - if (check_gs_query(ctx, shProg))
> - *params = shProg->Geom.VerticesOut;
> + if (check_gs_query(ctx, shProg)) {
> + *params = shProg->_LinkedShaders[MESA_SHADER_GEOMETRY]->
> + Geom.VerticesOut;
> + }
> return;
> case GL_GEOMETRY_SHADER_INVOCATIONS:
> if (!has_core_gs || !ctx->Extensions.ARB_gpu_shader5)
> break;
> - if (check_gs_query(ctx, shProg))
> - *params = shProg->Geom.Invocations;
> + if (check_gs_query(ctx, shProg)) {
> + *params = shProg->_LinkedShaders[MESA_SHADER_GEOMETRY]->
> + Geom.Invocations;
> + }
> return;
> case GL_GEOMETRY_INPUT_TYPE:
> if (!has_core_gs)
> break;
> - if (check_gs_query(ctx, shProg))
> - *params = shProg->Geom.InputType;
> + if (check_gs_query(ctx, shProg)) {
> + *params = shProg->_LinkedShaders[MESA_SHADER_GEOMETRY]->
> + Geom.InputType;
> + }
> return;
> case GL_GEOMETRY_OUTPUT_TYPE:
> if (!has_core_gs)
> break;
> - if (check_gs_query(ctx, shProg))
> - *params = shProg->Geom.OutputType;
> + if (check_gs_query(ctx, shProg)) {
> + *params = shProg->_LinkedShaders[MESA_SHADER_GEOMETRY]->
> + Geom.OutputType;
> + }
> return;
> case GL_ACTIVE_UNIFORM_BLOCK_MAX_NAME_LENGTH: {
> unsigned i;
> @@ -825,8 +833,10 @@ get_programiv(struct gl_context *ctx, GLuint program, GLenum pname,
> case GL_TESS_CONTROL_OUTPUT_VERTICES:
> if (!has_tess)
> break;
> - if (check_tcs_query(ctx, shProg))
> - *params = shProg->TessCtrl.VerticesOut;
> + if (check_tcs_query(ctx, shProg)) {
> + *params = shProg->_LinkedShaders[MESA_SHADER_TESS_CTRL]->
> + TessCtrl.VerticesOut;
> + }
> return;
> case GL_TESS_GEN_MODE:
> if (!has_tess)
> @@ -2163,7 +2173,8 @@ _mesa_copy_linked_program_data(gl_shader_stage type,
> case MESA_SHADER_TESS_CTRL: {
> struct gl_tess_ctrl_program *dst_tcp =
> (struct gl_tess_ctrl_program *) dst;
> - dst_tcp->VerticesOut = src->TessCtrl.VerticesOut;
> + dst_tcp->VerticesOut =
> + src->_LinkedShaders[MESA_SHADER_TESS_CTRL]->TessCtrl.VerticesOut;
> break;
> }
> case MESA_SHADER_TESS_EVAL: {
> @@ -2181,11 +2192,13 @@ _mesa_copy_linked_program_data(gl_shader_stage type,
> }
> case MESA_SHADER_GEOMETRY: {
> struct gl_geometry_program *dst_gp = (struct gl_geometry_program *) dst;
> + struct gl_shader *geom_sh = src->_LinkedShaders[MESA_SHADER_GEOMETRY];
> +
> dst_gp->VerticesIn = src->Geom.VerticesIn;
> - dst_gp->VerticesOut = src->Geom.VerticesOut;
> - dst_gp->Invocations = src->Geom.Invocations;
> - dst_gp->InputType = src->Geom.InputType;
> - dst_gp->OutputType = src->Geom.OutputType;
> + dst_gp->VerticesOut = geom_sh->Geom.VerticesOut;
> + dst_gp->Invocations = geom_sh->Geom.Invocations;
> + dst_gp->InputType = geom_sh->Geom.InputType;
> + dst_gp->OutputType = geom_sh->Geom.OutputType;
> dst->ClipDistanceArraySize = src->Geom.ClipDistanceArraySize;
> dst->CullDistanceArraySize = src->Geom.CullDistanceArraySize;
> dst_gp->UsesEndPrimitive = src->Geom.UsesEndPrimitive;
> diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
> index 8ffa24d..179a970 100644
> --- a/src/mesa/main/shaderobj.c
> +++ b/src/mesa/main/shaderobj.c
> @@ -92,6 +92,9 @@ void
> _mesa_init_shader(struct gl_context *ctx, struct gl_shader *shader)
> {
> shader->RefCount = 1;
> + shader->Geom.VerticesOut = -1;
> + shader->Geom.InputType = GL_TRIANGLES;
> + shader->Geom.OutputType = GL_TRIANGLE_STRIP;
> }
>
> /**
> @@ -231,9 +234,6 @@ init_shader_program(struct gl_shader_program *prog)
> prog->FragDataBindings = string_to_uint_map_ctor();
> prog->FragDataIndexBindings = string_to_uint_map_ctor();
>
> - prog->Geom.VerticesOut = -1;
> - prog->Geom.InputType = GL_TRIANGLES;
> - prog->Geom.OutputType = GL_TRIANGLE_STRIP;
> prog->Geom.UsesEndPrimitive = false;
> prog->Geom.UsesStreams = false;
>
More information about the mesa-dev
mailing list