[Mesa-dev] [PATCH] glsl: track total amount of uniform locations used

[Tapani Pälli]

On 01/08/2016 11:32 AM, Tapani Pälli wrote:
>
>
> On 01/08/2016 11:17 AM, Timothy Arceri wrote:
>> On Fri, 2016-01-08 at 08:20 +0200, Tapani Pälli wrote:
>>> Linker missed a check for situation where we exceed max amount of
>>> uniform locations with explicit + implicit locations. Patch adds this
>>> check to already existing iteration over uniforms in linker.
>>>
>>> Fixes following CTS test:
>>>     ES31-CTS.explicit_uniform_location.uniform-loc-negative-link-max
>>> -num-of-locations
>>>
>>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>>> ---
>>>   src/glsl/linker.cpp | 17 +++++++++++++++--
>>>   1 file changed, 15 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
>>> index 7a18523..ef23b36 100644
>>> --- a/src/glsl/linker.cpp
>>> +++ b/src/glsl/linker.cpp
>>> @@ -3130,6 +3130,7 @@ check_explicit_uniform_locations(struct
>>> gl_context *ctx,
>>>         return;
>>>      }
>>>
>>> +   unsigned entries_total = 0;
>>>      for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
>>>         struct gl_shader *sh = prog->_LinkedShaders[i];
>>>
>>> @@ -3138,8 +3139,12 @@ check_explicit_uniform_locations(struct
>>> gl_context *ctx,
>>>
>>>         foreach_in_list(ir_instruction, node, sh->ir) {
>>>            ir_variable *var = node->as_variable();
>>> -         if (var && (var->data.mode == ir_var_uniform &&
>>> -                     var->data.explicit_location)) {
>>> +         if (!var || var->data.mode != ir_var_uniform)
>>> +            continue;
>>> +
>>> +         entries_total += var->type->is_array() ? var->type->length
>>> : 1;
>>> +
>>> +         if (var->data.explicit_location) {
>>>               bool ret;
>>>               if (var->type->is_subroutine())
>>>                  ret = reserve_subroutine_explicit_locations(prog, sh,
>>> var);
>>> @@ -3153,6 +3158,14 @@ check_explicit_uniform_locations(struct
>>> gl_context *ctx,
>>>         }
>>>      }
>>>
>>> +   /* Verify that total amount of entries for explicit and implicit
>>> locations
>>> +    * is less than MAX_UNIFORM_LOCATIONS.
>>> +    */
>>> +   if (entries_total >= ctx
>>> ->Const.MaxUserAssignableUniformLocations) {
>>> +      linker_error(prog, "count of uniform locations >=
>>> MAX_UNIFORM_LOCATIONS"
>>> +                   "(%u >= %u)", entries_total,
>>> +                   ctx->Const.MaxUserAssignableUniformLocations);
>>> +   }
>>
>> I think this check would be better in link_assign_uniform_locations()
>> because check_explicit_uniform_locations() is called before arrays are
>> resized via update_array_sizes()
>>
>> Also in link_assign_uniform_locations() there is already a count of
>> uniform locations.
>>
>> See: const unsigned num_uniforms = uniform_size.num_active_uniforms;
>
> That function can't currently fail and it does not know the max no of
> uniforms so it was easier to plug here. I can move it there but it
> requires changing the function signature a bit, I guess not that big
> deal though.

I've made a version that counts used locations only at 
link_assign_uniform_locations(). Problem with that is that it happens 
too late. With the test case an uniform array gets optimized away (since 
it's not used in the shader, however as it is using explicit location, 
those locations must be reserved), therefore this happens too late. IMO 
we must do this calculation as the my first proposal did it.


>
>>
>>
>>>      delete uniform_map;
>>>   }
>>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev