[Mesa-dev] [PATCH] glsl: Change the parser to use the string buffer

Thomas Helland thomashelland90 at gmail.com
Sun May 28 15:03:08 UTC 2017


2017-05-28 15:19 GMT+02:00 Thomas Helland <thomashelland90 at gmail.com>:
> V2: Failure found by Timothy running it through CI
>    - Fix pp.c reralloc size issue and comment

There is still something here that doesn't work as intended.
Some tests are failing  I'll report once I find the cause.

> ---
>  src/compiler/glsl/glcpp/glcpp-parse.y | 85 ++++++++++++++++++-----------------
>  src/compiler/glsl/glcpp/glcpp.h       |  8 ++--
>  src/compiler/glsl/glcpp/pp.c          | 39 +++++++---------
>  3 files changed, 64 insertions(+), 68 deletions(-)
>
> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y b/src/compiler/glsl/glcpp/glcpp-parse.y
> index fe211a0f0b..e84b6fb055 100644
> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
> @@ -209,7 +209,7 @@ line:
>  |      SPACE control_line
>  |      text_line {
>                 _glcpp_parser_print_expanded_token_list (parser, $1);
> -               ralloc_asprintf_rewrite_tail (&parser->output, &parser->output_length, "\n");
> +               _mesa_string_buffer_append_char(parser->output, '\n');
>         }
>  |      expanded_line
>  ;
> @@ -228,20 +228,16 @@ expanded_line:
>  |      LINE_EXPANDED integer_constant NEWLINE {
>                 parser->has_new_line_number = 1;
>                 parser->new_line_number = $2;
> -               ralloc_asprintf_rewrite_tail (&parser->output,
> -                                             &parser->output_length,
> -                                             "#line %" PRIiMAX "\n",
> -                                             $2);
> +               _mesa_string_buffer_printf(parser->output, "#line %" PRIiMAX "\n", $2);
>         }
>  |      LINE_EXPANDED integer_constant integer_constant NEWLINE {
>                 parser->has_new_line_number = 1;
>                 parser->new_line_number = $2;
>                 parser->has_new_source_number = 1;
>                 parser->new_source_number = $3;
> -               ralloc_asprintf_rewrite_tail (&parser->output,
> -                                             &parser->output_length,
> -                                             "#line %" PRIiMAX " %" PRIiMAX "\n",
> -                                             $2, $3);
> +               _mesa_string_buffer_printf(parser->output,
> +                                          "#line %" PRIiMAX " %" PRIiMAX "\n",
> +                                           $2, $3);
>         }
>  ;
>
> @@ -259,7 +255,7 @@ define:
>
>  control_line:
>         control_line_success {
> -               ralloc_asprintf_rewrite_tail (&parser->output, &parser->output_length, "\n");
> +               _mesa_string_buffer_append_char(parser->output, '\n');
>         }
>  |      control_line_error
>  |      HASH_TOKEN LINE pp_tokens NEWLINE {
> @@ -432,7 +428,7 @@ control_line_success:
>                 glcpp_parser_resolve_implicit_version(parser);
>         }
>  |      HASH_TOKEN PRAGMA NEWLINE {
> -               ralloc_asprintf_rewrite_tail (&parser->output, &parser->output_length, "#%s", $2);
> +               _mesa_string_buffer_printf(parser->output, "#%s", $2);
>         }
>  ;
>
> @@ -1110,60 +1106,60 @@ _token_list_equal_ignoring_space(token_list_t *a, token_list_t *b)
>  }
>
>  static void
> -_token_print(char **out, size_t *len, token_t *token)
> +_token_print(struct _mesa_string_buffer *out, token_t *token)
>  {
>     if (token->type < 256) {
> -      ralloc_asprintf_rewrite_tail (out, len, "%c", token->type);
> +      _mesa_string_buffer_printf(out,"%c", token->type);
>        return;
>     }
>
>     switch (token->type) {
>     case INTEGER:
> -      ralloc_asprintf_rewrite_tail (out, len, "%" PRIiMAX, token->value.ival);
> +      _mesa_string_buffer_printf(out, "%" PRIiMAX, token->value.ival);
>        break;
>     case IDENTIFIER:
>     case INTEGER_STRING:
>     case OTHER:
> -      ralloc_asprintf_rewrite_tail (out, len, "%s", token->value.str);
> +      _mesa_string_buffer_append(out, token->value.str);
>        break;
>     case SPACE:
> -      ralloc_asprintf_rewrite_tail (out, len, " ");
> +      _mesa_string_buffer_append(out, " ");
>        break;
>     case LEFT_SHIFT:
> -      ralloc_asprintf_rewrite_tail (out, len, "<<");
> +      _mesa_string_buffer_append(out, "<<");
>        break;
>     case RIGHT_SHIFT:
> -      ralloc_asprintf_rewrite_tail (out, len, ">>");
> +      _mesa_string_buffer_append(out, ">>");
>        break;
>     case LESS_OR_EQUAL:
> -      ralloc_asprintf_rewrite_tail (out, len, "<=");
> +      _mesa_string_buffer_append(out, "<=");
>        break;
>     case GREATER_OR_EQUAL:
> -      ralloc_asprintf_rewrite_tail (out, len, ">=");
> +      _mesa_string_buffer_append(out, ">=");
>        break;
>     case EQUAL:
> -      ralloc_asprintf_rewrite_tail (out, len, "==");
> +      _mesa_string_buffer_append(out, "==");
>        break;
>     case NOT_EQUAL:
> -      ralloc_asprintf_rewrite_tail (out, len, "!=");
> +      _mesa_string_buffer_append(out, "!=");
>        break;
>     case AND:
> -      ralloc_asprintf_rewrite_tail (out, len, "&&");
> +      _mesa_string_buffer_append(out, "&&");
>        break;
>     case OR:
> -      ralloc_asprintf_rewrite_tail (out, len, "||");
> +      _mesa_string_buffer_append(out, "||");
>        break;
>     case PASTE:
> -      ralloc_asprintf_rewrite_tail (out, len, "##");
> +      _mesa_string_buffer_append(out, "##");
>        break;
>     case PLUS_PLUS:
> -      ralloc_asprintf_rewrite_tail (out, len, "++");
> +      _mesa_string_buffer_append(out, "++");
>        break;
>     case MINUS_MINUS:
> -      ralloc_asprintf_rewrite_tail (out, len, "--");
> +      _mesa_string_buffer_append(out, "--");
>        break;
>     case DEFINED:
> -      ralloc_asprintf_rewrite_tail (out, len, "defined");
> +      _mesa_string_buffer_append(out, "defined");
>        break;
>     case PLACEHOLDER:
>        /* Nothing to print. */
> @@ -1290,11 +1286,11 @@ _token_paste(glcpp_parser_t *parser, token_t *token, token_t *other)
>
>      FAIL:
>     glcpp_error (&token->location, parser, "");
> -   ralloc_asprintf_rewrite_tail (&parser->info_log, &parser->info_log_length, "Pasting \"");
> -   _token_print (&parser->info_log, &parser->info_log_length, token);
> -   ralloc_asprintf_rewrite_tail (&parser->info_log, &parser->info_log_length, "\" and \"");
> -   _token_print (&parser->info_log, &parser->info_log_length, other);
> -   ralloc_asprintf_rewrite_tail (&parser->info_log, &parser->info_log_length, "\" does not give a valid preprocessing token.\n");
> +   _mesa_string_buffer_append(parser->info_log, "Pasting \"");
> +   _token_print(parser->info_log, token);
> +   _mesa_string_buffer_append(parser->info_log, "\" and \"");
> +   _token_print(parser->info_log, other);
> +   _mesa_string_buffer_append(parser->info_log, "\" does not give a valid preprocessing token.\n");
>
>     return token;
>  }
> @@ -1308,7 +1304,7 @@ _token_list_print(glcpp_parser_t *parser, token_list_t *list)
>        return;
>
>     for (node = list->head; node; node = node->next)
> -      _token_print (&parser->output, &parser->output_length, node->token);
> +      _token_print(parser->output, node->token);
>  }
>
>  void
> @@ -1330,6 +1326,11 @@ add_builtin_define(glcpp_parser_t *parser, const char *name, int value)
>     _define_object_macro(parser, NULL, name, list);
>  }
>
> +/* Initial output buffer size, 4096 minus ralloc() overhead. It was selected
> + * to minimize total amount of allocated memory during shader-db run.
> + */
> +#define INITIAL_PP_OUTPUT_BUF_SIZE 4048
> +
>  glcpp_parser_t *
>  glcpp_parser_create(const struct gl_extensions *extension_list,
>                      glcpp_extension_iterator extensions, void *state, gl_api api)
> @@ -1360,10 +1361,10 @@ glcpp_parser_create(const struct gl_extensions *extension_list,
>     parser->lex_from_list = NULL;
>     parser->lex_from_node = NULL;
>
> -   parser->output = ralloc_strdup(parser, "");
> -   parser->output_length = 0;
> -   parser->info_log = ralloc_strdup(parser, "");
> -   parser->info_log_length = 0;
> +   parser->output = _mesa_string_buffer_create(parser,
> +                                               INITIAL_PP_OUTPUT_BUF_SIZE);
> +   parser->info_log = _mesa_string_buffer_create(parser,
> +                                                 INITIAL_PP_OUTPUT_BUF_SIZE);
>     parser->error = 0;
>
>     parser->extensions = extensions;
> @@ -2354,10 +2355,10 @@ _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t versio
>     }
>
>     if (explicitly_set) {
> -      ralloc_asprintf_rewrite_tail(&parser->output, &parser->output_length,
> -                                   "#version %" PRIiMAX "%s%s", version,
> -                                   es_identifier ? " " : "",
> -                                   es_identifier ? es_identifier : "");
> +      _mesa_string_buffer_printf(parser->output,
> +                                 "#version %" PRIiMAX "%s%s", version,
> +                                 es_identifier ? " " : "",
> +                                 es_identifier ? es_identifier : "");
>     }
>  }
>
> diff --git a/src/compiler/glsl/glcpp/glcpp.h b/src/compiler/glsl/glcpp/glcpp.h
> index 2804636c30..9d997cd924 100644
> --- a/src/compiler/glsl/glcpp/glcpp.h
> +++ b/src/compiler/glsl/glcpp/glcpp.h
> @@ -33,6 +33,8 @@
>
>  #include "util/hash_table.h"
>
> +#include "util/string_buffer.h"
> +
>  #define yyscan_t void*
>
>  /* Some data types used for parser values. */
> @@ -199,10 +201,8 @@ struct glcpp_parser {
>         int skipping;
>         token_list_t *lex_from_list;
>         token_node_t *lex_from_node;
> -       char *output;
> -       char *info_log;
> -       size_t output_length;
> -       size_t info_log_length;
> +       struct _mesa_string_buffer *output;
> +       struct _mesa_string_buffer *info_log;
>         int error;
>         glcpp_extension_iterator extensions;
>         const struct gl_extensions *extension_list;
> diff --git a/src/compiler/glsl/glcpp/pp.c b/src/compiler/glsl/glcpp/pp.c
> index 96125f2e2f..62d895328e 100644
> --- a/src/compiler/glsl/glcpp/pp.c
> +++ b/src/compiler/glsl/glcpp/pp.c
> @@ -32,20 +32,16 @@ glcpp_error (YYLTYPE *locp, glcpp_parser_t *parser, const char *fmt, ...)
>         va_list ap;
>
>         parser->error = 1;
> -       ralloc_asprintf_rewrite_tail(&parser->info_log,
> -                                    &parser->info_log_length,
> -                                    "%u:%u(%u): "
> -                                    "preprocessor error: ",
> -                                    locp->source,
> -                                    locp->first_line,
> -                                    locp->first_column);
> +       _mesa_string_buffer_printf(parser->info_log,
> +                                  "%u:%u(%u): "
> +                                  "preprocessor error: ",
> +                                  locp->source,
> +                                  locp->first_line,
> +                                  locp->first_column);
>         va_start(ap, fmt);
> -       ralloc_vasprintf_rewrite_tail(&parser->info_log,
> -                                     &parser->info_log_length,
> -                                     fmt, ap);
> +       _mesa_string_buffer_printf(parser->info_log, fmt, ap);
>         va_end(ap);
> -       ralloc_asprintf_rewrite_tail(&parser->info_log,
> -                                    &parser->info_log_length, "\n");
> +       _mesa_string_buffer_append(parser->info_log, "\n");
>  }
>
>  void
> @@ -53,20 +49,16 @@ glcpp_warning (YYLTYPE *locp, glcpp_parser_t *parser, const char *fmt, ...)
>  {
>         va_list ap;
>
> -       ralloc_asprintf_rewrite_tail(&parser->info_log,
> -                                    &parser->info_log_length,
> +       _mesa_string_buffer_printf(parser->info_log,
>                                      "%u:%u(%u): "
>                                      "preprocessor warning: ",
>                                      locp->source,
>                                      locp->first_line,
>                                      locp->first_column);
>         va_start(ap, fmt);
> -       ralloc_vasprintf_rewrite_tail(&parser->info_log,
> -                                     &parser->info_log_length,
> -                                     fmt, ap);
> +       _mesa_string_buffer_printf(parser->info_log, fmt, ap);
>         va_end(ap);
> -       ralloc_asprintf_rewrite_tail(&parser->info_log,
> -                                    &parser->info_log_length, "\n");
> +       _mesa_string_buffer_append(parser->info_log, "\n");
>  }
>
>  /* Given str, (that's expected to start with a newline terminator of some
> @@ -238,10 +230,13 @@ glcpp_preprocess(void *ralloc_ctx, const char **shader, char **info_log,
>
>         glcpp_parser_resolve_implicit_version(parser);
>
> -       ralloc_strcat(info_log, parser->info_log);
> +       ralloc_strcat(info_log, parser->info_log->buf);
> +
> +       /* Crimp the buffer first, to conserve memory */
> +       _mesa_string_buffer_crimp_to_fit(parser->output);
>
> -       ralloc_steal(ralloc_ctx, parser->output);
> -       *shader = parser->output;
> +       ralloc_steal(ralloc_ctx, parser->output->buf);
> +       *shader = parser->output->buf;
>
>         errors = parser->error;
>         glcpp_parser_destroy (parser);
> --
> 2.13.0
>


More information about the mesa-dev mailing list