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

[Ian Romanick]

This patch is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

I have a couple patches that go on top of this particular patch, and I'd
rather rebase before I send them out for review. :)

On 09/14/2017 03:39 PM, Thomas Helland wrote:
> Length of the token was already calculated by flex and stored in yyleng,
> no need to implicitly call strlen() via linear_strdup().
> 
> Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>
> Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
> 
> V2: Also convert this pattern in glsl_lexer.ll
> 
> V3: Remove a misplaced comment
> 
> V4: Use a temporary char to avoid type change
>     Remove bogus +1 on length check of identifier
> ---
>  src/compiler/glsl/glcpp/glcpp-lex.l |  9 ++++++++-
>  src/compiler/glsl/glsl_lexer.ll     | 39 +++++++++++++++++++++++++++++--------
>  2 files changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/src/compiler/glsl/glcpp/glcpp-lex.l b/src/compiler/glsl/glcpp/glcpp-lex.l
> index 381b97364a..9cfcc12022 100644
> --- a/src/compiler/glsl/glcpp/glcpp-lex.l
> +++ b/src/compiler/glsl/glcpp/glcpp-lex.l
> @@ -101,7 +101,14 @@ void glcpp_set_column (int  column_no , yyscan_t yyscanner);
>  #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 7c41455d98..56519bf92d 100644
> --- a/src/compiler/glsl/glsl_lexer.ll
> +++ b/src/compiler/glsl/glsl_lexer.ll
> @@ -81,8 +81,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;				\
> +         char *id = (char *) linear_alloc_child(mem_ctx, yyleng + 1);   \
> +         memcpy(id, yytext, yyleng + 1);                                \
> +         yylval->identifier = id;                                       \
>  	 return classify_identifier(yyextra, yytext);			\
>        }									\
>     } while (0)
> @@ -261,8 +266,14 @@ HASH		^{SPC}#{SPC}
>  <PP>[ \t\r]*			{ }
>  <PP>:				return COLON;
>  <PP>[_a-zA-Z][_a-zA-Z0-9]*	{
> -				   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;
> +                                    char *id = (char *) linear_alloc_child(mem_ctx, yyleng + 1);
> +                                    memcpy(id, yytext, yyleng + 1);
> +                                    yylval->identifier = id;
>  				   return IDENTIFIER;
>  				}
>  <PP>[1-9][0-9]*			{
> @@ -449,8 +460,14 @@ layout		{
>                        || yyextra->ARB_tessellation_shader_enable) {
>  		      return LAYOUT_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;
> +                      char *id = (char *) linear_alloc_child(mem_ctx, yyleng + 1);
> +                      memcpy(id, yytext, yyleng + 1);
> +                      yylval->identifier = id;
>  		      return classify_identifier(yyextra, yytext);
>  		   }
>  		}
> @@ -621,12 +638,18 @@ 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 > 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
> +			       */
> +                              char *id = (char *) linear_alloc_child(ctx, yyleng + 1);
> +                              memcpy(id, yytext, yyleng + 1);
> +                              yylval->identifier = id;
>  			    }
>  			    return classify_identifier(state, yytext);
>  			}
>