[Mesa-dev] [PATCH] glcpp: Avoid unnecessary call to strlen

Thomas Helland thomashelland90 at gmail.com
Wed Aug 30 20:19:31 UTC 2017


Length of the token was already calculated by flex and stored in yyleng,
no need to implicitly call strlen() via linear_strdup().

V2: Also convert this pattern in glsl_lexer.ll
    This has a surprising result of reducing executed cyles from
    1.06 trillion to 1.03 trillion, a significant change.
    Perf reports an increase in instructions per cycle from
    1,04 as a normal value, to 1,07. I'm also seeing a decent
    reduction in shader-db runtimes, from 113 seconds on master,
    down to 98 seconds. I'll have to do more thorough testing
    to verify if this is actually statistically significant,
    but the change in execution was drastic when adding the change
    to glsl_lexer.ll, and with the tests run as of now it looks good.
---
 src/compiler/glsl/glcpp/glcpp-lex.l | 13 ++++++++++++-
 src/compiler/glsl/glsl_lexer.ll     | 32 ++++++++++++++++++++++++++------
 src/compiler/glsl/glsl_parser.yy    |  2 +-
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l b/src/compiler/glsl/glcpp/glcpp-lex.l
index 381b97364a..a1fbcaa928 100644
--- a/src/compiler/glsl/glcpp/glcpp-lex.l
+++ b/src/compiler/glsl/glcpp/glcpp-lex.l
@@ -98,10 +98,21 @@ void glcpp_set_column (int  column_no , yyscan_t yyscanner);
 		}							\
 	} while(0)
 
+/* We're not doing linear_strdup here, to avoid an implicit call on
+ * strlen() for the length of the string, as this is already found by
+ * flex and stored in yyleng
+ */
 #define RETURN_STRING_TOKEN(token)					\
 	do {								\
 		if (! parser->skipping) {				\
-			yylval->str = linear_strdup(yyextra->linalloc, yytext);	\
+			/* We're not doing linear_strdup here, to avoid \
+			 * an implicit call on strlen() for the length  \
+			 * of the string, as this is already found by   \
+			 * flex and stored in yyleng */                 \
+			void *mem_ctx = yyextra->linalloc;		\
+			yylval->str = linear_alloc_child(mem_ctx,	\
+							 yyleng + 1);	\
+			memcpy(yylval->str, yytext, yyleng + 1);        \
 			RETURN_TOKEN_NEVER_SKIP (token);		\
 		}							\
 	} while(0)
diff --git a/src/compiler/glsl/glsl_lexer.ll b/src/compiler/glsl/glsl_lexer.ll
index d4d7ada1b1..5eceb546b9 100644
--- a/src/compiler/glsl/glsl_lexer.ll
+++ b/src/compiler/glsl/glsl_lexer.ll
@@ -80,8 +80,13 @@ static int classify_identifier(struct _mesa_glsl_parse_state *, const char *);
 			  "illegal use of reserved word `%s'", yytext);	\
 	 return ERROR_TOK;						\
       } else {								\
-	 void *mem_ctx = yyextra->linalloc;					\
-	 yylval->identifier = linear_strdup(mem_ctx, yytext);		\
+	 /* We're not doing linear_strdup here, to avoid an implicit    \
+	  * call on strlen() for the length of the string, as this is   \
+	  * already found by flex and stored in yyleng */               \
+	 void *mem_ctx = yyextra->linalloc;				\
+	 yylval->identifier = (char *) linear_alloc_child(mem_ctx,	\
+							  yyleng + 1);	\
+	 memcpy(yylval->identifier, yytext, yyleng + 1);		\
 	 return classify_identifier(yyextra, yytext);			\
       }									\
    } while (0)
@@ -260,8 +265,13 @@ HASH		^{SPC}#{SPC}
 <PP>[ \t\r]*			{ }
 <PP>:				return COLON;
 <PP>[_a-zA-Z][_a-zA-Z0-9]*	{
+				   /* We're not doing linear_strdup here, to avoid an implicit call
+				    * on strlen() for the length of the string, as this is already
+				    * found by flex and stored in yyleng
+				    */
 				   void *mem_ctx = yyextra->linalloc;
-				   yylval->identifier = linear_strdup(mem_ctx, yytext);
+				   yylval->identifier = (char *) linear_alloc_child(mem_ctx, yyleng + 1);
+				   memcpy(yylval->identifier, yytext, yyleng + 1);
 				   return IDENTIFIER;
 				}
 <PP>[1-9][0-9]*			{
@@ -448,8 +458,13 @@ layout		{
                       || yyextra->ARB_tessellation_shader_enable) {
 		      return LAYOUT_TOK;
 		   } else {
+		      /* We're not doing linear_strdup here, to avoid an implicit call
+		       * on strlen() for the length of the string, as this is already
+		       * found by flex and stored in yyleng
+		       */
 		      void *mem_ctx = yyextra->linalloc;
-		      yylval->identifier = linear_strdup(mem_ctx, yytext);
+		      yylval->identifier = (char *) linear_alloc_child(mem_ctx, yyleng + 1);
+		      memcpy(yylval->identifier, yytext, yyleng + 1);
 		      return classify_identifier(yyextra, yytext);
 		   }
 		}
@@ -620,12 +635,17 @@ u64vec4		KEYWORD_WITH_ALT(0, 0, 0, 0, yyextra->ARB_gpu_shader_int64_enable, U64V
 [_a-zA-Z][_a-zA-Z0-9]*	{
 			    struct _mesa_glsl_parse_state *state = yyextra;
 			    void *ctx = state->linalloc;
-			    if (state->es_shader && strlen(yytext) > 1024) {
+			    if (state->es_shader && yyleng + 1 > 1024) {
 			       _mesa_glsl_error(yylloc, state,
 			                        "Identifier `%s' exceeds 1024 characters",
 			                        yytext);
 			    } else {
-			      yylval->identifier = linear_strdup(ctx, yytext);
+			      /* We're not doing linear_strdup here, to avoid an implicit call
+			       * on strlen() for the length of the string, as this is already
+			       * found by flex and stored in yyleng
+			       */
+			      yylval->identifier = (char *) linear_alloc_child(ctx, yyleng + 1);
+			      memcpy(yylval->identifier, yytext, yyleng + 1);
 			    }
 			    return classify_identifier(state, yytext);
 			}
diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
index 7b93d34fa3..30b04beb4d 100644
--- a/src/compiler/glsl/glsl_parser.yy
+++ b/src/compiler/glsl/glsl_parser.yy
@@ -100,7 +100,7 @@ static bool match_layout_qualifier(const char *s1, const char *s2,
    int64_t n64;
    float real;
    double dreal;
-   const char *identifier;
+   char *identifier;
 
    struct ast_type_qualifier type_qualifier;
 
-- 
2.13.3



More information about the mesa-dev mailing list