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

Thomas Helland thomashelland90 at gmail.com
Sun May 28 15:40:27 UTC 2017


V2: Pointed out by Timothy running the CI
   - 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 | 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..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);
-- 
2.13.0



More information about the mesa-dev mailing list