On 06/09/17 11:23, Ilia Mirkin wrote:
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.
There are still use cases such as SSO, tessellation shaders and varyings
used by interpolateAt (although we just enable the enhanced layout
packing rules by default for those anyway) were we cannot use the
auto-packer.
As far as the patch goes this should really be in link_varyings.cpp
rather than linker.cpp, also there is already related validation code in
cross_validate_outputs_to_inputs() any reason for not just modifying the
code there?
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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
