[Mesa-dev] [PATCH] glsl: translate transform feedback varyings into low-level representation

[Kenneth Graunke]

On 10/26/2011 03:28 PM, Marek Olšák wrote:
> This adds a function that takes an array of varyings from
> glTranformFeedbackVaryingsEXT and generates gl_transform_feedback_info,
> which is supposed to be consumed by drivers. Useful for ir_to_mesa
> and glsl_to_tgsi.
> 
> With Dan McCabe's patch, I think this is all that's needed for transform
> feedback GLSL support. This work should probably be better integrated with
> Dan's code though (maybe some stuff that's done here should be done
> in the linker instead).
> 
> What's missing is reporting linker errors (AFAIK). This patch just covers
> the post-link part.
> 
> The piglit tests ext_transform_feedback-position,separate,interleaved
> pass with this.
> 
> ---
>  src/glsl/Makefile                           |    1 +
>  src/glsl/ir.h                               |   15 ++
>  src/glsl/ir_set_transform_feedback_outs.cpp |  299 +++++++++++++++++++++++++++
>  src/mesa/main/mtypes.h                      |   12 +
>  4 files changed, 327 insertions(+), 0 deletions(-)
>  create mode 100644 src/glsl/ir_set_transform_feedback_outs.cpp
> 
> diff --git a/src/glsl/Makefile b/src/glsl/Makefile
> index 504f1fb..f3b8e2e 100644
> --- a/src/glsl/Makefile
> +++ b/src/glsl/Makefile
> @@ -48,6 +48,7 @@ CXX_SOURCES = \
>  	ir_reader.cpp \
>  	ir_rvalue_visitor.cpp \
>  	ir_set_program_inouts.cpp \
> +	ir_set_transform_feedback_outs.cpp \
>  	ir_validate.cpp \
>  	ir_variable.cpp \
>  	ir_variable_refcount.cpp \
> diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> index b707634..c17473c 100644
> --- a/src/glsl/ir.h
> +++ b/src/glsl/ir.h
> @@ -1685,4 +1685,19 @@ extern char *
>  prototype_string(const glsl_type *return_type, const char *name,
>  		 exec_list *parameters);
>  
> +/**
> + * Set transform feedback output locations and other related info
> + * in gl_program.
> + *
> + * \param shaderprog  The inputs are the parameters
> + *                    from glTransformFeedbackVaryings expected
> + *                    to be in gl_shader_program::TransformFeedback.
> + *
> + * \param info        Where the resulting info is stored.
> + */
> +extern void
> +do_set_transform_feedback_outs(exec_list *instructions,
> +                               struct gl_shader_program *shaderprog,
> +                               struct gl_transform_feedback_info *info);
> +
>  #endif /* IR_H */
> diff --git a/src/glsl/ir_set_transform_feedback_outs.cpp b/src/glsl/ir_set_transform_feedback_outs.cpp
> new file mode 100644
> index 0000000..0138024
> --- /dev/null
> +++ b/src/glsl/ir_set_transform_feedback_outs.cpp
> @@ -0,0 +1,299 @@
> +/*
> + * Copyright © 2010 Intel Corporation
> + * Copyright © 2011 Marek Olšák <maraeo at gmail.com>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + */
> +
> +/**
> + * \file ir_set_transform_feedback_outs.cpp
> + *
> + * Used to obtain info about shader outputs from the GLSL IR
> + * for transform feedback.
> + * The driver codegen backend needs to know locations of the outputs
> + * which are to be stored in transform feedback buffers and the number
> + * of components each such output has.
> + *
> + * This is similar to ir_set_program_inouts.
> + */
> +
> +extern "C" {
> +#include "program/hash_table.h"
> +}
> +#include "main/core.h"
> +#include "ir.h"
> +#include "ir_visitor.h"
> +#include "glsl_types.h"
> +#include <cmath>
> +
> +
> +struct tfeedback_decl {
> +   char name[1024];

Not so excited about the arbitrarily sized string...

> +   bool is_array;
> +   unsigned array_index;
> +};
> +
> +
> +/* This expects expressions of the form "var" and "var[i]",
> + * where i is a literal.
> + *
> + * We don't have to be pedantic about what is a valid GLSL variable name,
> + * because any variable with an invalid name can't exist in the IR anyway. */
> +static bool parse_tfeedback_decl(const char *input,
> +                                 struct tfeedback_decl *decl)
> +{
> +   unsigned name_index = 0;
> +
> +   memset(decl, 0, sizeof(*decl));
> +
> +   /* Parse the variable name. */
> +   while (*input &&
> +          ((*input > 'a' && *input < 'z') ||
> +           (*input > 'A' && *input < 'Z') ||
> +           (*input > '0' && *input < '9') ||
> +           *input == '_') &&
> +          name_index < Elements(decl->name)-1) {
> +      decl->name[name_index++] = *input;
> +      input++;
> +   }
> +
> +   if (!*input) {
> +      return true; /* Found: "var" */
> +   }
> +
> +   /* Parse the array index. */
> +   if (*input == '[') {
> +      input++;
> +
> +      if (*input && *input >= '0' && *input <= '9' &&
> +          sscanf(input, "%u", &decl->array_index) == 1) {
> +         decl->is_array = true;
> +         input += (unsigned)(log(decl->array_index)/log(10)) + 1;
> +
> +         if (*input && *input == ']') {
> +            input++;
> +            if (!*input) {
> +               return true; /* Found: "var[i]" */
> +            }
> +         }
> +      }
> +   }
> +
> +   return false;

I think this could be simplified a bit.  How about:

char *bracket = strrchr(input, '[');
if (bracket) {
   decl->name = ralloc_strndup(mem_ctx, input, bracket - input);
   if (sscanf(bracket, "[%u]", &decl->array_index) == 1) {
      decl->is_array = true;
      return true; /* Found: "var[i]" */
   }
} else {
   decl->name = ralloc_strdup(mem_ctx, input);
}

This does let a lot more slide, but...we don't have to be strict here,
right?