[Mesa-dev] [PATCH] glsl: Drop the round-trip through ast_type_specifier for many builtin types.

Kenneth Graunke kenneth at whitecape.org
Thu Mar 29 22:53:09 PDT 2012


On 03/29/2012 02:37 PM, Eric Anholt wrote:
> We have lexer recognition of a bunch of our types based on the
> handling.  This code was mapping those recognized tokens to an enum
> and then to a string of their name.  Just drop the enums and provide
> the string directly in the parser.
> ---
>   src/glsl/ast.h          |   66 +----------------------------
>   src/glsl/ast_to_hir.cpp |    4 +-
>   src/glsl/ast_type.cpp   |   68 +-----------------------------
>   src/glsl/glsl_parser.yy |  106 +++++++++++++++++++++++------------------------
>   4 files changed, 58 insertions(+), 186 deletions(-)
>
> diff --git a/src/glsl/ast.h b/src/glsl/ast.h
> index 1f78af8..e6707ac 100644
> --- a/src/glsl/ast.h
> +++ b/src/glsl/ast.h
> @@ -418,72 +418,12 @@ public:
>   };
>
>
> -enum ast_types {
> -   ast_void,
> -   ast_float,
> -   ast_int,
> -   ast_uint,
> -   ast_bool,
> -   ast_vec2,
> -   ast_vec3,
> -   ast_vec4,
> -   ast_bvec2,
> -   ast_bvec3,
> -   ast_bvec4,
> -   ast_ivec2,
> -   ast_ivec3,
> -   ast_ivec4,
> -   ast_uvec2,
> -   ast_uvec3,
> -   ast_uvec4,
> -   ast_mat2,
> -   ast_mat2x3,
> -   ast_mat2x4,
> -   ast_mat3x2,
> -   ast_mat3,
> -   ast_mat3x4,
> -   ast_mat4x2,
> -   ast_mat4x3,
> -   ast_mat4,
> -   ast_sampler1d,
> -   ast_sampler2d,
> -   ast_sampler2drect,
> -   ast_sampler3d,
> -   ast_samplercube,
> -   ast_samplerexternaloes,
> -   ast_sampler1dshadow,
> -   ast_sampler2dshadow,
> -   ast_sampler2drectshadow,
> -   ast_samplercubeshadow,
> -   ast_sampler1darray,
> -   ast_sampler2darray,
> -   ast_sampler1darrayshadow,
> -   ast_sampler2darrayshadow,
> -   ast_isampler1d,
> -   ast_isampler2d,
> -   ast_isampler3d,
> -   ast_isamplercube,
> -   ast_isampler1darray,
> -   ast_isampler2darray,
> -   ast_usampler1d,
> -   ast_usampler2d,
> -   ast_usampler3d,
> -   ast_usamplercube,
> -   ast_usampler1darray,
> -   ast_usampler2darray,
> -
> -   ast_struct,
> -   ast_type_name
> -};
> -
>
>   class ast_type_specifier : public ast_node {
>   public:
> -   ast_type_specifier(int specifier);
> -
>      /** Construct a type specifier from a type name */
>      ast_type_specifier(const char *name)
> -      : type_specifier(ast_type_name), type_name(name), structure(NULL),
> +      : type_name(name), structure(NULL),
>   	is_array(false), array_size(NULL), precision(ast_precision_none),
>   	is_precision_statement(false)
>      {
> @@ -492,7 +432,7 @@ public:
>
>      /** Construct a type specifier from a structure definition */
>      ast_type_specifier(ast_struct_specifier *s)
> -      : type_specifier(ast_struct), type_name(s->name), structure(s),
> +      : type_name(s->name), structure(s),
>   	is_array(false), array_size(NULL), precision(ast_precision_none),
>   	is_precision_statement(false)
>      {
> @@ -507,8 +447,6 @@ public:
>
>      ir_rvalue *hir(exec_list *, struct _mesa_glsl_parse_state *);
>
> -   enum ast_types type_specifier;
> -
>      const char *type_name;
>      ast_struct_specifier *structure;
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index ff56e33..dd02301 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -3904,8 +3904,8 @@ ast_type_specifier::hir(exec_list *instructions,
>                             "arrays");
>            return NULL;
>         }
> -      if (this->type_specifier != ast_float
> -&&  this->type_specifier != ast_int) {
> +      if (strcmp(this->type_name, "float") != 0&&
> +	  strcmp(this->type_name, "int") != 0) {
>            _mesa_glsl_error(&loc, state,
>                             "default precision statements apply only to types "
>                             "float and int");
> diff --git a/src/glsl/ast_type.cpp b/src/glsl/ast_type.cpp
> index 79c43ee..6c44f8c 100644
> --- a/src/glsl/ast_type.cpp
> +++ b/src/glsl/ast_type.cpp
> @@ -29,7 +29,7 @@ extern "C" {
>   void
>   ast_type_specifier::print(void) const
>   {
> -   if (type_specifier == ast_struct) {
> +   if (structure) {
>         structure->print();
>      } else {
>         printf("%s ", type_name);
> @@ -46,72 +46,6 @@ ast_type_specifier::print(void) const
>      }
>   }
>
> -ast_type_specifier::ast_type_specifier(int specifier)
> -      : type_specifier(ast_types(specifier)), type_name(NULL), structure(NULL),
> -	is_array(false), array_size(NULL), precision(ast_precision_none),
> -	is_precision_statement(false)
> -{
> -   static const char *const names[] = {
> -      "void",
> -      "float",
> -      "int",
> -      "uint",
> -      "bool",
> -      "vec2",
> -      "vec3",
> -      "vec4",
> -      "bvec2",
> -      "bvec3",
> -      "bvec4",
> -      "ivec2",
> -      "ivec3",
> -      "ivec4",
> -      "uvec2",
> -      "uvec3",
> -      "uvec4",
> -      "mat2",
> -      "mat2x3",
> -      "mat2x4",
> -      "mat3x2",
> -      "mat3",
> -      "mat3x4",
> -      "mat4x2",
> -      "mat4x3",
> -      "mat4",
> -      "sampler1D",
> -      "sampler2D",
> -      "sampler2DRect",
> -      "sampler3D",
> -      "samplerCube",
> -      "samplerExternalOES",
> -      "sampler1DShadow",
> -      "sampler2DShadow",
> -      "sampler2DRectShadow",
> -      "samplerCubeShadow",
> -      "sampler1DArray",
> -      "sampler2DArray",
> -      "sampler1DArrayShadow",
> -      "sampler2DArrayShadow",
> -      "isampler1D",
> -      "isampler2D",
> -      "isampler3D",
> -      "isamplerCube",
> -      "isampler1DArray",
> -      "isampler2DArray",
> -      "usampler1D",
> -      "usampler2D",
> -      "usampler3D",
> -      "usamplerCube",
> -      "usampler1DArray",
> -      "usampler2DArray",
> -
> -      NULL, /* ast_struct */
> -      NULL  /* ast_type_name */
> -   };
> -
> -   type_name = names[specifier];
> -}
> -
>   bool
>   ast_fully_specified_type::has_qualifiers() const
>   {
> diff --git a/src/glsl/glsl_parser.yy b/src/glsl/glsl_parser.yy
> index 64506b6..5ce69b6 100644
> --- a/src/glsl/glsl_parser.yy
> +++ b/src/glsl/glsl_parser.yy
> @@ -156,7 +156,7 @@ static void yyerror(YYLTYPE *loc, _mesa_glsl_parse_state *st, const char *msg)
>   %type<type_specifier>  type_specifier
>   %type<type_specifier>  type_specifier_no_prec
>   %type<type_specifier>  type_specifier_nonarray
> -%type<n>  basic_type_specifier_nonarray
> +%type<identifier>  basic_type_specifier_nonarray
>   %type<fully_specified_type>  fully_specified_type
>   %type<function>  function_prototype
>   %type<function>  function_header
> @@ -1365,58 +1365,58 @@ type_specifier_nonarray:
>   	;
>
>   basic_type_specifier_nonarray:
> -	VOID_TOK		{ $$ = ast_void; }
> -	| FLOAT_TOK		{ $$ = ast_float; }
> -	| INT_TOK		{ $$ = ast_int; }
> -	| UINT_TOK		{ $$ = ast_uint; }
> -	| BOOL_TOK		{ $$ = ast_bool; }
> -	| VEC2			{ $$ = ast_vec2; }
> -	| VEC3			{ $$ = ast_vec3; }
> -	| VEC4			{ $$ = ast_vec4; }
> -	| BVEC2			{ $$ = ast_bvec2; }
> -	| BVEC3			{ $$ = ast_bvec3; }
> -	| BVEC4			{ $$ = ast_bvec4; }
> -	| IVEC2			{ $$ = ast_ivec2; }
> -	| IVEC3			{ $$ = ast_ivec3; }
> -	| IVEC4			{ $$ = ast_ivec4; }
> -	| UVEC2			{ $$ = ast_uvec2; }
> -	| UVEC3			{ $$ = ast_uvec3; }
> -	| UVEC4			{ $$ = ast_uvec4; }
> -	| MAT2X2		{ $$ = ast_mat2; }
> -	| MAT2X3		{ $$ = ast_mat2x3; }
> -	| MAT2X4		{ $$ = ast_mat2x4; }
> -	| MAT3X2		{ $$ = ast_mat3x2; }
> -	| MAT3X3		{ $$ = ast_mat3; }
> -	| MAT3X4		{ $$ = ast_mat3x4; }
> -	| MAT4X2		{ $$ = ast_mat4x2; }
> -	| MAT4X3		{ $$ = ast_mat4x3; }
> -	| MAT4X4		{ $$ = ast_mat4; }
> -	| SAMPLER1D		{ $$ = ast_sampler1d; }
> -	| SAMPLER2D		{ $$ = ast_sampler2d; }
> -	| SAMPLER2DRECT		{ $$ = ast_sampler2drect; }
> -	| SAMPLER3D		{ $$ = ast_sampler3d; }
> -	| SAMPLERCUBE		{ $$ = ast_samplercube; }
> -	| SAMPLEREXTERNALOES	{ $$ = ast_samplerexternaloes; }
> -	| SAMPLER1DSHADOW	{ $$ = ast_sampler1dshadow; }
> -	| SAMPLER2DSHADOW	{ $$ = ast_sampler2dshadow; }
> -	| SAMPLER2DRECTSHADOW	{ $$ = ast_sampler2drectshadow; }
> -	| SAMPLERCUBESHADOW	{ $$ = ast_samplercubeshadow; }
> -	| SAMPLER1DARRAY	{ $$ = ast_sampler1darray; }
> -	| SAMPLER2DARRAY	{ $$ = ast_sampler2darray; }
> -	| SAMPLER1DARRAYSHADOW	{ $$ = ast_sampler1darrayshadow; }
> -	| SAMPLER2DARRAYSHADOW	{ $$ = ast_sampler2darrayshadow; }
> -	| ISAMPLER1D		{ $$ = ast_isampler1d; }
> -	| ISAMPLER2D		{ $$ = ast_isampler2d; }
> -	| ISAMPLER3D		{ $$ = ast_isampler3d; }
> -	| ISAMPLERCUBE		{ $$ = ast_isamplercube; }
> -	| ISAMPLER1DARRAY	{ $$ = ast_isampler1darray; }
> -	| ISAMPLER2DARRAY	{ $$ = ast_isampler2darray; }
> -	| USAMPLER1D		{ $$ = ast_usampler1d; }
> -	| USAMPLER2D		{ $$ = ast_usampler2d; }
> -	| USAMPLER3D		{ $$ = ast_usampler3d; }
> -	| USAMPLERCUBE		{ $$ = ast_usamplercube; }
> -	| USAMPLER1DARRAY	{ $$ = ast_usampler1darray; }
> -	| USAMPLER2DARRAY	{ $$ = ast_usampler2darray; }
> +	VOID_TOK		{ $$ = (char *)"void"; }
> +	| FLOAT_TOK		{ $$ = (char *)"float"; }
> +	| INT_TOK		{ $$ = (char *)"int"; }
> +	| UINT_TOK		{ $$ = (char *)"uint"; }
> +	| BOOL_TOK		{ $$ = (char *)"bool"; }
> +	| VEC2			{ $$ = (char *)"vec2"; }
> +	| VEC3			{ $$ = (char *)"vec3"; }
> +	| VEC4			{ $$ = (char *)"vec4"; }
> +	| BVEC2			{ $$ = (char *)"bvec2"; }
> +	| BVEC3			{ $$ = (char *)"bvec3"; }
> +	| BVEC4			{ $$ = (char *)"bvec4"; }
> +	| IVEC2			{ $$ = (char *)"ivec2"; }
> +	| IVEC3			{ $$ = (char *)"ivec3"; }
> +	| IVEC4			{ $$ = (char *)"ivec4"; }
> +	| UVEC2			{ $$ = (char *)"uvec2"; }
> +	| UVEC3			{ $$ = (char *)"uvec3"; }
> +	| UVEC4			{ $$ = (char *)"uvec4"; }
> +	| MAT2X2		{ $$ = (char *)"mat2"; }
> +	| MAT2X3		{ $$ = (char *)"mat2x3"; }
> +	| MAT2X4		{ $$ = (char *)"mat2x4"; }
> +	| MAT3X2		{ $$ = (char *)"mat3x2"; }
> +	| MAT3X3		{ $$ = (char *)"mat3"; }
> +	| MAT3X4		{ $$ = (char *)"mat3x4"; }
> +	| MAT4X2		{ $$ = (char *)"mat4x2"; }
> +	| MAT4X3		{ $$ = (char *)"mat4x3"; }
> +	| MAT4X4		{ $$ = (char *)"mat4"; }
> +	| SAMPLER1D		{ $$ = (char *)"sampler1D"; }
> +	| SAMPLER2D		{ $$ = (char *)"sampler2D"; }
> +	| SAMPLER2DRECT		{ $$ = (char *)"sampler2DRect"; }
> +	| SAMPLER3D		{ $$ = (char *)"sampler3D"; }
> +	| SAMPLERCUBE		{ $$ = (char *)"samplerCube"; }
> +	| SAMPLEREXTERNALOES	{ $$ = (char *)"samplerExternalOES"; }
> +	| SAMPLER1DSHADOW	{ $$ = (char *)"sampler1DShadow"; }
> +	| SAMPLER2DSHADOW	{ $$ = (char *)"sampler2DShadow"; }
> +	| SAMPLER2DRECTSHADOW	{ $$ = (char *)"sampler2DRectShadow"; }
> +	| SAMPLERCUBESHADOW	{ $$ = (char *)"samplerCubeShadow"; }
> +	| SAMPLER1DARRAY	{ $$ = (char *)"sampler1DArray"; }
> +	| SAMPLER2DARRAY	{ $$ = (char *)"sampler2DArray"; }
> +	| SAMPLER1DARRAYSHADOW	{ $$ = (char *)"sampler1DArrayShadow"; }
> +	| SAMPLER2DARRAYSHADOW	{ $$ = (char *)"sampler2DArrayShadow"; }
> +	| ISAMPLER1D		{ $$ = (char *)"isampler1D"; }
> +	| ISAMPLER2D		{ $$ = (char *)"isampler2D"; }
> +	| ISAMPLER3D		{ $$ = (char *)"isampler3D"; }
> +	| ISAMPLERCUBE		{ $$ = (char *)"isamplerCube"; }
> +	| ISAMPLER1DARRAY	{ $$ = (char *)"isampler1DArray"; }
> +	| ISAMPLER2DARRAY	{ $$ = (char *)"isampler2DArray"; }
> +	| USAMPLER1D		{ $$ = (char *)"usampler1D"; }
> +	| USAMPLER2D		{ $$ = (char *)"usampler2D"; }
> +	| USAMPLER3D		{ $$ = (char *)"usampler3D"; }
> +	| USAMPLERCUBE		{ $$ = (char *)"usamplerCube"; }
> +	| USAMPLER1DARRAY	{ $$ = (char *)"usampler1DArray"; }
> +	| USAMPLER2DARRAY	{ $$ = (char *)"usampler2DArray"; }
>   	;
>
>   precision_qualifier:

I am a fan of this patch---it both simplifies and clarifies.  However, 
casting string literals to (char *) makes me cringe.  There's a reason 
that warning exists.

On the plus side, it looks like there's no reason we actually need to 
use (char *)...we can just const the strings in the AST structures and 
everything works out nicely.  I've made a patch to do that, and will 
send it out as soon as I verify it passes Piglit.


More information about the mesa-dev mailing list