[Mesa-dev] [PATCH 1/2] Allow for GLSL shaders to have #extension directive anywhere in source file.
Ian Romanick
idr at freedesktop.org
Tue Dec 3 08:43:47 PST 2013
Can we please hold off on this? I'm in the process of getting the GLSL
spec updated to match what people actually ship. We've always taken the
approach of following the spec, and if the spec doesn't match reality,
fix the spec. I've done this about a dozen times over the last 3 years,
and I don't feel like there's a compelling reason to deviate for this case.
I also don't think the behavior implemented by this patch exactly
matches other vendors. I don't want to put a set of hacks in now only
to replace them with something different in a few weeks.
On 12/03/2013 12:00 AM, Kevin Rogovin wrote:
> Allow for GLSL shaders to have #extension directive
> anywhere in source file; during preprocessor, #extension
> directives are saved to a list rather than being added
> to the pre-processed string. That list is played back
> before parsing of pre-processed text takes place.
>
> ---
> src/glsl/Makefile.sources | 3 +-
> src/glsl/glcpp/glcpp-lex.l | 19 +++++-
> src/glsl/glcpp/glcpp-parse.y | 31 +++++++++-
> src/glsl/glcpp/glcpp.c | 2 +-
> src/glsl/glcpp/glcpp.h | 17 +++++-
> src/glsl/glcpp/glcpp_extras.c | 126 ++++++++++++++++++++++++++++++++++++++++
> src/glsl/glcpp/glcpp_extras.h | 97 +++++++++++++++++++++++++++++++
> src/glsl/glcpp/pp.c | 5 +-
> src/glsl/glsl_parser_extras.cpp | 55 +++++++++++++++++-
> src/glsl/glsl_parser_extras.h | 14 ++++-
> 10 files changed, 358 insertions(+), 11 deletions(-)
> create mode 100644 src/glsl/glcpp/glcpp_extras.c
> create mode 100644 src/glsl/glcpp/glcpp_extras.h
>
> diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources
> index 2e81ded..90f57ad 100644
> --- a/src/glsl/Makefile.sources
> +++ b/src/glsl/Makefile.sources
> @@ -7,7 +7,8 @@ GLSL_BUILDDIR = $(top_builddir)/src/glsl
>
> LIBGLCPP_FILES = \
> $(GLSL_SRCDIR)/ralloc.c \
> - $(GLSL_SRCDIR)/glcpp/pp.c
> + $(GLSL_SRCDIR)/glcpp/pp.c \
> + $(GLSL_SRCDIR)/glcpp/glcpp_extras.c
>
> LIBGLCPP_GENERATED_FILES = \
> $(GLSL_BUILDDIR)/glcpp/glcpp-lex.c \
> diff --git a/src/glsl/glcpp/glcpp-lex.l b/src/glsl/glcpp/glcpp-lex.l
> index a029f62..31b246f 100644
> --- a/src/glsl/glcpp/glcpp-lex.l
> +++ b/src/glsl/glcpp/glcpp-lex.l
> @@ -126,15 +126,30 @@ HEXADECIMAL_INTEGER 0[xX][0-9a-fA-F]+[uU]?
> return HASH_VERSION;
> }
>
> - /* glcpp doesn't handle #extension, #version, or #pragma directives.
> + /* glcpp doesn't handle #pragma directives.
> * Simply pass them through to the main compiler's lexer/parser. */
> -{HASH}(extension|pragma)[^\n]+ {
> +{HASH}(pragma)[^\n]+ {
> yylval->str = ralloc_strdup (yyextra, yytext);
> yylineno++;
> yycolumn = 0;
> return OTHER;
> }
>
> + /* glcpp will handle the #extension directive if
> + * and only if parser->directive_ext_list is non-NULL
> + * otherwise it will just send that off to the parser */
> +{HASH}(extension)[^\n]+ {
> + yylval->str = ralloc_strdup (yyextra, yytext);
> + yylineno++;
> + yycolumn = 0;
> + if(parser->directive_ext_list) {
> + return EXTENSION_DIRECTIVE;
> + } else {
> + return OTHER;
> + }
> +}
> +
> +
> {HASH}line{HSPACE}+ {
> return HASH_LINE;
> }
> diff --git a/src/glsl/glcpp/glcpp-parse.y b/src/glsl/glcpp/glcpp-parse.y
> index 7edc274..d1d58c3 100644
> --- a/src/glsl/glcpp/glcpp-parse.y
> +++ b/src/glsl/glcpp/glcpp-parse.y
> @@ -166,6 +166,7 @@ add_builtin_define(glcpp_parser_t *parser, const char *name, int value);
> %expect 0
> %token COMMA_FINAL DEFINED ELIF_EXPANDED HASH HASH_DEFINE FUNC_IDENTIFIER OBJ_IDENTIFIER HASH_ELIF HASH_ELSE HASH_ENDIF HASH_IF HASH_IFDEF HASH_IFNDEF HASH_LINE HASH_UNDEF HASH_VERSION IDENTIFIER IF_EXPANDED INTEGER INTEGER_STRING LINE_EXPANDED NEWLINE OTHER PLACEHOLDER SPACE
> %token PASTE
> +%token <str> EXTENSION_DIRECTIVE
> %type <ival> expression INTEGER operator SPACE integer_constant
> %type <str> IDENTIFIER FUNC_IDENTIFIER OBJ_IDENTIFIER INTEGER_STRING OTHER
> %type <string_list> identifier_list
> @@ -208,9 +209,34 @@ line:
> ralloc_free ($1);
> }
> | expanded_line
> +| extension_directive
> | HASH non_directive
> ;
>
> +extension_directive:
> + EXTENSION_DIRECTIVE {
> + int cnt;
> +
> + assert(parser->directive_ext_list);
> + /*
> + the lexer increments yylineno when it encounters
> + #extension. If it did not, then the line used by
> + the glsl parser when then be wrong. We get around
> + the issue by decrementing by number of #extension
> + processed so far.
> + */
> + cnt=parser->directive_ext_list->count;
> + add_extension_from_preprocessor($1,
> + @1.first_line - cnt,
> + @1.first_column,
> + @1.last_line - cnt,
> + @1.last_column,
> + @1.source,
> + parser->directive_ext_list);
> + }
> +;
> +
> +
> expanded_line:
> IF_EXPANDED expression NEWLINE {
> _glcpp_parser_skip_stack_push_if (parser, & @1, $2);
> @@ -1148,7 +1174,8 @@ static void add_builtin_define(glcpp_parser_t *parser,
> }
>
> glcpp_parser_t *
> -glcpp_parser_create (const struct gl_extensions *extensions, int api)
> +glcpp_parser_create (const struct gl_extensions *extensions, int api,
> + struct glcpp_extension_directive_list *ext_list)
> {
> glcpp_parser_t *parser;
> int language_version;
> @@ -1182,6 +1209,7 @@ glcpp_parser_create (const struct gl_extensions *extensions, int api)
> parser->new_source_number = 0;
>
> parser->is_gles = false;
> + parser->directive_ext_list = ext_list;
>
> /* Add pre-defined macros. */
> if (api == API_OPENGLES2) {
> @@ -2098,3 +2126,4 @@ _glcpp_parser_handle_version_declaration(glcpp_parser_t *parser, intmax_t versio
> es_identifier ? " " : "",
> es_identifier ? es_identifier : "");
> }
> +
> diff --git a/src/glsl/glcpp/glcpp.c b/src/glsl/glcpp/glcpp.c
> index 6994d7b..bc2ac3c 100644
> --- a/src/glsl/glcpp/glcpp.c
> +++ b/src/glsl/glcpp/glcpp.c
> @@ -164,7 +164,7 @@ main (int argc, char *argv[])
> if (shader == NULL)
> return 1;
>
> - ret = glcpp_preprocess(ctx, &shader, &info_log, NULL, &gl_ctx);
> + ret = glcpp_preprocess(ctx, &shader, &info_log, NULL, &gl_ctx, NULL);
>
> printf("%s", shader);
> fprintf(stderr, "%s", info_log);
> diff --git a/src/glsl/glcpp/glcpp.h b/src/glsl/glcpp/glcpp.h
> index 8aaa551..e7835cf 100644
> --- a/src/glsl/glcpp/glcpp.h
> +++ b/src/glsl/glcpp/glcpp.h
> @@ -28,6 +28,7 @@
> #include <stdbool.h>
>
> #include "main/mtypes.h"
> +#include "glcpp_extras.h"
>
> #include "../ralloc.h"
>
> @@ -185,12 +186,14 @@ struct glcpp_parser {
> bool has_new_source_number;
> int new_source_number;
> bool is_gles;
> + struct glcpp_extension_directive_list *directive_ext_list;
> };
>
> struct gl_extensions;
>
> glcpp_parser_t *
> -glcpp_parser_create (const struct gl_extensions *extensions, int api);
> +glcpp_parser_create (const struct gl_extensions *extensions, int api,
> + struct glcpp_extension_directive_list *ext_list);
>
> int
> glcpp_parser_parse (glcpp_parser_t *parser);
> @@ -198,9 +201,19 @@ glcpp_parser_parse (glcpp_parser_t *parser);
> void
> glcpp_parser_destroy (glcpp_parser_t *parser);
>
> +/**
> + Execute pre-processor on source code provided by *shader
> + and writing output to *shader on success
> + \param ext_list if NON-null indicates that the pre-processor
> + should intercept #extension directives
> + and build a list of the directives. The
> + outputted source code will have those
> + directive missing.
> + */
> int
> glcpp_preprocess(void *ralloc_ctx, const char **shader, char **info_log,
> - const struct gl_extensions *extensions, struct gl_context *g_ctx);
> + const struct gl_extensions *extensions, struct gl_context *g_ctx,
> + struct glcpp_extension_directive_list *ext_list);
>
> /* Functions for writing to the info log */
>
> diff --git a/src/glsl/glcpp/glcpp_extras.c b/src/glsl/glcpp/glcpp_extras.c
> new file mode 100644
> index 0000000..1672f9f
> --- /dev/null
> +++ b/src/glsl/glcpp/glcpp_extras.c
> @@ -0,0 +1,126 @@
> +#include <string.h>
> +#include <ctype.h>
> +#include "glcpp_extras.h"
> +#include "main/imports.h"
> +
> +#define MAX_STRING_LENGTH 200
> +
> +static const char*
> +remove_surrounding_white_space(char *value)
> +{
> + size_t len;
> + char *back;
> +
> + /*
> + remark:
> + -since the return value of remove_surrounding_white_space
> + is to never be freed, we can safely return a non-NULL
> + "empty" string if value was NULL.
> + */
> +
> + if(value==NULL || *value==0) {
> + return "";
> + }
> +
> + while(*value!=0 && isspace(*value)) {
> + ++value;
> + }
> +
> + len=strlen(value);
> + if(len==0) {
> + return value;
> + }
> +
> + for(back = value + len - 1; back!=value && isspace(*back); --back) {
> + *back=0;
> + }
> +
> + return value;
> +}
> +
> +static char*
> +copy_string(const char *src)
> +{
> + char *dst;
> + size_t len;
> +
> + len=strnlen(src, MAX_STRING_LENGTH);
> + dst=MALLOC(len+1);
> + strncpy(dst, src, len);
> +
> + dst[len]=0;
> + return dst;
> +}
> +
> +static
> +void
> +delete_glcpp_extension_directive_entry(struct glcpp_extension_directive_list_element *p)
> +{
> + FREE(p->line_string);
> + FREE(p->strtoked_string);
> + FREE(p);
> +}
> +
> +void
> +init_glcpp_extension_directive_list(struct glcpp_extension_directive_list *p)
> +{
> + assert(p!=NULL);
> + p->head=p->tail=NULL;
> + p->count=0;
> +}
> +
> +void
> +delete_glcpp_extension_directive_list_contents(struct glcpp_extension_directive_list *p)
> +{
> + struct glcpp_extension_directive_list_element *current;
> +
> + assert(p!=NULL);
> + current=p->head;
> + while(current!=NULL) {
> + struct glcpp_extension_directive_list_element *next;
> +
> + next=current->next;
> + delete_glcpp_extension_directive_entry(current);
> + current=next;
> + }
> + p->head=p->tail=NULL;
> + p->count=0;
> +}
> +
> +void
> +add_extension_from_preprocessor(const char *l,
> + int first_line,
> + int first_column,
> + int last_line,
> + int last_column,
> + unsigned source,
> + struct glcpp_extension_directive_list *directive_list)
> +{
> + struct glcpp_extension_directive_list_element *current;
> + char *save_strok_state;
> +
> + current=MALLOC_STRUCT(glcpp_extension_directive_list_element);
> + current->line_string=copy_string(l);
> +
> + current->location.first_line = first_line;
> + current->location.first_column = first_column;
> + current->location.last_line = last_line;
> + current->location.last_column = last_column;
> + current->location.source = source;
> +
> + current->strtoked_string=copy_string(l);
> + strtok_r(current->strtoked_string, " ", &save_strok_state); //get #extension
> + current->name=remove_surrounding_white_space(strtok_r(NULL, ":", &save_strok_state));
> + current->behavior_string=remove_surrounding_white_space(strtok_r(NULL, " ", &save_strok_state));
> + current->next=NULL;
> +
> + if(directive_list->tail==NULL) {
> + assert(directive_list->head==NULL);
> + directive_list->head=current;
> + } else {
> + directive_list->tail->next=current;
> + }
> + directive_list->tail=current;
> + directive_list->count++;
> +}
> +
> diff --git a/src/glsl/glcpp/glcpp_extras.h b/src/glsl/glcpp/glcpp_extras.h
> new file mode 100644
> index 0000000..3c67ea9
> --- /dev/null
> +++ b/src/glsl/glcpp/glcpp_extras.h
> @@ -0,0 +1,97 @@
> +#ifndef GLCPP_EXTRAS_H
> +#define GLCPP_EXTRAS_H
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * Specifies a single #extension directive picked up
> + * by the pre-processor.
> + */
> +struct glcpp_extension_directive_list_element {
> +
> + char *line_string; /*!< string from source line of GLSL */
> +
> + struct {
> + int first_line;
> + int first_column;
> + int last_line;
> + int last_column;
> + unsigned source;
> + } location; /*!< location of directive in GLSL source */
> +
> + /**
> + name of extension, is a substring of
> + \ref strtoked_string or a substring of
> + a const (compiled) string
> + */
> + const char *name;
> +
> + /**
> + behavior of extension, is a substring of
> + \ref strtoked_string or a substring of
> + a const (compiled) string
> + */
> + const char *behavior_string;
> +
> + /**
> + * next element in linked list
> + */
> + struct glcpp_extension_directive_list_element *next;
> +
> + char *strtoked_string; /*!< original string modified by strtok_r */
> +};
> +
> +/**
> + Convenience wrapper for linked list of elements.
> + */
> +struct glcpp_extension_directive_list {
> + struct glcpp_extension_directive_list_element *head;
> + struct glcpp_extension_directive_list_element *tail;
> + int count;
> +};
> +
> +/**
> + * Initialize the contents of a \ref glcpp_extension_directive_list
> + * \param l struct to initialize, must not be NULL
> + */
> +extern void
> +init_glcpp_extension_directive_list(struct glcpp_extension_directive_list *l);
> +
> +/**
> + Delete the <i>contents</i> of a \ref glcpp_extension_directive_list.
> + Does NOT delete the containing struct.
> + \param l struct to clear, must not be NULL.
> + */
> +extern void
> +delete_glcpp_extension_directive_list_contents(struct glcpp_extension_directive_list *l);
> +
> +/**
> + * Means of allowing #extension directives to come
> + * -after- source code by processing the directives
> + * in the pre-processor.
> + *
> + * \param line_string line of source of the extension directive, string is copied
> + * \param first_line value of first_line field of location of directive
> + * \param first_column value of first_column field of location of directive
> + * \param last_line value of last_line field of location of directive
> + * \param last_column value of field last_column of location of directive
> + * \param source value of source field of location of directive
> + * \param l list to which to append the directive
> + */
> +extern void
> +add_extension_from_preprocessor(const char *line_string,
> + int first_line,
> + int first_column,
> + int last_line,
> + int last_column,
> + unsigned source,
> + struct glcpp_extension_directive_list *l);
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +
> +#endif
> diff --git a/src/glsl/glcpp/pp.c b/src/glsl/glcpp/pp.c
> index 7e1b6c6..48176a6 100644
> --- a/src/glsl/glcpp/pp.c
> +++ b/src/glsl/glcpp/pp.c
> @@ -136,10 +136,11 @@ remove_line_continuations(glcpp_parser_t *ctx, const char *shader)
>
> int
> glcpp_preprocess(void *ralloc_ctx, const char **shader, char **info_log,
> - const struct gl_extensions *extensions, struct gl_context *gl_ctx)
> + const struct gl_extensions *extensions, struct gl_context *gl_ctx,
> + struct glcpp_extension_directive_list *ext_list)
> {
> int errors;
> - glcpp_parser_t *parser = glcpp_parser_create (extensions, gl_ctx->API);
> + glcpp_parser_t *parser = glcpp_parser_create (extensions, gl_ctx->API, ext_list);
>
> if (! gl_ctx->Const.DisableGLSLLineContinuations)
> *shader = remove_line_continuations(parser, *shader);
> diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
> index d76d94b..75640e1 100644
> --- a/src/glsl/glsl_parser_extras.cpp
> +++ b/src/glsl/glsl_parser_extras.cpp
> @@ -603,6 +603,41 @@ static const _mesa_glsl_extension *find_extension(const char *name)
> return NULL;
> }
>
> +/**
> + * Walks the directive list built by the preprocessor and
> + * sets the parser state to enable them, in addition frees
> + * the elements of the linked list.
> + */
> +static
> +void
> +_mesa_glsl_enable_extensions_from_cpp(struct _mesa_glsl_parse_state *state)
> +{
> + if(state->ext_directive_list) {
> + YYLTYPE location;
> + struct glcpp_extension_directive_list_element *current;
> + bool extension_ok;
> +
> + for(current=state->ext_directive_list->head; current!=NULL; current=current->next) {
> + location.first_line = current->location.first_line;
> + location.first_column = current->location.first_column;
> + location.last_line = current->location.last_line;
> + location.last_column = current->location.last_column;
> + location.source = current->location.source;
> + extension_ok=_mesa_glsl_process_extension(current->name, &location,
> + current->behavior_string, &location,
> + state);
> +
> + if(!extension_ok) {
> + _mesa_glsl_error(&location, state, "%s", current->line_string);
> + }
> +
> + }
> + delete_glcpp_extension_directive_list_contents(state->ext_directive_list);
> + state->ext_directive_list=NULL;
> + }
> +}
> +
> +
>
> bool
> _mesa_glsl_process_extension(const char *name, YYLTYPE *name_locp,
> @@ -1475,15 +1510,33 @@ void
> _mesa_glsl_compile_shader(struct gl_context *ctx, struct gl_shader *shader,
> bool dump_ast, bool dump_hir)
> {
> + struct glcpp_extension_directive_list extension_directive_list;
> struct _mesa_glsl_parse_state *state =
> new(shader) _mesa_glsl_parse_state(ctx, shader->Type, shader);
> const char *source = shader->Source;
>
> + if(1) {
> + state->ext_directive_list=&extension_directive_list;
> + init_glcpp_extension_directive_list(state->ext_directive_list);
> + } else {
> + state->ext_directive_list=NULL;
> + }
> +
> state->error = glcpp_preprocess(state, &source, &state->info_log,
> - &ctx->Extensions, ctx);
> + &ctx->Extensions, ctx,
> + state->ext_directive_list);
>
> +
> if (!state->error) {
> _mesa_glsl_lexer_ctor(state, source);
> +
> + /*
> + before parsing, walk the directive list
> + and enable the extensions.
> + */
> + _mesa_glsl_enable_extensions_from_cpp(state);
> +
> +
> _mesa_glsl_parse(state);
> _mesa_glsl_lexer_dtor(state);
> }
> diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
> index d232bb3..c163ae5 100644
> --- a/src/glsl/glsl_parser_extras.h
> +++ b/src/glsl/glsl_parser_extras.h
> @@ -25,6 +25,9 @@
> #ifndef GLSL_PARSER_EXTRAS_H
> #define GLSL_PARSER_EXTRAS_H
>
> +
> +#include "glcpp/glcpp_extras.h"
> +
> /*
> * Most of the definitions here only apply to C++
> */
> @@ -376,6 +379,14 @@ struct _mesa_glsl_parse_state {
>
> /** Atomic counter offsets by binding */
> unsigned atomic_counter_offsets[MAX_COMBINED_ATOMIC_BUFFERS];
> +
> + /**
> + If non-NULL gives a list of directives picked up
> + by the preprocessor.
> + */
> + struct glcpp_extension_directive_list *ext_directive_list;
> +
> +
> };
>
> # define YYLLOC_DEFAULT(Current, Rhs, N) \
> @@ -450,7 +461,8 @@ extern const char *
> _mesa_glsl_shader_target_name(GLenum type);
>
> extern int glcpp_preprocess(void *ctx, const char **shader, char **info_log,
> - const struct gl_extensions *extensions, struct gl_context *gl_ctx);
> + const struct gl_extensions *extensions, struct gl_context *gl_ctx,
> + struct glcpp_extension_directive_list *ext_list);
>
> extern void _mesa_destroy_shader_compiler(void);
> extern void _mesa_destroy_shader_compiler_caches(void);
>
More information about the mesa-dev
mailing list