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

[Tapani Pälli]

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.


>
>
>>      delete uniform_map;
>>   }
>>