[Mesa-dev] [PATCH] glsl: disallow mixed varying types within a location
Ilia Mirkin
imirkin at alum.mit.edu
Wed Sep 6 01:23:40 UTC 2017
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 <imirkin at 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
More information about the mesa-dev
mailing list