[Mesa-dev] [PATCH 2/4] i965: Add functions for comparing two brw_wm/vs_prog_data structs.
Kenneth Graunke
kenneth at whitecape.org
Fri Aug 31 20:37:42 PDT 2012
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/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 * sizeof(void *)))
> + return false;
> +
> + if (memcmp(a->pull_param, b->pull_param, a->nr_pull_params * sizeof(void *)))
> + return false;
> +
> + return true;
> +}
>
> static bool
> do_vs_prog(struct brw_context *brw,
> diff --git a/src/mesa/drivers/dri/i965/brw_vs.h b/src/mesa/drivers/dri/i965/brw_vs.h
> index a68a620..8fcdbb3 100644
> --- a/src/mesa/drivers/dri/i965/brw_vs.h
> +++ b/src/mesa/drivers/dri/i965/brw_vs.h
> @@ -123,5 +123,6 @@ bool brw_vs_precompile(struct gl_context *ctx, struct gl_shader_program *prog);
> void brw_vs_debug_recompile(struct brw_context *brw,
> struct gl_shader_program *prog,
> const struct brw_vs_prog_key *key);
> +bool brw_vs_prog_data_compare(void *a, void *b, int aux_size, void *key);
>
> #endif
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
> index 7ba1337..f16bed6 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm.c
> @@ -253,6 +253,25 @@ brw_wm_payload_setup(struct brw_context *brw,
> }
> }
>
> +bool
> +brw_wm_prog_data_compare(void *in_a, void *in_b, int aux_size, void *in_key)
> +{
> + struct brw_wm_prog_data *a = in_a;
> + struct brw_wm_prog_data *b = in_b;
> +
> + /* Compare all the struct up to the pointers. */
> + if (memcmp(a, b, offsetof(struct brw_wm_prog_data, param)))
> + return false;
> +
> + if (memcmp(a->param, b->param, a->nr_params * sizeof(void *)))
> + return false;
> +
> + if (memcmp(a->pull_param, b->pull_param, a->nr_pull_params * sizeof(void *)))
> + return false;
> +
> + return true;
> +}
> +
> /**
> * All Mesa program -> GPU code generation goes through this function.
> * Depending on the instructions used (i.e. flow control instructions)
> diff --git a/src/mesa/drivers/dri/i965/brw_wm.h b/src/mesa/drivers/dri/i965/brw_wm.h
> index 2938449..1f61412 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm.h
> +++ b/src/mesa/drivers/dri/i965/brw_wm.h
> @@ -484,5 +484,6 @@ bool do_wm_prog(struct brw_context *brw,
> void brw_wm_debug_recompile(struct brw_context *brw,
> struct gl_shader_program *prog,
> const struct brw_wm_prog_key *key);
> +bool brw_wm_prog_data_compare(void *a, void *b, int aux_size, void *key);
>
> #endif
>
More information about the mesa-dev
mailing list