[Mesa-dev] [PATCH 8/8] glsl: use ralloc_str_append() rather than ralloc_asprintf_rewrite_tail()

Thomas Helland thomashelland90 at gmail.com
Tue Aug 8 20:56:41 UTC 2017


2017-08-07 2:18 GMT+00:00 Timothy Arceri <tarceri at itsqueeze.com>:
> The Deus Ex: Mankind Divided shaders go from spending ~20 seconds
> in the GLSL IR compilers front-end down to ~18.5 seconds on a
> Ryzen 1800X.
>
> Tested by compiling once with shader-db then deleting the index file
> from the shader cache and compiling again.

The changes themselves look OK, but you are changing the append
function that was added in the previous patch. Rebase failure?
I'd like to see the changes here rebased onto the previous patch.

I think that should be everything except for patch 6 and 7.
I'll try to look into those more carefully tomorrow, as they're a bit tricky.

> ---
>  src/compiler/glsl/glcpp/glcpp-parse.y | 144 ++++++++++++++++++++++++++--------
>  src/util/ralloc.c                     |  12 +--
>  src/util/ralloc.h                     |   4 +-
>  3 files changed, 122 insertions(+), 38 deletions(-)
>
> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y b/src/compiler/glsl/glcpp/glcpp-parse.y
> index f1719f90b1..898a26044f 100644
> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
> @@ -202,21 +202,26 @@ add_builtin_define(glcpp_parser_t *parser, const char *name, int value);
>  input:
>         /* empty */
>  |      input line
>  ;
>
>  line:
>         control_line
>  |      SPACE control_line
>  |      text_line {
>                 _glcpp_parser_print_expanded_token_list (parser, $1);
> -               ralloc_asprintf_rewrite_tail (&parser->output, &parser->output_length, "\n");
> +               const char *newline_str = "\n";
> +               size_t size = strlen(newline_str);
> +
> +               ralloc_str_append(&parser->output, newline_str,
> +                                 parser->output_length, size);
> +               parser->output_length += size;
>         }
>  |      expanded_line
>  ;
>
>  expanded_line:
>         IF_EXPANDED expression NEWLINE {
>                 if (parser->is_gles && $2.undefined_macro)
>                         glcpp_error(& @1, parser, "undefined macro %s in expression (illegal in GLES)", $2.undefined_macro);
>                 _glcpp_parser_skip_stack_push_if (parser, & @1, $2.value);
>         }
> @@ -252,21 +257,26 @@ define:
>  |      FUNC_IDENTIFIER '(' ')' replacement_list NEWLINE {
>                 _define_function_macro (parser, & @1, $1, NULL, $4);
>         }
>  |      FUNC_IDENTIFIER '(' identifier_list ')' replacement_list NEWLINE {
>                 _define_function_macro (parser, & @1, $1, $3, $5);
>         }
>  ;
>
>  control_line:
>         control_line_success {
> -               ralloc_asprintf_rewrite_tail (&parser->output, &parser->output_length, "\n");
> +               const char *newline_str = "\n";
> +               size_t size = strlen(newline_str);
> +
> +               ralloc_str_append(&parser->output, newline_str,
> +                                 parser->output_length, size);
> +               parser->output_length += size;
>         }
>  |      control_line_error
>  |      HASH_TOKEN LINE pp_tokens NEWLINE {
>
>                 if (parser->skip_stack == NULL ||
>                     parser->skip_stack->type == SKIP_NO_SKIP)
>                 {
>                         _glcpp_parser_expand_and_lex_from (parser,
>                                                            LINE_EXPANDED, $3,
>                                                            EXPANSION_MODE_IGNORE_DEFINED);
> @@ -1123,72 +1133,144 @@ _token_list_equal_ignoring_space(token_list_t *a, token_list_t *b)
>        node_b = node_b->next;
>     }
>
>     return 1;
>  }
>
>  static void
>  _token_print(char **out, size_t *len, token_t *token)
>  {
>     if (token->type < 256) {
> -      ralloc_asprintf_rewrite_tail (out, len, "%c", token->type);
> +      size_t size = sizeof(char);
> +
> +      ralloc_str_append(out, (char *) &token->type, *len, size);
> +      *len += size;
>        return;
>     }
>
>     switch (token->type) {
>     case INTEGER:
>        ralloc_asprintf_rewrite_tail (out, len, "%" PRIiMAX, token->value.ival);
>        break;
>     case IDENTIFIER:
>     case INTEGER_STRING:
> -   case OTHER:
> -      ralloc_asprintf_rewrite_tail (out, len, "%s", token->value.str);
> +   case OTHER: {
> +      size_t size = strlen(token->value.str);
> +
> +      ralloc_str_append(out, token->value.str, *len, size);
> +      *len += size;
>        break;
> -   case SPACE:
> -      ralloc_asprintf_rewrite_tail (out, len, " ");
> +   }
> +   case SPACE: {
> +      const char *token_str = " ";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case LEFT_SHIFT:
> -      ralloc_asprintf_rewrite_tail (out, len, "<<");
> +   }
> +   case LEFT_SHIFT: {
> +      const char *token_str = "<<";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case RIGHT_SHIFT:
> -      ralloc_asprintf_rewrite_tail (out, len, ">>");
> +   }
> +   case RIGHT_SHIFT: {
> +      const char *token_str = ">>";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case LESS_OR_EQUAL:
> -      ralloc_asprintf_rewrite_tail (out, len, "<=");
> +   }
> +   case LESS_OR_EQUAL: {
> +      const char *token_str = "<=";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case GREATER_OR_EQUAL:
> -      ralloc_asprintf_rewrite_tail (out, len, ">=");
> +   }
> +   case GREATER_OR_EQUAL: {
> +      const char *token_str = ">=";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case EQUAL:
> -      ralloc_asprintf_rewrite_tail (out, len, "==");
> +   }
> +   case EQUAL: {
> +      const char *token_str = "==";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case NOT_EQUAL:
> -      ralloc_asprintf_rewrite_tail (out, len, "!=");
> +   }
> +   case NOT_EQUAL: {
> +      const char *token_str = "!=";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case AND:
> -      ralloc_asprintf_rewrite_tail (out, len, "&&");
> +   }
> +   case AND: {
> +      const char *token_str = "&&";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case OR:
> -      ralloc_asprintf_rewrite_tail (out, len, "||");
> +   }
> +   case OR: {
> +      const char *token_str = "||";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case PASTE:
> -      ralloc_asprintf_rewrite_tail (out, len, "##");
> +   }
> +   case PASTE: {
> +      const char *token_str = "##";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case PLUS_PLUS:
> -      ralloc_asprintf_rewrite_tail (out, len, "++");
> +   }
> +   case PLUS_PLUS: {
> +      const char *token_str = "++";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case MINUS_MINUS:
> -      ralloc_asprintf_rewrite_tail (out, len, "--");
> +   }
> +   case MINUS_MINUS: {
> +      const char *token_str = "--";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> -   case DEFINED:
> -      ralloc_asprintf_rewrite_tail (out, len, "defined");
> +   }
> +   case DEFINED: {
> +      const char *token_str = "defined";
> +      size_t size = strlen(token_str);
> +
> +      ralloc_str_append(out, token_str, *len, size);
> +      *len += size;
>        break;
> +   }
>     case PLACEHOLDER:
>        /* Nothing to print. */
>        break;
>     default:
>        assert(!"Error: Don't know how to print token.");
>
>        break;
>     }
>  }
>
> diff --git a/src/util/ralloc.c b/src/util/ralloc.c
> index b2b0315e65..bf46439df4 100644
> --- a/src/util/ralloc.c
> +++ b/src/util/ralloc.c
> @@ -397,34 +397,36 @@ ralloc_strcat(char **dest, const char *str)
>     return cat(dest, str, strlen(str));
>  }
>
>  bool
>  ralloc_strncat(char **dest, const char *str, size_t n)
>  {
>     return cat(dest, str, strnlen(str, n));
>  }
>
>  bool
> -ralloc_str_append(char **dest, size_t existing_length, const char *str,
> -                  size_t n)
> +ralloc_str_append(char **dest, const char *str,
> +                  size_t existing_length, size_t str_size)
>  {
>     char *both;
>     assert(dest != NULL && *dest != NULL);
>
> -   both = resize(*dest, existing_length + n + 1);
> +   both = resize(*dest, existing_length + str_size + 1);
>     if (unlikely(both == NULL))
>        return false;
>
> -   memcpy(both + existing_length, str, n);
> -   both[existing_length + n] = '\0';
> +   memcpy(both + existing_length, str, str_size);
> +   both[existing_length + str_size] = '\0';
>
>     *dest = both;
> +
> +   return true;
>  }
>
>  char *
>  ralloc_asprintf(const void *ctx, const char *fmt, ...)
>  {
>     char *ptr;
>     va_list args;
>     va_start(args, fmt);
>     ptr = ralloc_vasprintf(ctx, fmt, args);
>     va_end(args);
> diff --git a/src/util/ralloc.h b/src/util/ralloc.h
> index f2d3d3671e..05ae8f8407 100644
> --- a/src/util/ralloc.h
> +++ b/src/util/ralloc.h
> @@ -300,22 +300,22 @@ bool ralloc_strncat(char **dest, const char *str, size_t n);
>   * new pointer unless allocation fails.
>   *
>   * The result will always be null-terminated.
>   *
>   * This function differs from ralloc_strcat() and ralloc_strncat() in that it
>   * does not do any strlen() calls which can become costly on large strings.
>   *
>   * \return True unless allocation failed.
>   */
>  bool
> -ralloc_str_append(char **dest, size_t existing_length, const char *str,
> -                  size_t n);
> +ralloc_str_append(char **dest, const char *str,
> +                  size_t existing_length, size_t str_size);
>
>  /**
>   * Print to a string.
>   *
>   * This is analogous to \c sprintf, but allocates enough space (using \p ctx
>   * as the context) for the resulting string.
>   *
>   * \return The newly allocated string.
>   */
>  char *ralloc_asprintf (const void *ctx, const char *fmt, ...) PRINTFLIKE(2, 3) MALLOCLIKE;
> --
> 2.13.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list