On 4 October 2011 10:54, Ian Romanick <i...@freedesktop.org> wrote:
> On 10/03/2011 03:11 PM, Paul Berry wrote:
>
>> 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,
>> brw_init_compile(brw,&c.func, mem_ctx);
>>
>>
>> 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;
>>
>
> 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.
>
FWIW, we discussed this on list last week and both Ken and Eric were in
favor of removing them--see
http://lists.freedesktop.org/archives/mesa-dev/2011-September/012630.html.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
