<div class="gmail_quote">On 7 November 2011 22:28, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> <div class="im">This patch modifies the GLSL linker to assign additional slots for varying variables used by transform feedback, and record the varying slots used by transform feedback for use by the driver back-end. This required modifying assign_varying_locations() so that it assigns a varying location if either (a) the varying is used by the next stage of the GL pipeline, or (b) the varying is required by transform feedback. In order to avoid duplicating the code to assign a single varying location, I moved it into its own function, assign_varying_location(). </div>In addition, to support transform feedback in the case where there is no fragment shader, it is now possible to call assign_varying_locations() with a consumer of NULL. --- Changes from v1: - Fixed loop bound in tfeedback_decl::store() (was this->vector_elements, should have been this->matrix_columns). - Fixed the case where transform feedback is in use but there is no fragment shader. src/glsl/linker.cpp | 552 ++++++++++++++++++++++++++++++++++++++++++------ src/mesa/main/mtypes.h | 13 ++ 2 files changed, 502 insertions(+), 63 deletions(-) diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp index 915d5bb..5cccd7f 100644 --- a/src/glsl/linker.cpp +++ b/src/glsl/linker.cpp @@ -1519,10 +1519,358 @@ demote_shader_inputs_and_outputs(gl_shader *sh, enum ir_variable_mode mode) <div class="im"> } +/** + * Data structure tracking information about a transform feedback declaration + * during linking. + */ +class tfeedback_decl +{ +public: + bool init(struct gl_shader_program *prog, const void *mem_ctx, + const char *input); + static bool is_same(const tfeedback_decl &x, const tfeedback_decl &y); + bool assign_location(struct gl_context *ctx, struct gl_shader_program *prog, + ir_variable *output_var); </div>+ bool store(struct gl_shader_program *prog, + struct gl_transform_feedback_info *info, unsigned buffer) const; + + <div><div class="h5">+ /** + * True if assign_location() has been called for this object. + */ + bool is_assigned() const + { + return this->location != -1; + } + + /** + * Determine whether this object refers to the variable var. + */ + bool matches_var(ir_variable *var) const + { + return strcmp(var->name, this->var_name) == 0; + } + + /** + * The total number of varying components taken up by this variable. Only + * valid if is_assigned() is true. + */ + unsigned num_components() const + { + return this->vector_elements * this->matrix_columns; + } + +private: + /** + * The name that was supplied to glTransformFeedbackVaryings. Used for + * error reporting. + */ + const char *orig_name; + + /** + * The name of the variable, parsed from orig_name. + */ + char *var_name; + + /** + * True if the declaration in orig_name represents an array. + */ + bool is_array; + + /** + * If is_array is true, the array index that was specified in orig_name. + */ + unsigned array_index; + + /** + * The vertex shader output location that the linker assigned for this + * variable. -1 if a location hasn't been assigned yet. + */ + int location; + + /** + * If location != -1, the number of vector elements in this variable, or 1 + * if this variable is a scalar. + */ + unsigned vector_elements; + + /** + * If location != -1, the number of matrix columns in this variable, or 1 + * if this variable is not a matrix. + */ + unsigned matrix_columns; +}; + + +/** + * Initialize this object based on a string that was passed to + * glTransformFeedbackVaryings. If there is a parse error, the error is </div></div>+ * reported using linker_error(), and false is returned. + */ +bool <div><div class="h5">+tfeedback_decl::init(struct gl_shader_program *prog, const void *mem_ctx, + const char *input) +{ + /* 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. + */ + + this->location = -1; + this->orig_name = input; + + const char *bracket = strrchr(input, '['); + + if (bracket) { + this->var_name = ralloc_strndup(mem_ctx, input, bracket - input); + if (sscanf(bracket, "[%u]", &this->array_index) == 1) { + this->is_array = true; + return true; + } + } else { + this->var_name = ralloc_strdup(mem_ctx, input); + this->is_array = false; + return true; + } + + linker_error(prog, "Cannot parse transform feedback varying %s", input); </div></div><div class="im">+ return false; +} + </div><div class="im">+ +/** + * Determine whether two tfeedback_decl objects refer to the same variable and + * array index (if applicable). + */ +bool +tfeedback_decl::is_same(const tfeedback_decl &x, const tfeedback_decl &y) +{ + if (strcmp(x.var_name, y.var_name) != 0) + return false; + if (x.is_array != y.is_array) + return false; + if (x.is_array && x.array_index != y.array_index) + return false; + return true; +} + + +/** + * Assign a location for this tfeedback_decl object based on the location + * assignment in output_var. </div><div class="im">+ * + * If an error occurs, the error is reported through linker_error() and false + * is returned. + */ +bool </div><div class="im">+tfeedback_decl::assign_location(struct gl_context *ctx, + struct gl_shader_program *prog, + ir_variable *output_var) +{ + if (output_var->type->is_array()) { + /* Array variable */ + if (!this->is_array) { + linker_error(prog, "Transform feedback varying %s found, " </div>+ "but it's not an array ([] not expected).", <div class="im">+ this->orig_name); + return false; + } </div><div class="im">+ /* Check array bounds. */ + if (this->array_index >= + (unsigned) output_var->type->array_size()) { + linker_error(prog, "Transform feedback varying %s has index " + "%i, but the array size is %i.", + this->orig_name, this->array_index, + output_var->type->array_size()); </div></blockquote><div> </div><div>I just noticed that a "return false;" is missing right here. I'll fix this before pushing the patch tomorrow morning.</div> <div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"><div class="im"> + } + const unsigned matrix_cols = + output_var->type->fields.array->matrix_columns; + this->location = output_var->location + this->array_index * matrix_cols; + this->vector_elements = output_var->type->fields.array->vector_elements; + this->matrix_columns = matrix_cols; + } else { + /* Regular variable (scalar, vector, or matrix) */ + if (this->is_array) { + linker_error(prog, "Transform feedback varying %s found, " </div>+ "but it's an array ([] expected).", <div class="im">+ this->orig_name); + return false; + } </div><div class="im">+ this->location = output_var->location; + this->vector_elements = output_var->type->vector_elements; + this->matrix_columns = output_var->type->matrix_columns; + } </div><div class="im">+ /* From GL_EXT_transform_feedback: + * A program will fail to link if: + * </div><div class="im">+ * * the total number of components to capture in any varying + * variable in <varyings> is greater than the constant + * MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS_EXT and the + * buffer mode is SEPARATE_ATTRIBS_EXT; </div>+ */ <div class="im">+ if (prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS && + this->num_components() > + ctx->Const.MaxTransformFeedbackSeparateComponents) { </div>+ linker_error(prog, "Transform feedback varying %s exceeds " + "MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS.", <div class="im">+ this->orig_name); + return false; + } + </div><div class="im">+ return true; +} + + </div><div class="im">+/** + * Update gl_transform_feedback_info to reflect this tfeedback_decl. + * + * If an error occurs, the error is reported through linker_error() and false + * is returned. + */ +bool +tfeedback_decl::store(struct gl_shader_program *prog, + struct gl_transform_feedback_info *info, + unsigned buffer) const +{ + if (!this->is_assigned()) { </div><div class="im">+ /* From GL_EXT_transform_feedback: + * A program will fail to link if: + * </div><div class="im">+ * * any variable name specified in the <varyings> array is not + * declared as an output in the geometry shader (if present) or + * the vertex shader (if no geometry shader is present); + */ + linker_error(prog, "Transform feedback varying %s undeclared.", + this->orig_name); + return false; + } </div>+ for (unsigned v = 0; v < this->matrix_columns; ++v) { <div class="im">+ info->Outputs[info->NumOutputs].OutputRegister = this->location + v; + info->Outputs[info->NumOutputs].NumComponents = this->vector_elements; + info->Outputs[info->NumOutputs].OutputBuffer = buffer; + ++info->NumOutputs; + } + return true; +} </div><div class="im">+ + +/** + * Parse all the transform feedback declarations that were passed to + * glTransformFeedbackVaryings() and store them in tfeedback_decl objects. </div><div class="im">+ * + * If an error occurs, the error is reported through linker_error() and false + * is returned. + */ </div><div class="im">+static bool +parse_tfeedback_decls(struct gl_shader_program *prog, const void *mem_ctx, + unsigned num_names, char **varying_names, + tfeedback_decl *decls) </div>+{ <div class="im">+ for (unsigned i = 0; i < num_names; ++i) { + if (!decls[i].init(prog, mem_ctx, varying_names[i])) + return false; </div><div class="im">+ /* From GL_EXT_transform_feedback: + * A program will fail to link if: + * </div><div class="im">+ * * any two entries in the <varyings> array specify the same varying + * variable; + * + * We interpret this to mean "any two entries in the <varyings> array + * specify the same varying variable and array index", since transform + * feedback of arrays would be useless otherwise. + */ + for (unsigned j = 0; j < i; ++j) { + if (tfeedback_decl::is_same(decls[i], decls[j])) { + linker_error(prog, "Transform feedback varying %s specified " + "more than once.", varying_names[i]); + } </div><div class="im">+ return false; + } + } </div><div class="im">+ return true; +} + + +/** + * Assign a location for a variable that is produced in one pipeline stage + * (the "producer") and consumed in the next stage (the "consumer"). + * + * \param input_var is the input variable declaration in the consumer. + * + * \param output_var is the output variable declaration in the producer. + * + * \param input_index is the counter that keeps track of assigned input + * locations in the consumer. + * + * \param output_index is the counter that keeps track of assigned output + * locations in the producer. + * + * It is permissible for \c input_var to be NULL (this happens if a variable + * is output by the producer and consumed by transform feedback, but not + * consumed by the consumer). </div>+ * <div><div class="h5">+ * If the variable has already been assigned a location, this function has no + * effect. + */ +void +assign_varying_location(ir_variable *input_var, ir_variable *output_var, + unsigned *input_index, unsigned *output_index) +{ + if (output_var->location != -1) { + /* Location already assigned. */ + return; + } + + if (input_var) { + assert(input_var->location == -1); + input_var->location = *input_index; + } + + output_var->location = *output_index; + + /* FINISHME: Support for "varying" records in GLSL 1.50. */ + assert(!output_var->type->is_record()); + + if (output_var->type->is_array()) { + const unsigned slots = output_var->type->length + * output_var->type->fields.array->matrix_columns; + + *output_index += slots; + *input_index += slots; + } else { + const unsigned slots = output_var->type->matrix_columns; + + *output_index += slots; + *input_index += slots; + } +} + + +/** + * Assign locations for all variables that are produced in one pipeline stage + * (the "producer") and consumed in the next stage (the "consumer"). + * + * Variables produced by the producer may also be consumed by transform + * feedback. + * + * \param num_tfeedback_decls is the number of declarations indicating + * variables that may be consumed by transform feedback. + * + * \param tfeedback_decls is a pointer to an array of tfeedback_decl objects + * representing the result of parsing the strings passed to + * glTransformFeedbackVaryings(). assign_location() will be called for + * each of these objects that matches one of the outputs of the + * producer. + * </div></div>+ * When num_tfeedback_decls is nonzero, it is permissible for the consumer to + * be NULL. In this case, varying locations are assigned solely based on the + * requirements of transform feedback. <div class="im">+ */ bool assign_varying_locations(struct gl_context *ctx, struct gl_shader_program *prog, - gl_shader *producer, gl_shader *consumer) + gl_shader *producer, gl_shader *consumer, + unsigned num_tfeedback_decls, + tfeedback_decl *tfeedback_decls) { /* FINISHME: Set dynamically when geometry shader support is added. */ unsigned output_index = VERT_RESULT_VAR0; </div>@@ -1540,79 +1888,77 @@ assign_varying_locations(struct gl_context *ctx, */ invalidate_variable_locations(producer, ir_var_out, VERT_RESULT_VAR0); - invalidate_variable_locations(consumer, ir_var_in, FRAG_ATTRIB_VAR0); + if (consumer) + invalidate_variable_locations(consumer, ir_var_in, FRAG_ATTRIB_VAR0); <div class="im"> foreach_list(node, producer->ir) { ir_variable *const output_var = ((ir_instruction *) node)->as_variable(); - if ((output_var == NULL) || (output_var->mode != ir_var_out) - || (output_var->location != -1)) + if ((output_var == NULL) || (output_var->mode != ir_var_out)) continue; - ir_variable *const input_var = </div>- consumer->symbols->get_variable(output_var->name); <div class="im">- - if ((input_var == NULL) || (input_var->mode != ir_var_in)) - continue; - - assert(input_var->location == -1); - - output_var->location = output_index; - input_var->location = input_index; - - /* FINISHME: Support for "varying" records in GLSL 1.50. */ - assert(!output_var->type->is_record()); </div>+ ir_variable *input_var = + consumer ? consumer->symbols->get_variable(output_var->name) : NULL; <div class="im"> - if (output_var->type->is_array()) { - const unsigned slots = output_var->type->length - * output_var->type->fields.array->matrix_columns; + if (input_var && input_var->mode != ir_var_in) + input_var = NULL; </div><div class="im">- output_index += slots; - input_index += slots; - } else { - const unsigned slots = output_var->type->matrix_columns; + if (input_var) { + assign_varying_location(input_var, output_var, &input_index, + &output_index); + } - output_index += slots; - input_index += slots; + for (unsigned i = 0; i < num_tfeedback_decls; ++i) { + if (!tfeedback_decls[i].is_assigned() && + tfeedback_decls[i].matches_var(output_var)) { + if (output_var->location == -1) { + assign_varying_location(input_var, output_var, &input_index, + &output_index); + } + if (!tfeedback_decls[i].assign_location(ctx, prog, output_var)) + return false; + } } } </div> unsigned varying_vectors = 0; - foreach_list(node, consumer->ir) { - ir_variable *const var = ((ir_instruction *) node)->as_variable(); - - if ((var == NULL) || (var->mode != ir_var_in)) - continue; - - if (var->location == -1) { - if (prog->Version <= 120) { - /* On page 25 (page 31 of the PDF) of the GLSL 1.20 spec: - * - * Only those varying variables used (i.e. read) in - * the fragment shader executable must be written to - * by the vertex shader executable; declaring - * superfluous varying variables in a vertex shader is - * permissible. - * - * We interpret this text as meaning that the VS must - * write the variable for the FS to read it. See - * "glsl1-varying read but not written" in piglit. - */ - - linker_error(prog, "fragment shader varying %s not written " - "by vertex shader\n.", var->name); - } + if (consumer) { + foreach_list(node, consumer->ir) { + ir_variable *const var = ((ir_instruction *) node)->as_variable(); + + if ((var == NULL) || (var->mode != ir_var_in)) + continue; + + if (var->location == -1) { + if (prog->Version <= 120) { + /* On page 25 (page 31 of the PDF) of the GLSL 1.20 spec: + * + * Only those varying variables used (i.e. read) in + * the fragment shader executable must be written to + * by the vertex shader executable; declaring + * superfluous varying variables in a vertex shader is + * permissible. + * + * We interpret this text as meaning that the VS must + * write the variable for the FS to read it. See + * "glsl1-varying read but not written" in piglit. + */ + + linker_error(prog, "fragment shader varying %s not written " + "by vertex shader\n.", var->name); + } - /* An 'in' variable is only really a shader input if its - * value is written by the previous stage. - */ - var->mode = ir_var_auto; - } else { - /* The packing rules are used for vertex shader inputs are also used - * for fragment shader inputs. - */ - varying_vectors += count_attribute_slots(var->type); + /* An 'in' variable is only really a shader input if its + * value is written by the previous stage. + */ + var->mode = ir_var_auto; + } else { + /* The packing rules are used for vertex shader inputs are also + * used for fragment shader inputs. + */ + varying_vectors += count_attribute_slots(var->type); + } } } @@ -1637,9 +1983,54 @@ assign_varying_locations(struct gl_context *ctx, <div class="im"> } +/** + * Store transform feedback location assignments into + * prog->LinkedTransformFeedback based on the data stored in tfeedback_decls. </div><div class="im">+ * + * If an error occurs, the error is reported through linker_error() and false + * is returned. + */ </div><div class="im">+static bool +store_tfeedback_info(struct gl_context *ctx, struct gl_shader_program *prog, + unsigned num_tfeedback_decls, + tfeedback_decl *tfeedback_decls) +{ + unsigned total_tfeedback_components = 0; + prog->LinkedTransformFeedback.NumOutputs = 0; + for (unsigned i = 0; i < num_tfeedback_decls; ++i) { + unsigned buffer = + prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS ? i : 0; + if (!tfeedback_decls[i].store(prog, &prog->LinkedTransformFeedback, + buffer)) + return false; + total_tfeedback_components += tfeedback_decls[i].num_components(); + } + </div><div class="im">+ /* From GL_EXT_transform_feedback: + * A program will fail to link if: + * </div><div class="im">+ * * the total number of components to capture is greater than + * the constant MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS_EXT + * and the buffer mode is INTERLEAVED_ATTRIBS_EXT. </div>+ */ <div class="im">+ if (prog->TransformFeedback.BufferMode == GL_INTERLEAVED_ATTRIBS && + total_tfeedback_components > + ctx->Const.MaxTransformFeedbackInterleavedComponents) { + linker_error(prog, "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS " + "limit has been exceeded."); </div><div class="im">+ return false; + } + </div><div class="im">+ return true; +} + + void </div><div class="im"> link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) </div><div class="im"> { + tfeedback_decl *tfeedback_decls = NULL; </div>+ unsigned num_tfeedback_decls = prog->TransformFeedback.NumVarying; + void *mem_ctx = ralloc_context(NULL); // temporary linker context prog->LinkStatus = false; @@ -1806,19 +2197,54 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) break; } + if (num_tfeedback_decls != 0) { <div class="im">+ /* From GL_EXT_transform_feedback: + * A program will fail to link if: + * + * * the <count> specified by TransformFeedbackVaryingsEXT is + * non-zero, but the program object has no vertex or geometry + * shader; + */ </div>+ if (prev >= MESA_SHADER_FRAGMENT) { <div class="im">+ linker_error(prog, "Transform feedback varyings specified, but " + "no vertex or geometry shader is present."); + goto done; + } + </div><div class="im">+ tfeedback_decls = ralloc_array(mem_ctx, tfeedback_decl, + prog->TransformFeedback.NumVarying); + if (!parse_tfeedback_decls(prog, mem_ctx, num_tfeedback_decls, + prog->TransformFeedback.VaryingNames, + tfeedback_decls)) + goto done; + } + </div><div class="im"> for (unsigned i = prev + 1; i < MESA_SHADER_TYPES; i++) { if (prog->_LinkedShaders[i] == NULL) continue; </div>- if (!assign_varying_locations(ctx, prog, - prog->_LinkedShaders[prev], <div class="im">- prog->_LinkedShaders[i])) { </div>+ if (!assign_varying_locations( + ctx, prog, prog->_LinkedShaders[prev], prog->_LinkedShaders[i], + i == MESA_SHADER_FRAGMENT ? num_tfeedback_decls : 0, + tfeedback_decls)) goto done; - } prev = i; } + if (prev != MESA_SHADER_FRAGMENT && num_tfeedback_decls != 0) { + /* There was no fragment shader, but we still have to assign varying + * locations for use by transform feedback. + */ + if (!assign_varying_locations( + ctx, prog, prog->_LinkedShaders[prev], NULL, num_tfeedback_decls, <div class="im">+ tfeedback_decls)) + goto done; + } + </div>+ if (!store_tfeedback_info(ctx, prog, num_tfeedback_decls, tfeedback_decls)) + goto done; + if (prog->_LinkedShaders[MESA_SHADER_VERTEX] != NULL) { demote_shader_inputs_and_outputs(prog->_LinkedShaders[MESA_SHADER_VERTEX], ir_var_out); <div class="HOEnZb"><div class="h5">diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 6190972..5df898c 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -1818,6 +1818,16 @@ struct prog_instruction; struct gl_program_parameter_list; struct gl_uniform_list; +/** Post-link transform feedback info. */ +struct gl_transform_feedback_info { + unsigned NumOutputs; + + struct { + unsigned OutputRegister; + unsigned OutputBuffer; + unsigned NumComponents; + } Outputs[MAX_PROGRAM_OUTPUTS]; +}; /** * Base class for any kind of program object @@ -2191,6 +2201,9 @@ struct gl_shader_program GLchar **VaryingNames; /**< Array [NumVarying] of char * */ } TransformFeedback; + /** Post-link transform feedback info. */ + struct gl_transform_feedback_info LinkedTransformFeedback; + /** Geometry shader state - copied into gl_geometry_program at link time */ struct { GLint VerticesOut; -- 1.7.6.4 </div></div></blockquote></div>