[Mesa-dev] [PATCH 1/7] mesa: Clean up nomenclature for pipeline stages.
Brian Paul
brianp at vmware.com
Tue Jan 7 15:06:27 PST 2014
On 01/07/2014 03:13 PM, Paul Berry wrote:
> Previously, we had an enum called gl_shader_type which represented
> pipeline stages in the order they occur in the pipeline
> (i.e. MESA_SHADER_VERTEX=0, MESA_SHADER_GEOMETRY=1, etc), and several
> inconsistently named functions for converting between it and other
> representations:
>
> - _mesa_shader_type_to_string: gl_shader_type -> string
> - _mesa_shader_type_to_index: GLenum (GL_*_SHADER) -> gl_shader_type
> - _mesa_program_target_to_index: GLenum (GL_*_PROGRAM) -> gl_shader_type
> - _mesa_shader_enum_to_string: GLenum (GL_*_{SHADER,PROGRAM}) -> string
>
> This patch tries to clean things up so that we use more consistent
> terminology: the enum is now called gl_shader_stage (to emphasize that
> it is in the order of pipeline stages), and the conversion functions are:
>
> - _mesa_shader_stage_to_string: gl_shader_stage -> string
> - _mesa_shader_enum_to_shader_stage: GLenum (GL_*_SHADER) -> gl_shader_stage
> - _mesa_program_enum_to_shader_stage: GLenum (GL_*_PROGRAM) -> gl_shader_type
> - _mesa_progshader_enum_to_string: GLenum (GL_*_{SHADER,PROGRAM}) -> string
>
> In addition, MESA_SHADER_TYPES has been renamed to MESA_SHADER_STAGES,
> for consistency with the new name for the enum.
This sounds great, Paul. Minor comments below and in a few other patches.
Reviewed-by: Brian Paul <brianp at vmware.com>
> ---
> src/glsl/ast_to_hir.cpp | 12 ++---
> src/glsl/glsl_parser_extras.cpp | 14 +++---
> src/glsl/glsl_parser_extras.h | 8 +--
> src/glsl/ir_uniform.h | 2 +-
> src/glsl/link_atomics.cpp | 22 ++++-----
> src/glsl/link_uniform_initializers.cpp | 8 +--
> src/glsl/link_uniforms.cpp | 12 ++---
> src/glsl/link_varyings.cpp | 24 ++++-----
> src/glsl/linker.cpp | 62 ++++++++++++------------
> src/glsl/main.cpp | 2 +-
> src/glsl/standalone_scaffolding.cpp | 2 +-
> src/glsl/standalone_scaffolding.h | 6 +--
> src/glsl/test_optpass.cpp | 2 +-
> src/glsl/tests/set_uniform_initializer_tests.cpp | 4 +-
> src/mesa/drivers/dri/i965/brw_context.c | 2 +-
> src/mesa/drivers/dri/i965/brw_shader.cpp | 10 ++--
> src/mesa/main/context.c | 8 +--
> src/mesa/main/mtypes.h | 12 ++---
> src/mesa/main/shaderapi.c | 14 +++---
> src/mesa/main/shaderapi.h | 2 +-
> src/mesa/main/shaderobj.c | 4 +-
> src/mesa/main/shaderobj.h | 6 +--
> src/mesa/main/uniform_query.cpp | 6 +--
> src/mesa/main/uniforms.c | 4 +-
> src/mesa/program/ir_to_mesa.cpp | 18 +++----
> src/mesa/program/program.c | 2 +-
> src/mesa/program/program.h | 8 +--
> src/mesa/program/sampler.cpp | 2 +-
> src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 12 ++---
> 29 files changed, 145 insertions(+), 145 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 91810f9..9f1e4e6 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -1882,7 +1882,7 @@ ast_fully_specified_type::glsl_type(const char **name,
> * this function will produce undefined results.
> */
> static bool
> -is_varying_var(ir_variable *var, gl_shader_type target)
> +is_varying_var(ir_variable *var, gl_shader_stage target)
> {
> switch (target) {
> case MESA_SHADER_VERTEX:
> @@ -2110,7 +2110,7 @@ validate_explicit_location(const struct ast_type_qualifier *qual,
> _mesa_glsl_error(loc, state,
> "%s cannot be given an explicit location in %s shader",
> mode_string(var),
> - _mesa_shader_type_to_string(state->target));
> + _mesa_shader_stage_to_string(state->target));
> } else {
> var->data.explicit_location = true;
>
> @@ -2188,7 +2188,7 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
> _mesa_glsl_error(loc, state,
> "`attribute' variables may not be declared in the "
> "%s shader",
> - _mesa_shader_type_to_string(state->target));
> + _mesa_shader_stage_to_string(state->target));
> }
>
> /* Section 6.1.1 (Function Calling Conventions) of the GLSL 1.10 spec says:
> @@ -2599,7 +2599,7 @@ process_initializer(ir_variable *var, ast_declaration *decl,
> if ((var->data.mode == ir_var_shader_in) && (state->current_function == NULL)) {
> _mesa_glsl_error(& initializer_loc, state,
> "cannot initialize %s shader input / %s",
> - _mesa_shader_type_to_string(state->target),
> + _mesa_shader_stage_to_string(state->target),
> (state->target == MESA_SHADER_VERTEX)
> ? "attribute" : "varying");
> }
> @@ -4890,7 +4890,7 @@ ast_interface_block::hir(exec_list *instructions,
> _mesa_glsl_error(&loc, state,
> "redeclaration of gl_PerVertex input not allowed "
> "in the %s shader",
> - _mesa_shader_type_to_string(state->target));
> + _mesa_shader_stage_to_string(state->target));
> }
> if (this->instance_name == NULL ||
> strcmp(this->instance_name, "gl_in") != 0 || !this->is_array) {
> @@ -4907,7 +4907,7 @@ ast_interface_block::hir(exec_list *instructions,
> _mesa_glsl_error(&loc, state,
> "redeclaration of gl_PerVertex output not "
> "allowed in the %s shader",
> - _mesa_shader_type_to_string(state->target));
> + _mesa_shader_stage_to_string(state->target));
> }
> if (this->instance_name != NULL) {
> _mesa_glsl_error(&loc, state,
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index e839caf..492f2ac 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -342,7 +342,7 @@ extern "C" {
> * gl_shader->Type.
> */
> const char *
> -_mesa_shader_enum_to_string(GLenum type)
> +_mesa_progshader_enum_to_string(GLenum type)
> {
> switch (type) {
> case GL_VERTEX_SHADER:
> @@ -362,11 +362,11 @@ _mesa_shader_enum_to_string(GLenum type)
> } /* extern "C" */
>
> /**
> - * Translate a gl_shader_type to a short shader stage name for debug printouts
> - * and error messages.
> + * Translate a gl_shader_stage to a short shader stage name for debug
> + * printouts and error messages.
> */
> const char *
> -_mesa_shader_type_to_string(unsigned target)
> +_mesa_shader_stage_to_string(unsigned target)
> {
> switch (target) {
> case MESA_SHADER_VERTEX: return "vertex";
> @@ -651,11 +651,11 @@ _mesa_glsl_process_extension(const char *name, YYLTYPE *name_locp,
>
> if (behavior == extension_require) {
> _mesa_glsl_error(name_locp, state, fmt,
> - name, _mesa_shader_type_to_string(state->target));
> + name, _mesa_shader_stage_to_string(state->target));
> return false;
> } else {
> _mesa_glsl_warning(name_locp, state, fmt,
> - name, _mesa_shader_type_to_string(state->target));
> + name, _mesa_shader_stage_to_string(state->target));
> }
> }
> }
> @@ -1516,7 +1516,7 @@ _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
>
> if (!state->error && !shader->ir->is_empty()) {
> struct gl_shader_compiler_options *options =
> - &ctx->ShaderCompilerOptions[_mesa_shader_type_to_index(shader->Type)];
> + &ctx->ShaderCompilerOptions[_mesa_shader_enum_to_shader_stage(shader->Type)];
>
> /* Do some optimization at compile time to reduce shader IR size
> * and reduce later work if the same shader is linked multiple times
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index 0e281ae..ecf832f 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -165,7 +165,7 @@ struct _mesa_glsl_parse_state {
>
> bool es_shader;
> unsigned language_version;
> - gl_shader_type target;
> + gl_shader_stage target;
Should "target" be renamed to "stage"?
>
> /**
> * Number of nested struct_specifier levels
> @@ -425,10 +425,10 @@ extern bool _mesa_glsl_process_extension(const char *name, YYLTYPE *name_locp,
>
> /**
> * Get the textual name of the specified shader target (which is a
> - * gl_shader_type).
> + * gl_shader_stage).
> */
> extern const char *
> -_mesa_shader_type_to_string(unsigned target);
> +_mesa_shader_stage_to_string(unsigned target);
s/target/stage/ ??
More information about the mesa-dev
mailing list