Re: [Mesa-dev] [PATCH 2/4] i965: Add functions for comparing two brw_wm/vs_prog_data structs.
Kenneth Graunke writes: > On 08/31/2012 03:05 PM, Eric Anholt wrote: >> Currently, this just avoids comparing all unused parts of param[] and >> pull_param[], but it's a step toward getting rid of those giant statically >> sized arrays. >> --- >> src/mesa/drivers/dri/i965/brw_context.h | 29 >> ++- >> src/mesa/drivers/dri/i965/brw_state_cache.c | 10 - >> src/mesa/drivers/dri/i965/brw_vs.c | 18 + >> src/mesa/drivers/dri/i965/brw_vs.h |1 + >> src/mesa/drivers/dri/i965/brw_wm.c | 19 ++ >> src/mesa/drivers/dri/i965/brw_wm.h |1 + >> 6 files changed, 72 insertions(+), 6 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_context.h >> b/src/mesa/drivers/dri/i965/brw_context.h >> index 71cd0f4..6cc567c 100644 >> --- a/src/mesa/drivers/dri/i965/brw_context.h >> +++ b/src/mesa/drivers/dri/i965/brw_context.h >> @@ -288,7 +288,10 @@ struct brw_shader_program { >> /* Data about a particular attempt to compile a program. Note that >> * there can be many of these, each in a different GL state >> * corresponding to a different brw_wm_prog_key struct, with different >> - * compiled programs: >> + * compiled programs. >> + * >> + * Note: brw_wm_prog_data_compare() must be updated when adding fields to >> this >> + * struct! >> */ >> struct brw_wm_prog_data { >> GLuint curb_read_length; >> @@ -313,8 +316,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). >> +* >> +* These must be the last fields of the struct (see >> +* brw_wm_prog_data_compare()). >> */ >> const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ >> const float *pull_param[MAX_UNIFORMS * 4]; >> @@ -426,6 +432,9 @@ struct brw_gs_prog_data { >> unsigned svbi_postincrement_value; >> }; >> >> +/* Note: brw_vs_prog_data_compare() must be updated when adding fields to >> this >> + * struct! >> + */ >> struct brw_vs_prog_data { >> struct brw_vue_map vue_map; >> >> @@ -443,14 +452,15 @@ struct brw_vs_prog_data { >> */ >> GLuint urb_entry_size; >> >> - const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ >> - const float *pull_param[MAX_UNIFORMS * 4]; >> - >> bool uses_new_param_layout; >> bool uses_vertexid; >> bool userclip; >> >> int num_surfaces; >> + >> + /* These pointers must appear last. See brw_vs_prog_data_compare(). */ >> + const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ >> + const float *pull_param[MAX_UNIFORMS * 4]; >> }; >> >> >> @@ -619,6 +629,8 @@ struct brw_cache_item { >> }; >> >> >> +typedef bool (*cache_aux_compare_func)(void *a, void *b, int aux_size, >> + void *key); >> >> struct brw_cache { >> struct brw_context *brw; >> @@ -629,6 +641,13 @@ struct brw_cache { >> >> uint32_t next_offset; >> bool bo_used_by_gpu; >> + >> + /** >> +* Optional functions used in determining whether the prog_data for a new >> +* cache item matches an existing cache item (in case there's relevant >> data >> +* outside of the prog_data). If NULL, a plain memcmp is done. >> +*/ >> + cache_aux_compare_func aux_compare[BRW_MAX_CACHE]; >> }; >> >> >> diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c >> b/src/mesa/drivers/dri/i965/brw_state_cache.c >> index 8823b22..354d94b 100644 >> --- a/src/mesa/drivers/dri/i965/brw_state_cache.c >> +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c >> @@ -47,6 +47,8 @@ >> #include "main/imports.h" >> #include "intel_batchbuffer.h" >> #include "brw_state.h" >> +#include "brw_vs.h" >> +#include "brw_wm.h" >> >> #define FILE_DEBUG_FLAG DEBUG_STATE >> >> @@ -211,7 +213,10 @@ brw_try_upload_using_copy(struct brw_cache *cache, >> continue; >> } >> >> - if (memcmp(item_aux, aux, item->aux_size) != 0) { >> + if (cache->aux_compare[result_item->cache_id]) { >> +if (!cache->aux_compare[result_item->cache_id]) >> + continue; > > It sure doesn't look like you're calling the function...just checking > > if (function_pointer) { >if (!function_pointer) > continue; > } > > which is really not what you want :) > > Otherwise this looks good. Thanks. pgpCqTOzwsnyR.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] i965: Add functions for comparing two brw_wm/vs_prog_data structs.
On 08/31/2012 03:05 PM, Eric Anholt wrote: > Currently, this just avoids comparing all unused parts of param[] and > pull_param[], but it's a step toward getting rid of those giant statically > sized arrays. > --- > src/mesa/drivers/dri/i965/brw_context.h | 29 > ++- > src/mesa/drivers/dri/i965/brw_state_cache.c | 10 - > src/mesa/drivers/dri/i965/brw_vs.c | 18 + > src/mesa/drivers/dri/i965/brw_vs.h |1 + > src/mesa/drivers/dri/i965/brw_wm.c | 19 ++ > src/mesa/drivers/dri/i965/brw_wm.h |1 + > 6 files changed, 72 insertions(+), 6 deletions(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.h > b/src/mesa/drivers/dri/i965/brw_context.h > index 71cd0f4..6cc567c 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.h > +++ b/src/mesa/drivers/dri/i965/brw_context.h > @@ -288,7 +288,10 @@ struct brw_shader_program { > /* Data about a particular attempt to compile a program. Note that > * there can be many of these, each in a different GL state > * corresponding to a different brw_wm_prog_key struct, with different > - * compiled programs: > + * compiled programs. > + * > + * Note: brw_wm_prog_data_compare() must be updated when adding fields to > this > + * struct! > */ > struct brw_wm_prog_data { > GLuint curb_read_length; > @@ -313,8 +316,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). > +* > +* These must be the last fields of the struct (see > +* brw_wm_prog_data_compare()). > */ > const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ > const float *pull_param[MAX_UNIFORMS * 4]; > @@ -426,6 +432,9 @@ struct brw_gs_prog_data { > unsigned svbi_postincrement_value; > }; > > +/* Note: brw_vs_prog_data_compare() must be updated when adding fields to > this > + * struct! > + */ > struct brw_vs_prog_data { > struct brw_vue_map vue_map; > > @@ -443,14 +452,15 @@ struct brw_vs_prog_data { > */ > GLuint urb_entry_size; > > - const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ > - const float *pull_param[MAX_UNIFORMS * 4]; > - > bool uses_new_param_layout; > bool uses_vertexid; > bool userclip; > > int num_surfaces; > + > + /* These pointers must appear last. See brw_vs_prog_data_compare(). */ > + const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ > + const float *pull_param[MAX_UNIFORMS * 4]; > }; > > > @@ -619,6 +629,8 @@ struct brw_cache_item { > }; > > > +typedef bool (*cache_aux_compare_func)(void *a, void *b, int aux_size, > + void *key); > > struct brw_cache { > struct brw_context *brw; > @@ -629,6 +641,13 @@ struct brw_cache { > > uint32_t next_offset; > bool bo_used_by_gpu; > + > + /** > +* Optional functions used in determining whether the prog_data for a new > +* cache item matches an existing cache item (in case there's relevant > data > +* outside of the prog_data). If NULL, a plain memcmp is done. > +*/ > + cache_aux_compare_func aux_compare[BRW_MAX_CACHE]; > }; > > > diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c > b/src/mesa/drivers/dri/i965/brw_state_cache.c > index 8823b22..354d94b 100644 > --- a/src/mesa/drivers/dri/i965/brw_state_cache.c > +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c > @@ -47,6 +47,8 @@ > #include "main/imports.h" > #include "intel_batchbuffer.h" > #include "brw_state.h" > +#include "brw_vs.h" > +#include "brw_wm.h" > > #define FILE_DEBUG_FLAG DEBUG_STATE > > @@ -211,7 +213,10 @@ brw_try_upload_using_copy(struct brw_cache *cache, > continue; >} > > - if (memcmp(item_aux, aux, item->aux_size) != 0) { > + if (cache->aux_compare[result_item->cache_id]) { > +if (!cache->aux_compare[result_item->cache_id]) > + continue; It sure doesn't look like you're calling the function...just checking if (function_pointer) { if (!function_pointer) continue; } which is really not what you want :) Otherwise this looks good. > + } else if (memcmp(item_aux, aux, item->aux_size) != 0) { > continue; >} > > @@ -333,6 +338,9 @@ brw_init_caches(struct brw_context *brw) > cache->bo = drm_intel_bo_alloc(intel->bufmgr, > "program cache", > 4096, 64); > + > + cache->aux_compare[BRW_VS_PROG] = brw_vs_prog_data_compare; > + cache->aux_compare[BRW_WM_PROG] = brw_wm_prog_data_compare; > } > > static void > diff --git a/src/mesa/drivers/dri/i965/brw_vs.c > b/src/mesa/drivers/dri/i965/brw_vs.c > index 710ffe8..a54999d 100644 > --- a/src/mesa/drivers/
[Mesa-dev] [PATCH 2/4] i965: Add functions for comparing two brw_wm/vs_prog_data structs.
Currently, this just avoids comparing all unused parts of param[] and pull_param[], but it's a step toward getting rid of those giant statically sized arrays. --- src/mesa/drivers/dri/i965/brw_context.h | 29 ++- src/mesa/drivers/dri/i965/brw_state_cache.c | 10 - src/mesa/drivers/dri/i965/brw_vs.c | 18 + src/mesa/drivers/dri/i965/brw_vs.h |1 + src/mesa/drivers/dri/i965/brw_wm.c | 19 ++ src/mesa/drivers/dri/i965/brw_wm.h |1 + 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index 71cd0f4..6cc567c 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -288,7 +288,10 @@ struct brw_shader_program { /* Data about a particular attempt to compile a program. Note that * there can be many of these, each in a different GL state * corresponding to a different brw_wm_prog_key struct, with different - * compiled programs: + * compiled programs. + * + * Note: brw_wm_prog_data_compare() must be updated when adding fields to this + * struct! */ struct brw_wm_prog_data { GLuint curb_read_length; @@ -313,8 +316,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). +* +* These must be the last fields of the struct (see +* brw_wm_prog_data_compare()). */ const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ const float *pull_param[MAX_UNIFORMS * 4]; @@ -426,6 +432,9 @@ struct brw_gs_prog_data { unsigned svbi_postincrement_value; }; +/* Note: brw_vs_prog_data_compare() must be updated when adding fields to this + * struct! + */ struct brw_vs_prog_data { struct brw_vue_map vue_map; @@ -443,14 +452,15 @@ struct brw_vs_prog_data { */ GLuint urb_entry_size; - const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ - const float *pull_param[MAX_UNIFORMS * 4]; - bool uses_new_param_layout; bool uses_vertexid; bool userclip; int num_surfaces; + + /* These pointers must appear last. See brw_vs_prog_data_compare(). */ + const float *param[MAX_UNIFORMS * 4]; /* should be: BRW_MAX_CURBE */ + const float *pull_param[MAX_UNIFORMS * 4]; }; @@ -619,6 +629,8 @@ struct brw_cache_item { }; +typedef bool (*cache_aux_compare_func)(void *a, void *b, int aux_size, + void *key); struct brw_cache { struct brw_context *brw; @@ -629,6 +641,13 @@ struct brw_cache { uint32_t next_offset; bool bo_used_by_gpu; + + /** +* Optional functions used in determining whether the prog_data for a new +* cache item matches an existing cache item (in case there's relevant data +* outside of the prog_data). If NULL, a plain memcmp is done. +*/ + cache_aux_compare_func aux_compare[BRW_MAX_CACHE]; }; diff --git a/src/mesa/drivers/dri/i965/brw_state_cache.c b/src/mesa/drivers/dri/i965/brw_state_cache.c index 8823b22..354d94b 100644 --- a/src/mesa/drivers/dri/i965/brw_state_cache.c +++ b/src/mesa/drivers/dri/i965/brw_state_cache.c @@ -47,6 +47,8 @@ #include "main/imports.h" #include "intel_batchbuffer.h" #include "brw_state.h" +#include "brw_vs.h" +#include "brw_wm.h" #define FILE_DEBUG_FLAG DEBUG_STATE @@ -211,7 +213,10 @@ brw_try_upload_using_copy(struct brw_cache *cache, continue; } -if (memcmp(item_aux, aux, item->aux_size) != 0) { + if (cache->aux_compare[result_item->cache_id]) { +if (!cache->aux_compare[result_item->cache_id]) + continue; + } else if (memcmp(item_aux, aux, item->aux_size) != 0) { continue; } @@ -333,6 +338,9 @@ brw_init_caches(struct brw_context *brw) cache->bo = drm_intel_bo_alloc(intel->bufmgr, "program cache", 4096, 64); + + cache->aux_compare[BRW_VS_PROG] = brw_vs_prog_data_compare; + cache->aux_compare[BRW_WM_PROG] = brw_wm_prog_data_compare; } static void diff --git a/src/mesa/drivers/dri/i965/brw_vs.c b/src/mesa/drivers/dri/i965/brw_vs.c index 710ffe8..a54999d 100644 --- a/src/mesa/drivers/dri/i965/brw_vs.c +++ b/src/mesa/drivers/dri/i965/brw_vs.c @@ -186,6 +186,24 @@ gl_clip_plane *brw_select_clip_planes(struct gl_context *ctx) } } +bool +brw_vs_prog_data_compare(void *in_a, void *in_b, int aux_size, void *in_key) +{ + struct brw_vs_prog_data *a = in_a; + struct brw_vs_prog_data *b = in_b; + + /* Compare all the struct up to the pointers. */ + if (memcmp(a, b, offsetof(struct brw_vs_prog_data, param))) + return false; + + if (memcmp(a->param, b->param, a->nr_params * sizeo