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

Nicolai Hähnle nhaehnle at gmail.com
Wed Sep 6 08:49:35 UTC 2017


On 29.08.2017 21:56, Thomas Helland wrote:
> V2: Pointed out by Timothy
>     - Fix pp.c reralloc size issue and comment
> 
> V3 - Use vprintf instead of printf where we should
>     - Fixes failing make-check tests
> ---
>   src/compiler/glsl/glcpp/glcpp-parse.y | 195 ++++++++++------------------------
>   src/compiler/glsl/glcpp/glcpp.h       |   8 +-
>   src/compiler/glsl/glcpp/pp.c          |  39 +++----
>   3 files changed, 78 insertions(+), 164 deletions(-)
> 
> diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y b/src/compiler/glsl/glcpp/glcpp-parse.y
> index 898a26044f..debf12bef8 100644
> --- a/src/compiler/glsl/glcpp/glcpp-parse.y
> +++ b/src/compiler/glsl/glcpp/glcpp-parse.y
> @@ -209,12 +209,7 @@ line:
>   |	SPACE control_line
>   |	text_line {
>   		_glcpp_parser_print_expanded_token_list (parser, $1);
> -		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;
> +		_mesa_string_buffer_append_char(parser->output, '\n');
>   	}
>   |	expanded_line
>   ;
> @@ -233,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);
>   	}
>   ;
>   
> @@ -264,12 +255,7 @@ define:
>   
>   control_line:
>   	control_line_success {
> -		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;
> +		_mesa_string_buffer_append_char(parser->output, '\n');
>   	}
>   |	control_line_error
>   |	HASH_TOKEN LINE pp_tokens NEWLINE {
> @@ -459,7 +445,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);
>   	}
>   ;
>   
> @@ -1137,133 +1123,61 @@ _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) {
> -      size_t size = sizeof(char);
> -
> -      ralloc_str_append(out, (char *) &token->type, *len, size);
> -      *len += size;
> +      _mesa_string_buffer_printf(out,"%c", token->type);

_mesa_string_buffer_append_char?


>         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: {
> -      size_t size = strlen(token->value.str);
> -
> -      ralloc_str_append(out, token->value.str, *len, size);
> -      *len += size;
> +   case OTHER:
> +      _mesa_string_buffer_append(out, token->value.str);
>         break;
> -   }
> -   case SPACE: {
> -      const char *token_str = " ";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case SPACE:
> +      _mesa_string_buffer_append(out, " ");

_mesa_string_buffer_append_char?

Apart from those:

Reviewed-by: Nicolai Hähnle <nicolai.haehnle at amd.com>


>         break;
> -   }
> -   case LEFT_SHIFT: {
> -      const char *token_str = "<<";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case LEFT_SHIFT:
> +      _mesa_string_buffer_append(out, "<<");
>         break;
> -   }
> -   case RIGHT_SHIFT: {
> -      const char *token_str = ">>";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case RIGHT_SHIFT:
> +      _mesa_string_buffer_append(out, ">>");
>         break;
> -   }
> -   case LESS_OR_EQUAL: {
> -      const char *token_str = "<=";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case LESS_OR_EQUAL:
> +      _mesa_string_buffer_append(out, "<=");
>         break;
> -   }
> -   case GREATER_OR_EQUAL: {
> -      const char *token_str = ">=";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case GREATER_OR_EQUAL:
> +      _mesa_string_buffer_append(out, ">=");
>         break;
> -   }
> -   case EQUAL: {
> -      const char *token_str = "==";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case EQUAL:
> +      _mesa_string_buffer_append(out, "==");
>         break;
> -   }
> -   case NOT_EQUAL: {
> -      const char *token_str = "!=";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case NOT_EQUAL:
> +      _mesa_string_buffer_append(out, "!=");
>         break;
> -   }
> -   case AND: {
> -      const char *token_str = "&&";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case AND:
> +      _mesa_string_buffer_append(out, "&&");
>         break;
> -   }
> -   case OR: {
> -      const char *token_str = "||";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case OR:
> +      _mesa_string_buffer_append(out, "||");
>         break;
> -   }
> -   case PASTE: {
> -      const char *token_str = "##";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case PASTE:
> +      _mesa_string_buffer_append(out, "##");
>         break;
> -   }
> -   case PLUS_PLUS: {
> -      const char *token_str = "++";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case PLUS_PLUS:
> +      _mesa_string_buffer_append(out, "++");
>         break;
> -   }
> -   case MINUS_MINUS: {
> -      const char *token_str = "--";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case MINUS_MINUS:
> +      _mesa_string_buffer_append(out, "--");
>         break;
> -   }
> -   case DEFINED: {
> -      const char *token_str = "defined";
> -      size_t size = strlen(token_str);
> -
> -      ralloc_str_append(out, token_str, *len, size);
> -      *len += size;
> +   case DEFINED:
> +      _mesa_string_buffer_append(out, "defined");
>         break;
> -   }
>      case PLACEHOLDER:
>         /* Nothing to print. */
>         break;
> @@ -1389,11 +1303,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;
>   }
> @@ -1407,7 +1321,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
> @@ -1429,6 +1343,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)
> @@ -1459,10 +1378,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;
> @@ -2453,10 +2372,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..40e0f033d2 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_vprintf(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_vprintf(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);
> 


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.


More information about the mesa-dev mailing list