[Mesa-dev] [WIP 04/13] mesa: add a storage for inactive/removed uniform variables

[Tapani Pälli]

On 04/05/2014 01:13 AM, Ian Romanick wrote:
> I've been thinking about this a bit, and I think this is more complex
> than it needs to be.  I don't have a complete design, but here are some
> thoughts on the matter.
>
> When linking is all done, we just want the UniformRemapTable.  It's
> entires should have one of three kinds of values:
>
>  - NULL: Invalid entry.  Any function that gets this as a uniform handle
> should generate an error.
>
>  - (intptr_t)-1: Explicitly assigned location for a uniform that was
> eliminated.  Any function that gets this as a uniform handle should exit
> without error.
>
>  - Anything else: Valid.  No changes need to any of the functions that
> operate on uniforms.
>
> At the beginning of linking, we can prime the table.  Make a pass over
> all the uniforms in all the shaders.  Set elements in the table that
> correspond to the explicit location to (intptr_t)-1.  Set every other
> location to NULL.  A later pass in linking can set the active slots to
> the "real" values.

ok, I'll go this way instead.

> The timing of building the initial table is important.  Optimization
> passes in the linker can trim arrays, and we want the remap slots for
> the trimmed elements to get (intptr_t)-1.

Yes, this needs to happen before any optimizations.

> To further simplify things, I think we can put all the uniforms that
> lack explicit locations after the last explicit location.  If we want to
> pack non-explicit location uniforms in among the explicit location
> uniforms, I think we'll want a different data structure to track the
> avaialble ranges.  I don't know how often explicit/non-explicit mixing
> will occur in real apps.

I was playing around with a version that has a 'list of free location
spans' but that felt a bit too complex structure for a simple task. I
don't think location assignment itself can become a performance
bottleneck with my current implementation.

I can move them after explicit ones. We will have this theoretical
corner case though where app decides to use the last location and still
has some uniforms without explicit location, linker will have to be able
to fail or assign locations like I currently do in this case.


> On 03/27/2014 11:45 PM, Tapani Pälli wrote:
>> GL_ARB_explicit_uniform_location extension requires driver to reserve
>> locations for all uniforms (including inactive ones) when they have
>> explicit location set. Patch adds additional storage structure and a
>> list of locations reserved by inactive uniforms as part of the shader
>> program structure.
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> ---
>>  src/mesa/main/mtypes.h    | 13 +++++++++++++
>>  src/mesa/main/shaderobj.c |  7 +++++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
>> index c8b8fa7..87d69c1 100644
>> --- a/src/mesa/main/mtypes.h
>> +++ b/src/mesa/main/mtypes.h
>> @@ -2710,6 +2710,19 @@ struct gl_shader_program
>>     struct gl_uniform_storage **UniformRemapTable;
>>  
>>     /**
>> +    * This list contains explicit locations defined by inactive uniforms that
>> +    * were removed during the optimization pass (by opt_dead_code). This is
>> +    * used by linker to be able to still preserve locations for those
>> +    * uniforms.
>> +    *
>> +    * All removed uniforms have common storage 'RemovedUniformStorage' which
>> +    * is allocated once using ReservedUniformLocations as the ralloc context.
>> +    */
>> +   unsigned *ReservedUniformLocations;
>> +   unsigned NumReservedLocations;
>> +   struct gl_uniform_storage *RemovedUniformStorage;
>> +
>> +   /**
>>      * Size of the gl_ClipDistance array that is output from the last pipeline
>>      * stage before the fragment shader.
>>      */
>> diff --git a/src/mesa/main/shaderobj.c b/src/mesa/main/shaderobj.c
>> index b0f0bfa..7ec2827 100644
>> --- a/src/mesa/main/shaderobj.c
>> +++ b/src/mesa/main/shaderobj.c
>> @@ -287,6 +287,13 @@ _mesa_clear_shader_program_data(struct gl_context *ctx,
>>        shProg->UniformStorage = NULL;
>>     }
>>  
>> +   if (shProg->ReservedUniformLocations) {
>> +      ralloc_free(shProg->ReservedUniformLocations);
>> +      shProg->NumReservedLocations = 0;
>> +      shProg->ReservedUniformLocations = NULL;
>> +      shProg->RemovedUniformStorage = NULL;
>> +   }
>> +
>>     if (shProg->UniformRemapTable) {
>>        ralloc_free(shProg->UniformRemapTable);
>>        shProg->NumUniformRemapTable = 0;
>>