On 4 October 2011 10:54, Ian Romanick <<a href="mailto:idr@freedesktop.org">idr@freedesktop.org</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> <div><div></div><div class="h5">On 10/03/2011 03:11 PM, Paul Berry wrote: </div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div></div><div class="h5"> Previously, brw_compute_vue_map required an argument indicating the number of clip planes in use, but all it did with it was check if it was nonzero. This patch changes brw_compute_vue_map to take a boolean instead. This allows us to avoid some unnecessary recompilation of the Gen4/5 GS and SF threads. --- src/mesa/drivers/dri/i965/brw_clip.c | 2 +- src/mesa/drivers/dri/i965/brw_context.h | 3 ++- src/mesa/drivers/dri/i965/brw_gs.c | 4 ++-- src/mesa/drivers/dri/i965/brw_gs.h | 3 +-- src/mesa/drivers/dri/i965/brw_sf.c | 4 ++-- src/mesa/drivers/dri/i965/brw_sf.h | 3 +-- src/mesa/drivers/dri/i965/brw_vec4_emit.cpp | 2 +- src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 4 ++-- src/mesa/drivers/dri/i965/brw_vs.c | 6 ++++-- src/mesa/drivers/dri/i965/brw_vs.h | 6 ++++++ src/mesa/drivers/dri/i965/brw_vs_emit.c | 10 +++++----- src/mesa/drivers/dri/i965/gen6_sf_state.c | 6 +++--- src/mesa/drivers/dri/i965/gen7_sf_state.c | 4 ++-- 13 files changed, 32 insertions(+), 25 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_clip.c b/src/mesa/drivers/dri/i965/brw_clip.c index 2eb6044..fde3472 100644 --- a/src/mesa/drivers/dri/i965/brw_clip.c +++ b/src/mesa/drivers/dri/i965/brw_clip.c @@ -69,7 +69,7 @@ static void compile_clip_prog( struct brw_context *brw, c.func.single_program_flow = 1; c.key = *key; - brw_compute_vue_map(&c.vue_map, intel, c.key.nr_userclip, c.key.attrs); + brw_compute_vue_map(&c.vue_map, intel, c.key.nr_userclip> 0, c.key.attrs); /* nr_regs is the number of registers filled by reading data from the VUE. * This program accesses the entire VUE, so nr_regs needs to be the size of diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h index 7c5bfd0..c81713a 100644 --- a/src/mesa/drivers/dri/i965/brw_context.h +++ b/src/mesa/drivers/dri/i965/brw_context.h @@ -965,7 +965,8 @@ int brw_disasm (FILE *file, struct brw_instruction *inst, int gen); /* brw_vs.c */ void brw_compute_vue_map(struct brw_vue_map *vue_map, - const struct intel_context *intel, int nr_userclip, + const struct intel_context *intel, + bool userclip_active, GLbitfield64 outputs_written); GLclipplane *brw_select_clip_planes(struct gl_context *ctx); diff --git a/src/mesa/drivers/dri/i965/brw_gs.c b/src/mesa/drivers/dri/i965/brw_gs.c index 0a37485..3b56aae 100644 --- a/src/mesa/drivers/dri/i965/brw_gs.c +++ b/src/mesa/drivers/dri/i965/brw_gs.c @@ -64,7 +64,7 @@ static void compile_gs_prog( struct brw_context *brw, c.key = *key; /* The geometry shader needs to access the entire VUE. */ struct brw_vue_map vue_map; - brw_compute_vue_map(&vue_map, intel, c.key.nr_userclip, c.key.attrs); + brw_compute_vue_map(&vue_map, intel, c.key.userclip_active, c.key.attrs); c.nr_regs = (vue_map.num_slots + 1)/2; mem_ctx = NULL; @@ -159,7 +159,7 @@ static void populate_key( struct brw_context *brw, } /* _NEW_TRANSFORM */ - key->nr_userclip = brw_count_bits(ctx->Transform.ClipPlanesEnabled); + key->userclip_active = (ctx->Transform.ClipPlanesEnabled != 0); key->need_gs_prog = (intel->gen>= 6) ? 0 diff --git a/src/mesa/drivers/dri/i965/brw_gs.h b/src/mesa/drivers/dri/i965/brw_gs.h index d8637c8..cee7467 100644 --- a/src/mesa/drivers/dri/i965/brw_gs.h +++ b/src/mesa/drivers/dri/i965/brw_gs.h @@ -44,8 +44,7 @@ struct brw_gs_prog_key { GLuint primitive:4; GLuint pv_first:1; GLuint need_gs_prog:1; - GLuint nr_userclip:4; - GLuint pad:22; + GLuint userclip_active:1; }; struct brw_gs_compile { diff --git a/src/mesa/drivers/dri/i965/brw_sf.c b/src/mesa/drivers/dri/i965/brw_sf.c index 4e0434a..7a0cd92 100644 --- a/src/mesa/drivers/dri/i965/brw_sf.c +++ b/src/mesa/drivers/dri/i965/brw_sf.c @@ -63,7 +63,7 @@ static void compile_sf_prog( struct brw_context *brw, </div></div> brw_init_compile(brw,&c.func, mem_ctx);<div class="im"> c.key = *key; - brw_compute_vue_map(&c.vue_map, intel, c.key.nr_userclip, c.key.attrs); + brw_compute_vue_map(&c.vue_map, intel, c.key.userclip_active, c.key.attrs); c.urb_entry_read_offset = brw_sf_compute_urb_entry_read_offset(intel); c.nr_attr_regs = (c.vue_map.num_slots + 1)/2 - c.urb_entry_read_offset; c.nr_setup_regs = c.nr_attr_regs; @@ -154,7 +154,7 @@ static void upload_sf_prog(struct brw_context *brw) } /* _NEW_TRANSFORM */ - key.nr_userclip = brw_count_bits(ctx->Transform.ClipPlanesEnabled); + key.userclip_active = (ctx->Transform.ClipPlanesEnabled != 0); /* _NEW_POINT */ key.do_point_sprite = ctx->Point.PointSprite; diff --git a/src/mesa/drivers/dri/i965/brw_sf.h b/src/mesa/drivers/dri/i965/brw_sf.h index f39ad27..8de5f5a 100644 --- a/src/mesa/drivers/dri/i965/brw_sf.h +++ b/src/mesa/drivers/dri/i965/brw_sf.h @@ -53,8 +53,7 @@ struct brw_sf_prog_key { GLuint frontface_ccw:1; GLuint do_point_sprite:1; GLuint sprite_origin_lower_left:1; - GLuint nr_userclip:4; - GLuint pad:20; + GLuint userclip_active:1; </div></blockquote> In the past we've had a bunch of pad elements to document how many bits were left. I've never been too fond of that as it creates extra maintenance. However, others might care, and this change is easy to miss in the rest of the patch. </blockquote><div> FWIW, we discussed this on list last week and both Ken and Eric were in favor of removing them--see <a href="http://lists.freedesktop.org/archives/mesa-dev/2011-September/012630.html">http://lists.freedesktop.org/archives/mesa-dev/2011-September/012630.html</a>. </div></div>