On Mon, Aug 27, 2012 at 2:48 PM, Kenneth Graunke <kenn...@whitecape.org> wrote:
> On 08/27/2012 10:49 AM, Eric Anholt wrote:
>> Saves 26.5MB of wasted memory allocation in the l4d2 demo.
>> ---
>> src/mesa/drivers/dri/i965/brw_context.h | 15 ++++++++---
>> src/mesa/drivers/dri/i965/brw_fs.cpp | 2 --
>> src/mesa/drivers/dri/i965/brw_state_cache.c | 7 +++++
>> src/mesa/drivers/dri/i965/brw_wm.c | 38
>> +++++++++++++++++++++++++++
>> src/mesa/drivers/dri/i965/brw_wm.h | 1 +
>> 5 files changed, 58 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h
>> b/src/mesa/drivers/dri/i965/brw_context.h
>> index 15ef0a3..c1cd500 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>> +++ b/src/mesa/drivers/dri/i965/brw_context.h
>> @@ -313,11 +313,11 @@ struct brw_wm_prog_data {
>> */
>> uint32_t barycentric_interp_modes;
>>
>> - /* Pointer to tracked values (only valid once
>> + /* Pointers to tracked values (only valid once
>> * _mesa_load_state_parameters has been called at runtime).
>> */
>> - const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */
>> - const float *pull_param[MAX_UNIFORMS * 4];
>> + const float **param;
>> + const float **pull_param;
>> };
>>
>> /**
>> @@ -619,6 +619,7 @@ struct brw_cache_item {
>> };
>>
>>
>> +typedef void (*cache_aux_free_func)(const void *aux);
>>
>> struct brw_cache {
>> struct brw_context *brw;
>> @@ -629,6 +630,14 @@ struct brw_cache {
>>
>> uint32_t next_offset;
>> bool bo_used_by_gpu;
>> +
>> + /**
>> + * Functions used in determining whether the prog_data for a new cache
>> item
>> + * matches an existing cache item. This is sometimes done by memcmp, but
>> + * for some cache ids that would mean statically sizing arrays instead of
>> + * using nice pointers.
>> + */
>
> This comment makes no sense to me. Your newly introduced function is a
> helper to free arrays. I don't see any changes to how key comparisons
> are done (memcmp or something else). I'm kind of scared that this
> comment implies that memcmp doesn't work but I don't see a change to not
> use memcmp.
Yes I think the code is wrong in that respect.
The shader cache key is basically made of strcat(shader, uniforms),
and we match it using a memcmp.
With this change I think there is a way to have two pairs (shader,
uniforms) which have the same memory representation, for example the
same 20 bytes spread among shader/uniforms as follows: shader1 is 8
bytes and uniforms1 are 12 bytes/ shader2 is 12 bytes and uniforms2
are 8 bytes.
This will have the same hash/size and also the same memcmp, so you'll
end up using shader1 instead of shader2.
The only reason it worked before is that the aux size was always constant.
Stéphane
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
