The enhanced layouts spec has all kinds of restrictions about what can and cannot be mixed in a location. Integer/float(/presumably double) can't occupy a single location, interpolation has to be the same, as well as auxiliary storage (including patch!).
The implication of this is ... don't specify explicit locations/components if you want better packing, since the auto-packer doesn't care at all about most of these restrictions. Sad. This fixes: KHR-GL45.enhanced_layouts.varying_location_aliasing_with_mixed_types KHR-GL45.enhanced_layouts.varying_location_aliasing_with_mixed_interpolation KHR-GL45.enhanced_layouts.varying_location_aliasing_with_mixed_auxiliary_storage See https://github.com/KhronosGroup/OpenGL-API/issues/13 for some more info, where I asked about patch vs non-patch locations. Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu> --- src/compiler/glsl/link_varyings.cpp | 23 -------- src/compiler/glsl/linker.cpp | 115 ++++++++++++++++++++++++++++++++++++ src/compiler/glsl/linker.h | 4 ++ 3 files changed, 119 insertions(+), 23 deletions(-) diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp index 528506fd0eb..20187166203 100644 --- a/src/compiler/glsl/link_varyings.cpp +++ b/src/compiler/glsl/link_varyings.cpp @@ -40,29 +40,6 @@ #include "program.h" -/** - * Get the varying type stripped of the outermost array if we're processing - * a stage whose varyings are arrays indexed by a vertex number (such as - * geometry shader inputs). - */ -static const glsl_type * -get_varying_type(const ir_variable *var, gl_shader_stage stage) -{ - const glsl_type *type = var->type; - - if (!var->data.patch && - ((var->data.mode == ir_var_shader_out && - stage == MESA_SHADER_TESS_CTRL) || - (var->data.mode == ir_var_shader_in && - (stage == MESA_SHADER_TESS_CTRL || stage == MESA_SHADER_TESS_EVAL || - stage == MESA_SHADER_GEOMETRY)))) { - assert(type->is_array()); - type = type->fields.array; - } - - return type; -} - static void create_xfb_varying_names(void *mem_ctx, const glsl_type *t, char **name, size_t name_length, unsigned *count, diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 5c3f1d12bbc..3afe5b52a91 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -2155,6 +2155,109 @@ link_cs_input_layout_qualifiers(struct gl_shader_program *prog, } } +/** + * Get the varying type stripped of the outermost array if we're processing + * a stage whose varyings are arrays indexed by a vertex number (such as + * geometry shader inputs). + */ +const glsl_type * +get_varying_type(const ir_variable *var, gl_shader_stage stage) +{ + const glsl_type *type = var->type; + + if (!var->data.patch && + ((var->data.mode == ir_var_shader_out && + stage == MESA_SHADER_TESS_CTRL) || + (var->data.mode == ir_var_shader_in && + (stage == MESA_SHADER_TESS_CTRL || stage == MESA_SHADER_TESS_EVAL || + stage == MESA_SHADER_GEOMETRY)))) { + assert(type->is_array()); + type = type->fields.array; + } + + return type; +} + +static void +validate_intrastage_location_types(struct gl_shader_program *prog, + struct gl_linked_shader *shader) +{ + struct data_type { + // 0: unused, 1: float, 2: int, 3: 64-bit + unsigned var_type:2; + unsigned interpolation:2; + bool centroid:1; + bool sample:1; + bool patch:1; + } data_types[2][MAX_VARYING] = {}; + + foreach_in_list(ir_instruction, node, shader->ir) { + ir_variable *var = node->as_variable(); + if (!var || !var->data.explicit_location) + continue; + + if (var->data.mode != ir_var_shader_in && + var->data.mode != ir_var_shader_out) + continue; + + bool output = var->data.mode == ir_var_shader_out; + int var_slot; + if (!output && shader->Stage == MESA_SHADER_VERTEX) { + var_slot = var->data.location - VERT_ATTRIB_GENERIC0; + if (var_slot >= VERT_ATTRIB_GENERIC_MAX) + continue; + } else if (var->data.patch) { + var_slot = var->data.location - VARYING_SLOT_PATCH0; + if (var_slot >= MAX_VARYING) + continue; + } else { + var_slot = var->data.location - VARYING_SLOT_VAR0; + if (var_slot >= MAX_VARYING) + continue; + } + + if (var_slot < 0) + continue; + + const glsl_type *type = get_varying_type(var, shader->Stage); + const glsl_type *type_without_array = type->without_array(); + unsigned num_elements = type->count_attribute_slots(false); + unsigned var_type; + if (glsl_base_type_is_64bit(type_without_array->base_type)) + var_type = 3; + else if (glsl_base_type_is_integer(type_without_array->base_type)) + var_type = 2; + else + var_type = 1; + + const char *var_dir = output ? "out" : "in"; + + for (unsigned i = 0; i < num_elements; i++, var_slot++) { + data_type& existing = data_types[output][var_slot]; + + if (existing.var_type) { + if (existing.var_type != var_type) + linker_error(prog, "Mismatching %s varying types at location=%d", + var_dir, var_slot); + if (existing.interpolation != var->data.interpolation) + linker_error(prog, "Mismatching %s varying interpolation at location=%d", + var_dir, var_slot); + if (existing.centroid != var->data.centroid || + existing.sample != var->data.sample || + existing.patch != var->data.patch) + linker_error(prog, "Mismatching %s varying auxiliary storage at location=%d", + var_dir, var_slot); + } else { + existing.var_type = var_type; + existing.interpolation = var->data.interpolation; + existing.centroid = var->data.centroid; + existing.sample = var->data.sample; + existing.patch = var->data.patch; + } + } + } +} + /** * Combine a group of shaders for a single stage to generate a linked shader @@ -4941,6 +5044,18 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog) lower_named_interface_blocks(mem_ctx, prog->_LinkedShaders[i]); } + /* Check that each explicitly-assigned location doesn't mix + * types/interpolation/aux storage qualifiers. This must be done after + * lowering named interface blocks. + */ + for (unsigned int i = 0; i < MESA_SHADER_STAGES; i++) { + if (prog->_LinkedShaders[i] == NULL) + continue; + validate_intrastage_location_types(prog, prog->_LinkedShaders[i]); + if (!prog->data->LinkStatus) + goto done; + } + /* Implement the GLSL 1.30+ rule for discard vs infinite loops Do * it before optimization because we want most of the checks to get * dropped thanks to constant propagation. diff --git a/src/compiler/glsl/linker.h b/src/compiler/glsl/linker.h index 5cec121e634..6d6e8edd487 100644 --- a/src/compiler/glsl/linker.h +++ b/src/compiler/glsl/linker.h @@ -92,6 +92,10 @@ link_intrastage_shaders(void *mem_ctx, unsigned num_shaders, bool allow_missing_main); +const glsl_type * +get_varying_type(const ir_variable *var, gl_shader_stage stage); + + /** * Class for processing all of the leaf fields of a variable that corresponds * to a program resource. -- 2.13.5 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev