glsl: Return -1 for program interface query locations in many cases.

We were recording locations for all variables, even ones without an
explicit location set.  Implement the rules from the spec, and record
-1 in the resource list accordngly.  Make program_resource_location
stop doing math on negative values.  Remove hacks that are no longer
necessary now that we've stopped doing that.

Fixes 4 dEQP-GLES31.functional.program_interface_query tests:
- program_input.location.separable_fragment.var
- program_input.location.separable_fragment.var_array
- program_output.location.separable_vertex.var_array
- program_output.location.separable_vertex.var_array

v2: Delete more code

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Timothy Arceri <timothy.arceri@collabora.com>

src/compiler/glsl/linker.cpp

@@ -3497,7 +3497,8 @@ build_stageref(struct gl_shader_program *shProg, const char *name,
  * Create gl_shader_variable from ir_variable class.
  */
 static gl_shader_variable *
-create_shader_variable(struct gl_shader_program *shProg, const ir_variable *in)
+create_shader_variable(struct gl_shader_program *shProg,
+                       const ir_variable *in, bool use_implicit_location)
 {
    gl_shader_variable *out = ralloc(shProg, struct gl_shader_variable);
    if (!out)
@@ -3516,8 +3517,29 @@ create_shader_variable(struct gl_shader_program *shProg, const ir_variable *in)
    if (!out->name)
       return NULL;
 
+   /* From the ARB_program_interface_query specification:
+    *
+    * "Not all active variables are assigned valid locations; the
+    *  following variables will have an effective location of -1:
+    *
+    *  * uniforms declared as atomic counters;
+    *
+    *  * members of a uniform block;
+    *
+    *  * built-in inputs, outputs, and uniforms (starting with "gl_"); and
+    *
+    *  * inputs or outputs not declared with a "location" layout qualifier,
+    *    except for vertex shader inputs and fragment shader outputs."
+    */
+   if (in->type->base_type == GLSL_TYPE_ATOMIC_UINT ||
+       is_gl_identifier(in->name) ||
+       !(in->data.explicit_location || use_implicit_location)) {
+      out->location = -1;
+   } else {
+      out->location = in->data.location;
+   }
+
    out->type = in->type;
-   out->location = in->data.location;
    out->index = in->data.index;
    out->patch = in->data.patch;
    out->mode = in->data.mode;
@@ -3563,7 +3585,12 @@ add_interface_variables(struct gl_shader_program *shProg,
       if (strncmp(var->name, "gl_out_FragData", 15) == 0)
          continue;
 
-      gl_shader_variable *sha_v = create_shader_variable(shProg, var);
+      const bool vs_input_or_fs_output =
+         (stage == MESA_SHADER_VERTEX && var->data.mode == ir_var_shader_in) ||
+         (stage == MESA_SHADER_FRAGMENT && var->data.mode == ir_var_shader_out);
+
+      gl_shader_variable *sha_v =
+         create_shader_variable(shProg, var, vs_input_or_fs_output);
       if (!sha_v)
          return false;
 
@@ -3597,7 +3624,8 @@ add_packed_varyings(struct gl_shader_program *shProg, int stage, GLenum type)
          }
 
          if (type == iface) {
-            gl_shader_variable *sha_v = create_shader_variable(shProg, var);
+            gl_shader_variable *sha_v =
+               create_shader_variable(shProg, var, false);
             if (!sha_v)
                return false;
             if (!add_program_resource(shProg, iface, sha_v,
@@ -3622,7 +3650,7 @@ add_fragdata_arrays(struct gl_shader_program *shProg)
       ir_variable *var = node->as_variable();
       if (var) {
          assert(var->data.mode == ir_var_shader_out);
-         gl_shader_variable *sha_v = create_shader_variable(shProg, var);
+         gl_shader_variable *sha_v = create_shader_variable(shProg, var, true);
          if (!sha_v)
             return false;
          if (!add_program_resource(shProg, GL_PROGRAM_OUTPUT, sha_v,

src/mesa/main/shader_query.cpp

@@ -101,24 +101,6 @@ _mesa_BindAttribLocation(GLuint program, GLuint index,
     */
 }
 
-static bool
-is_active_attrib(const gl_shader_variable *var)
-{
-   if (!var)
-      return false;
-
-   switch (var->mode) {
-   case ir_var_shader_in:
-      return var->location != -1;
-
-   case ir_var_system_value:
-      return true;
-
-   default:
-      return false;
-   }
-}
-
 void GLAPIENTRY
 _mesa_GetActiveAttrib(GLuint program, GLuint desired_index,
                       GLsizei maxLength, GLsizei * length, GLint * size,
@@ -159,9 +141,6 @@ _mesa_GetActiveAttrib(GLuint program, GLuint desired_index,
 
    const gl_shader_variable *const var = RESOURCE_VAR(res);
 
-   if (!is_active_attrib(var))
-      return;
-
    const char *var_name = var->name;
 
    _mesa_copy_string(name, maxLength, length, var_name);
@@ -208,19 +187,7 @@ _mesa_GetAttribLocation(GLuint program, const GLchar * name)
    if (!res)
       return -1;
 
-   GLint loc = program_resource_location(shProg, res, name, array_index);
-
-   /* The extra check against against 0 is made because of builtin-attribute
-    * locations that have offset applied. Function program_resource_location
-    * can return built-in attribute locations < 0 and glGetAttribLocation
-    * cannot be used on "conventional" attributes.
-    *
-    * From page 95 of the OpenGL 3.0 spec:
-    *
-    *     "If name is not an active attribute, if name is a conventional
-    *     attribute, or if an error occurs, -1 will be returned."
-    */
-   return (loc >= 0) ? loc : -1;
+   return program_resource_location(shProg, res, name, array_index);
 }
 
 unsigned
@@ -235,8 +202,7 @@ _mesa_count_active_attribs(struct gl_shader_program *shProg)
    unsigned count = 0;
    for (unsigned j = 0; j < shProg->NumProgramResourceList; j++, res++) {
       if (res->Type == GL_PROGRAM_INPUT &&
-          res->StageReferences & (1 << MESA_SHADER_VERTEX) &&
-          is_active_attrib(RESOURCE_VAR(res)))
+          res->StageReferences & (1 << MESA_SHADER_VERTEX))
          count++;
    }
    return count;
@@ -394,19 +360,7 @@ _mesa_GetFragDataLocation(GLuint program, const GLchar *name)
    if (!res)
       return -1;
 
-   GLint loc = program_resource_location(shProg, res, name, array_index);
-
-   /* The extra check against against 0 is made because of builtin-attribute
-    * locations that have offset applied. Function program_resource_location
-    * can return built-in attribute locations < 0 and glGetFragDataLocation
-    * cannot be used on "conventional" attributes.
-    *
-    * From page 95 of the OpenGL 3.0 spec:
-    *
-    *     "If name is not an active attribute, if name is a conventional
-    *     attribute, or if an error occurs, -1 will be returned."
-    */
-   return (loc >= 0) ? loc : -1;
+   return program_resource_location(shProg, res, name, array_index);
 }
 
 const char*
@@ -826,10 +780,6 @@ program_resource_location(struct gl_shader_program *shProg,
                           struct gl_program_resource *res, const char *name,
                           unsigned array_index)
 {
-   /* Built-in locations should report GL_INVALID_INDEX. */
-   if (is_gl_identifier(name))
-      return GL_INVALID_INDEX;
-
    /* VERT_ATTRIB_GENERIC0 and FRAG_RESULT_DATA0 are decremented as these
     * offsets are used internally to differentiate between built-in attributes
     * and user-defined attributes.
@@ -838,6 +788,9 @@ program_resource_location(struct gl_shader_program *shProg,
    case GL_PROGRAM_INPUT: {
       const gl_shader_variable *var = RESOURCE_VAR(res);
 
+      if (var->location == -1)
+         return -1;
+
       /* If the input is an array, fail if the index is out of bounds. */
       if (array_index > 0
           && array_index >= var->type->length) {
@@ -848,6 +801,9 @@ program_resource_location(struct gl_shader_program *shProg,
 	      VERT_ATTRIB_GENERIC0);
    }
    case GL_PROGRAM_OUTPUT:
+      if (RESOURCE_VAR(res)->location == -1)
+         return -1;
+
       /* If the output is an array, fail if the index is out of bounds. */
       if (array_index > 0
           && array_index >= RESOURCE_VAR(res)->type->length) {