[Mesa-dev] [PATCH] linker: Allow aliased location assignments for shader inputs / outputs

[Ian Romanick]

From: Ian Romanick <ian.d.romanick at intel.com>

The linker quotes two bits of OpenGL 4.0 spec text and claims that
either two paragraphs are contradictory or misunderstood.  It appears
that the later was the case.

Page 61 of the OpenGL 4.0 spec:

     "LinkProgram will fail if the attribute bindings assigned by
     BindAttribLocation do not leave not enough space to assign a
     location for an active matrix attribute or an active attribute
     array, both of which require multiple contiguous generic
     attributes."

Later on page 61 of the OpenGL 4.0 spec:

    "It is possible for an application to bind more than one attribute
    name to the same location. This is referred to as aliasing. This
    will only work if only one of the aliased attributes is active in
    the executable program, or if no path through the shader consumes
    more than one attribute of a set of attributes aliased to the same
    location. A link error can occur if the linker determines that every
    path through the shader consumes multiple aliased attributes, but
    implementations are not required to generate an error in this case."

These two paragraphs seem a bit contradictory, but they're talking about
two different things.

The first paragraph is talking about assignments near the end of the
slot limit.  Some types (e.g., matrix types) use multiple slots.  A mat4
uses four slots.  It is illegal to assign a multi-slot variable to a
location that would cause part of the variable to be assign out of
bounds slots.

The second paragraph is talking about cases where multiple varaibles are
assigned the same slot.  Variables can be assigned the same slot, but
unpredictable things might happen.

At least one test in the OpenGL ES 3.0 conformance suite tries to assign
multiple variables to the same location without a link error.  It does
something like:

layout(location = 3) in vec4 a;
layout(location = 3) in vec4 b;

The change in this patch is to just generate an error if the assigned
location would cause a variable spill over the end (the situation
described by the first quoted spec paragraph).

Fixes es3conform's explicit_attrib_location_vertex_input_aliased test.

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
Reviewed-and-Tested-by: Matt Turner <mattst88 at gmail.com>
---
 src/glsl/linker.cpp | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 3b2ab96..01ebcb6 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1393,11 +1393,18 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
 	     *     but implementations are not required to generate an error
 	     *     in this case."
 	     *
-	     * These two paragraphs are either somewhat contradictory, or I
-	     * don't fully understand one or both of them.
-	     */
-	    /* FINISHME: The code as currently written does not support
-	     * FINISHME: attribute location aliasing (see comment above).
+	     * These two paragraphs seem a bit contradictory, but they're
+	     * talking about two different things.
+	     *
+	     * The first paragraph is talking about assignments near the end
+	     * of the slot limit.  Some types (e.g., matrix types) use
+	     * multiple slots.  A mat4 uses four slots.  It is illegal to
+	     * assign a multi-slot variable to a location that would cause
+	     * part of the variable to be assign out of bounds slots.
+	     *
+	     * The second paragraph is talking about cases where multiple
+	     * varaibles are assigned the same slot.  Variables can be
+	     * assigned the same slot, but unpredictable things might happen.
 	     */
 	    /* Mask representing the contiguous slots that will be used by
 	     * this attribute.
@@ -1405,16 +1412,15 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
 	    const unsigned attr = var->location - generic_base;
 	    const unsigned use_mask = (1 << slots) - 1;
 
-	    /* Generate a link error if the set of bits requested for this
-	     * attribute overlaps any previously allocated bits.
+	    /* Generate a link error if the location assigned to the variable
+	     * causes it to spill over the limit.
 	     */
-	    if ((~(use_mask << attr) & used_locations) != used_locations) {
+	    if (attr + slots > max_index) {
 	       const char *const string = (target_index == MESA_SHADER_VERTEX)
 		  ? "vertex shader input" : "fragment shader output";
 	       linker_error(prog,
-			    "insufficient contiguous locations "
-			    "available for %s `%s' %d %d %d", string,
-			    var->name, used_locations, use_mask, attr);
+			    "insufficient locations available for %s `%s'",
+			    string, var->name);
 	       return false;
 	    }
 
-- 
1.7.11.7