[Mesa-dev] [PATCH 4/6] i965: Make brw_compute_vue_map's userclip dependency a boolean.

Paul Berry stereotype441 at gmail.com
Tue Oct 4 11:29:01 PDT 2011


On 4 October 2011 10:54, Ian Romanick <idr at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20111004/b60c1a38/attachment.html>


More information about the mesa-dev mailing list