[Mesa-dev] [PATCH] glcpp: Avoid unnecessary call to strlen
Timothy Arceri
tarceri at itsqueeze.com
Thu Aug 31 00:20:32 UTC 2017
On 31/08/17 06:19, 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().
>
> 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.
Thanks for the updated patch. Unfortunately I didn't see the same
improvements in shader-db:
master:
$ time ./run -j 1 shaders > new-run 2> /dev/null
real 9m22.023s
user 9m15.424s
sys 0m3.487s
master with this patch:
$ time ./run -j 1 shaders > new-run 2> /dev/null
real 9m21.611s
user 9m15.682s
sys 0m3.339s
> ---
> 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;
>
>
More information about the mesa-dev
mailing list