[Mesa-dev] [PATCH 09/40] glsl: fix uniform remap table cache when explicit locations used

[Timothy Arceri]

On 10/02/17 21:52, Nicolai Hähnle wrote:

> On 07.02.2017 04:42, Timothy Arceri wrote:
>> From: Timothy Arceri <timothy.arceri at collabora.com>
>>
>> ---
>>  src/compiler/glsl/shader_cache.cpp | 32 
>> +++++++++++++++++++++++++-------
>>  1 file changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/compiler/glsl/shader_cache.cpp 
>> b/src/compiler/glsl/shader_cache.cpp
>> index ba05655..ff8d150 100644
>> --- a/src/compiler/glsl/shader_cache.cpp
>> +++ b/src/compiler/glsl/shader_cache.cpp
>> @@ -57,7 +57,7 @@
>>  #include "main/core.h"
>>  #include "nir.h"
>>  #include "program.h"
>> -#include "util/disk_cache.h"
>> +#include "shader_cache.h"
>>  #include "util/mesa-sha1.h"
>>  #include "util/string_to_uint_map.h"
>>
>> @@ -283,8 +283,20 @@ write_uniform_remap_table(struct blob *metadata,
>>     blob_write_uint32(metadata, prog->NumUniformRemapTable);
>>
>>     for (unsigned i = 0; i < prog->NumUniformRemapTable; i++) {
>> -      blob_write_uint32(metadata, prog->UniformRemapTable[i] -
>> -                           prog->data->UniformStorage);
>> +      blob_write_uint64(metadata,
>> + ptr_to_uint64_t(prog->UniformRemapTable[i]));
>> +
>> +      if (prog->UniformRemapTable[i] != 
>> INACTIVE_UNIFORM_EXPLICIT_LOCATION &&
>> +          prog->UniformRemapTable[i] != NULL) {
>> +
>> +         /* Here we store the offset rather than calculating it on 
>> restore
>> +          * because gl_uniform_storage may have a different size on the
>> +          * platform we are restoring the cache on.
>> +          */
>> +         uint32_t offset =
>> +            prog->UniformRemapTable[i] - prog->data->UniformStorage;
>> +         blob_write_uint32(metadata, offset);
>> +      }
>>     }
>>  }
>>
>> @@ -294,12 +306,18 @@ read_uniform_remap_table(struct blob_reader 
>> *metadata,
>>  {
>>     prog->NumUniformRemapTable = blob_read_uint32(metadata);
>>
>> -   prog->UniformRemapTable =rzalloc_array(prog, struct 
>> gl_uniform_storage *,
>> - prog->NumUniformRemapTable);
>> +   prog->UniformRemapTable = rzalloc_array(prog, struct 
>> gl_uniform_storage *,
>> + prog->NumUniformRemapTable);
>>
>>     for (unsigned i = 0; i < prog->NumUniformRemapTable; i++) {
>> -      prog->UniformRemapTable[i] =
>> -         prog->data->UniformStorage + blob_read_uint32(metadata);
>> +      uint64_t uni_ptr = blob_read_uint64(metadata);
>> +      if (uni_ptr == (uint64_t) INACTIVE_UNIFORM_EXPLICIT_LOCATION ||
>> +          uni_ptr == (uint64_t) NULL) {
>> +         prog->UniformRemapTable[i] = (gl_uniform_storage *) uni_ptr;
>> +      } else {
>> +         uint32_t uni_offset = blob_read_uint32(metadata);
>> +         prog->UniformRemapTable[i] = prog->data->UniformStorage + 
>> uni_offset;
>> +      }
>
> Instead of storing a pointer that isn't used anyway, IMHO it would be 
> better to store an explicit "union tag" enum.

The glUniform* functions check for INACTIVE_UNIFORM_EXPLICIT_LOCATION so 
it's not unused.

>
> Nicolai
>
>>     }
>>  }
>>
>>
>