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
