diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h index 6d79d42af45..cd1c53c29e7 100644 --- a/src/compiler/glsl/ir.h +++ b/src/compiler/glsl/ir.h @@ -766,6 +766,13 @@ public: */ unsigned is_unmatched_generic_inout:1; + /** + * Is this varying used by transform feedback? + * + * This is used by the linker to decide if it's safe to pack the varying. + */ + unsigned is_xfb:1; + /** * Is this varying used only by transform feedback? * diff --git a/src/compiler/glsl/ir_optimization.h b/src/compiler/glsl/ir_optimization.h index cd0be38e56f..11695c5aba2 100644 --- a/src/compiler/glsl/ir_optimization.h +++ b/src/compiler/glsl/ir_optimization.h @@ -156,7 +156,9 @@ void lower_packed_varyings(void *mem_ctx, ir_variable_mode mode, unsigned gs_input_vertices, gl_linked_shader *shader, - bool disable_varying_packing, bool xfb_enabled); + bool disable_varying_packing, + bool disable_xfb_packing, + bool xfb_enabled); bool lower_vector_insert(exec_list *instructions, bool lower_nonconstant_index); bool lower_vector_derefs(gl_linked_shader *shader); void lower_named_interface_blocks(void *mem_ctx, gl_linked_shader *shader); diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index 0868d1504db..4b63f7693f6 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -1590,7 +1590,9 @@ namespace { class varying_matches { public: - varying_matches(bool disable_varying_packing, bool xfb_enabled, + varying_matches(bool disable_varying_packing, + bool disable_xfb_packing, + bool xfb_enabled, bool enhanced_layouts_enabled, gl_shader_stage producer_stage, gl_shader_stage consumer_stage); @@ -1616,11 +1618,17 @@ private: */ const bool disable_varying_packing; + /** + * If true, this driver disables packing for varyings used by transform + * feedback. + */ + const bool disable_xfb_packing; + /** * If true, this driver has transform feedback enabled. The transform - * feedback code requires at least some packing be done even when varying - * packing is disabled, fortunately where transform feedback requires - * packing it's safe to override the disabled setting. See + * feedback code usually requires at least some packing be done even + * when varying packing is disabled, fortunately where transform feedback + * requires packing it's safe to override the disabled setting. See * is_varying_packing_safe(). */ const bool xfb_enabled; @@ -1647,6 +1655,7 @@ private: static packing_order_enum compute_packing_order(const ir_variable *var); static int match_comparator(const void *x_generic, const void *y_generic); static int xfb_comparator(const void *x_generic, const void *y_generic); + static int not_xfb_comparator(const void *x_generic, const void *y_generic); /** * Structure recording the relationship between a single producer output @@ -1702,11 +1711,13 @@ private: } /* anonymous namespace */ varying_matches::varying_matches(bool disable_varying_packing, + bool disable_xfb_packing, bool xfb_enabled, bool enhanced_layouts_enabled, gl_shader_stage producer_stage, gl_shader_stage consumer_stage) : disable_varying_packing(disable_varying_packing), + disable_xfb_packing(disable_xfb_packing), xfb_enabled(xfb_enabled), enhanced_layouts_enabled(enhanced_layouts_enabled), producer_stage(producer_stage), @@ -1785,6 +1796,7 @@ varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var) producer_var->type->contains_double()); if (!disable_varying_packing && + (!disable_xfb_packing || producer_var == NULL || !producer_var->data.is_xfb) && (needs_flat_qualifier || (consumer_stage != MESA_SHADER_NONE && consumer_stage != MESA_SHADER_FRAGMENT))) { /* Since this varying is not being consumed by the fragment shader, its @@ -1850,6 +1862,7 @@ varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var) this->matches[this->num_matches].packing_order = this->compute_packing_order(var); if ((this->disable_varying_packing && !is_varying_packing_safe(type, var)) || + (this->disable_xfb_packing && var->data.is_xfb) || var->data.must_be_shader_input) { unsigned slots = type->count_attribute_slots(false); this->matches[this->num_matches].num_components = slots * 4; @@ -1890,19 +1903,29 @@ varying_matches::assign_locations(struct gl_shader_program *prog, * When packing is disabled the sort orders varyings used by transform * feedback first, but also depends on *undefined behaviour* of qsort to * reverse the order of the varyings. See: xfb_comparator(). + * + * If packing is only disabled for xfb varyings (mutually exclusive with + * disable_varying_packing), we then group varyings depending on if they + * are captured for transform feedback. The same *undefined behaviour* is + * taken advantage of. */ - if (!this->disable_varying_packing) { - /* Sort varying matches into an order that makes them easy to pack. */ - qsort(this->matches, this->num_matches, sizeof(*this->matches), - &varying_matches::match_comparator); - } else { + if (this->disable_varying_packing) { /* Only sort varyings that are only used by transform feedback. */ qsort(this->matches, this->num_matches, sizeof(*this->matches), &varying_matches::xfb_comparator); + } else if (this->disable_xfb_packing) { + /* Only sort varyings that are NOT used by transform feedback. */ + qsort(this->matches, this->num_matches, sizeof(*this->matches), + &varying_matches::not_xfb_comparator); + } else { + /* Sort varying matches into an order that makes them easy to pack. */ + qsort(this->matches, this->num_matches, sizeof(*this->matches), + &varying_matches::match_comparator); } unsigned generic_location = 0; unsigned generic_patch_location = MAX_VARYING*4; + bool previous_var_xfb = false; bool previous_var_xfb_only = false; unsigned previous_packing_class = ~0u; @@ -1939,6 +1962,9 @@ varying_matches::assign_locations(struct gl_shader_program *prog, * class than the previous one, and we're not already on a slot * boundary. * + * Also advance if varying packing is disabled for transform feedback, + * and previous or current varying is used for transform feedback. + * * Also advance to the next slot if packing is disabled. This makes sure * we don't assign varyings the same locations which is possible * because we still pack individual arrays, records and matrices even @@ -1947,6 +1973,8 @@ varying_matches::assign_locations(struct gl_shader_program *prog, * feedback. */ if (var->data.must_be_shader_input || + (this->disable_xfb_packing && + (previous_var_xfb || var->data.is_xfb)) || (this->disable_varying_packing && !(previous_var_xfb_only && var->data.is_xfb_only)) || (previous_packing_class != this->matches[i].packing_class) || @@ -1955,6 +1983,7 @@ varying_matches::assign_locations(struct gl_shader_program *prog, *location = ALIGN(*location, 4); } + previous_var_xfb = var->data.is_xfb; previous_var_xfb_only = var->data.is_xfb_only; previous_packing_class = this->matches[i].packing_class; @@ -2211,6 +2240,32 @@ varying_matches::xfb_comparator(const void *x_generic, const void *y_generic) } +/** + * Comparison function passed to qsort() to sort varyings NOT used by + * transform feedback when packing of xfb varyings is disabled. + */ +int +varying_matches::not_xfb_comparator(const void *x_generic, const void *y_generic) +{ + const match *x = (const match *) x_generic; + + if (x->producer_var != NULL && !x->producer_var->data.is_xfb) + return match_comparator(x_generic, y_generic); + + /* FIXME: When the comparator returns 0 it means the elements being + * compared are equivalent. However the qsort documentation says: + * + * "The order of equivalent elements is undefined." + * + * In practice the sort ends up reversing the order of the varyings which + * means locations are also assigned in this reversed order and happens to + * be what we want. This is also whats happening in + * varying_matches::match_comparator(). + */ + return 0; +} + + /** * Is the given variable a varying variable to be counted against the * limit in ctx->Const.MaxVarying? @@ -2558,11 +2613,17 @@ assign_varying_locations(struct gl_context *ctx, /* Transform feedback code assumes varying arrays are packed, so if the * driver has disabled varying packing, make sure to at least enable - * packing required by transform feedback. + * packing required by transform feedback. See below for exception. */ bool xfb_enabled = ctx->Extensions.EXT_transform_feedback && !unpackable_tess; + /* Some drivers actually requires packing to be explicitly disabled + * for varyings used by transform feedback. + */ + bool disable_xfb_packing = + ctx->Const.DisableTransformFeedbackPacking; + /* Disable packing on outward facing interfaces for SSO because in ES we * need to retain the unpacked varying information for draw time * validation. @@ -2577,7 +2638,9 @@ assign_varying_locations(struct gl_context *ctx, if (prog->SeparateShader && (producer == NULL || consumer == NULL)) disable_varying_packing = true; - varying_matches matches(disable_varying_packing, xfb_enabled, + varying_matches matches(disable_varying_packing, + disable_xfb_packing, + xfb_enabled, ctx->Extensions.ARB_enhanced_layouts, producer ? producer->Stage : MESA_SHADER_NONE, consumer ? consumer->Stage : MESA_SHADER_NONE); @@ -2732,8 +2795,10 @@ assign_varying_locations(struct gl_context *ctx, consumer_inputs, consumer_interface_inputs, consumer_inputs_with_locations); - if (input_var) + if (input_var) { + input_var->data.is_xfb = 1; input_var->data.always_active_io = 1; + } if (matched_candidate->toplevel_var->data.is_unmatched_generic_inout) { matched_candidate->toplevel_var->data.is_xfb_only = 1; @@ -2804,13 +2869,13 @@ assign_varying_locations(struct gl_context *ctx, if (producer) { lower_packed_varyings(mem_ctx, slots_used, components, ir_var_shader_out, 0, producer, disable_varying_packing, - xfb_enabled); + disable_xfb_packing, xfb_enabled); } if (consumer) { lower_packed_varyings(mem_ctx, slots_used, components, ir_var_shader_in, - consumer_vertices, consumer, - disable_varying_packing, xfb_enabled); + consumer_vertices, consumer, disable_varying_packing, + disable_xfb_packing, xfb_enabled); } return true; diff --git a/src/compiler/glsl/lower_packed_varyings.cpp b/src/compiler/glsl/lower_packed_varyings.cpp index d15a45960b3..9c418ebae63 100644 --- a/src/compiler/glsl/lower_packed_varyings.cpp +++ b/src/compiler/glsl/lower_packed_varyings.cpp @@ -173,6 +173,7 @@ public: exec_list *out_instructions, exec_list *out_variables, bool disable_varying_packing, + bool disable_xfb_packing, bool xfb_enabled); void run(struct gl_linked_shader *shader); @@ -240,6 +241,7 @@ private: exec_list *out_variables; bool disable_varying_packing; + bool disable_xfb_packing; bool xfb_enabled; }; @@ -250,7 +252,7 @@ lower_packed_varyings_visitor::lower_packed_varyings_visitor( ir_variable_mode mode, unsigned gs_input_vertices, exec_list *out_instructions, exec_list *out_variables, bool disable_varying_packing, - bool xfb_enabled) + bool disable_xfb_packing, bool xfb_enabled) : mem_ctx(mem_ctx), locations_used(locations_used), components(components), @@ -262,6 +264,7 @@ lower_packed_varyings_visitor::lower_packed_varyings_visitor( out_instructions(out_instructions), out_variables(out_variables), disable_varying_packing(disable_varying_packing), + disable_xfb_packing(disable_xfb_packing), xfb_enabled(xfb_enabled) { } @@ -769,12 +772,21 @@ lower_packed_varyings_visitor::needs_lowering(ir_variable *var) if (var->data.explicit_location || var->data.must_be_shader_input) return false; + const glsl_type *type = var->type; + + /* Some drivers (e.g. panfrost) don't support packing of transform + * feedback varyings. + */ + if (disable_xfb_packing && var->data.is_xfb && + !(type->is_array() || type->is_struct() || type->is_matrix()) && + xfb_enabled) + return false; + /* Override disable_varying_packing if the var is only used by transform * feedback. Also override it if transform feedback is enabled and the * variable is an array, struct or matrix as the elements of these types * will always have the same interpolation and therefore are safe to pack. */ - const glsl_type *type = var->type; if (disable_varying_packing && !var->data.is_xfb_only && !((type->is_array() || type->is_struct() || type->is_matrix()) && xfb_enabled)) @@ -874,7 +886,7 @@ lower_packed_varyings(void *mem_ctx, unsigned locations_used, const uint8_t *components, ir_variable_mode mode, unsigned gs_input_vertices, gl_linked_shader *shader, bool disable_varying_packing, - bool xfb_enabled) + bool disable_xfb_packing, bool xfb_enabled) { exec_list *instructions = shader->ir; ir_function *main_func = shader->symbols->get_function("main"); @@ -890,6 +902,7 @@ lower_packed_varyings(void *mem_ctx, unsigned locations_used, &new_instructions, &new_variables, disable_varying_packing, + disable_xfb_packing, xfb_enabled); visitor.run(shader); if (mode == ir_var_shader_out) { diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 6d18d2f5de5..91fc71cfcfd 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -3974,6 +3974,15 @@ struct gl_constants */ GLboolean DisableVaryingPacking; + /** + * Disable varying packing if used for transform feedback. This is needed + * for some drivers (e.g. Panfrost) where transform feedback requires + * unpacked varyings. + * + * This variable is mutually exlusive with DisableVaryingPacking. + */ + GLboolean DisableTransformFeedbackPacking; + /** * UBOs and SSBOs can be packed tightly by the OpenGL implementation when * layout is set as shared (the default) or packed. However most Mesa drivers