[Mesa-dev] [PATCH] glsl: Drop the round-trip through ast_type_specifier for many builtin types.
Ian Romanick
idr at freedesktop.org
Thu Mar 29 18:16:35 PDT 2012
On 03/29/2012 02:59 PM, Matt Turner wrote:
> On Thu, Mar 29, 2012 at 5:37 PM, Eric Anholt<eric at anholt.net> 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:
>> --
>> 1.7.9.5
>
> I just came across this code in the parser a couple of days ago and
> couldn't figure out why we were doing backflips to get the string. I
> like this patch, but can't we go one step farther and return the
> string directly from the lexer? Looking at the KEYWORD macro, we
> already return the string when it's not identified as a keyword or
> reserved word. Could we just set yylval->identifier in all cases and
> use it directly in the parser?
We could, but we'd need to make sure the string was freed at the right
time. Using a static string eliminates the need.
More information about the mesa-dev
mailing list