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

[Thomas Helland]

I've now pushed the series, so feel free to rebase =)

2017-09-22 16:10 GMT+02:00 Ian Romanick <idr at freedesktop.org>:
> 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);
>>                       }
>>
>