[Mesa-dev] Mesa (master): draw: Only run prepare when state, prim and opt changes

Stéphane Marchesin stephane.marchesin at gmail.com
Thu Jan 26 19:15:21 PST 2012


I just took a look at it in gdb. Basically the jit_func pointer is
corrupted by the free_gallivm_state function (in lp_bld_init.c). There
is a comment to that effect already. It seems like the bug was always
there but hidden because we regenerated state more than we had to.
I'll keep digging...

Stéphane


2012/1/26 Stéphane Marchesin <stephane.marchesin at gmail.com>:
> Hmm, I'll take a look later today.
>
> Stéphane
>
> 2012/1/26 Jose Fonseca <jfonseca at vmware.com>:
>> Stephane,
>>
>> This commit caused a segmentation fault on glean texSwizzle test + llvmpipe:
>>
>> $ gdb --args glean --run results --overwrite --quick --tests texSwizzle
>> (gdb) r
>> Starting program: glean --run results --overwrite --quick --tests texSwizzle
>> [Thread debugging using libthread_db enabled]
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0xfffffffffffffffc in ?? ()
>> (gdb) bt
>> #0  0xfffffffffffffffc in ?? ()
>> #1  0x00007ffff6a26438 in llvm_pipeline_generic (middle=0x76e4a0, fetch_info=0x7fffffffd730, prim_info=0x7fffffffd700)
>>    at src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c:240
>> #2  0x00007ffff6a266fe in llvm_middle_end_linear_run (middle=0x76e4a0, start=0, count=4, prim_flags=0)
>>    at src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c:358
>> #3  0x00007ffff697bf23 in vsplit_segment_simple_linear (vsplit=0x76b670, flags=0, istart=0, icount=4) at src/gallium/auxiliary/draw/draw_pt_vsplit_tmp.h:237
>> #4  0x00007ffff697c228 in vsplit_run_linear (frontend=0x76b670, start=0, count=4) at src/gallium/auxiliary/draw/draw_split_tmp.h:61
>> #5  0x00007ffff697224e in draw_pt_arrays (draw=0x762510, prim=6, start=0, count=4) at src/gallium/auxiliary/draw/draw_pt.c:142
>> #6  0x00007ffff6972eb1 in draw_vbo (draw=0x762510, info=0x7fffffffd910) at src/gallium/auxiliary/draw/draw_pt.c:534
>> #7  0x00007ffff6689f67 in llvmpipe_draw_vbo (pipe=0x72fa10, info=0x7fffffffd910) at src/gallium/drivers/llvmpipe/lp_draw_arrays.c:85
>> #8  0x00007ffff68037f4 in st_draw_vbo (ctx=0x7c4b30, arrays=0x831c88, prims=0x7fffffffd9e0, nr_prims=1, ib=0x0, index_bounds_valid=1 '\001', min_index=0,
>>    max_index=3, tfb_vertcount=0x0) at src/mesa/state_tracker/st_draw.c:1113
>> #9  0x00007ffff689d811 in vbo_draw_arrays (ctx=0x7c4b30, mode=6, start=0, count=4, numInstances=1) at src/mesa/vbo/vbo_exec_array.c:635
>> #10 0x00007ffff689d950 in vbo_exec_DrawArrays (mode=6, start=0, count=4) at src/mesa/vbo/vbo_exec_array.c:667
>> #11 0x0000000000458205 in GLEAN::TexSwizzleTest::TestSwizzles (this=0x6f10e0) at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/ttexswizzle.cpp:293
>> #12 0x0000000000458558 in GLEAN::TexSwizzleTest::runOne (this=0x6f10e0, r=..., w=<optimized out>)
>>    at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/ttexswizzle.cpp:387
>> #13 0x0000000000458ec7 in GLEAN::BaseTest<GLEAN::TexSwizzleResult>::run (this=0x6f10e0, environment=<optimized out>)
>>    at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/tbase.h:317
>> #14 0x00000000004610b8 in main (argc=7, argv=0x7fffffffdec8) at /var/lib/hudson/jobs/glean-ubuntu64/workspace/src/glean/main.cpp:140
>> (gdb)
>>
>> Can you look into it?
>>
>> Jose
>>
>>
>> ----- Original Message -----
>>> Module: Mesa
>>> Branch: master
>>> Commit: b6d3a435a0e0e53a9e8cc4c4249dc7c2f897a83d
>>> URL:
>>>    http://cgit.freedesktop.org/mesa/mesa/commit/?id=b6d3a435a0e0e53a9e8cc4c4249dc7c2f897a83d
>>>
>>> Author: Jakob Bornecrantz <wallbraker at gmail.com>
>>> Date:   Mon Jan 24 02:11:59 2011 +0100
>>>
>>> draw: Only run prepare when state, prim and opt changes
>>>
>>> In bad applications like ipers which does a lot of draw calls with
>>> no state changes this helps to greatly reduce time spent in prepare.
>>> In ipers around 7% of CPU was spent in various prepare functions,
>>> after this commit no prepare function show on the profile.
>>>
>>> This commit also has the added benefit of now grouping all pipelined
>>> drawing into a single draw call if the driver uses vbuf_render.
>>>
>>> Reviewed-by: Stéphane Marchesin <marcheu at chromium.org>
>>> Tested-by: Stéphane Marchesin <marcheu at chromium.org>
>>>
>>> ---
>>>
>>>  src/gallium/auxiliary/draw/draw_context.c   |    6 +++
>>>  src/gallium/auxiliary/draw/draw_private.h   |    8 ++++
>>>  src/gallium/auxiliary/draw/draw_pt.c        |   49
>>>  ++++++++++++++++++++++++---
>>>  src/gallium/auxiliary/draw/draw_pt.h        |    2 +-
>>>  src/gallium/auxiliary/draw/draw_pt_vsplit.c |   11 ++++--
>>>  5 files changed, 66 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/draw/draw_context.c
>>> b/src/gallium/auxiliary/draw/draw_context.c
>>> index 4ce4445..3c0b1aa 100644
>>> --- a/src/gallium/auxiliary/draw/draw_context.c
>>> +++ b/src/gallium/auxiliary/draw/draw_context.c
>>> @@ -355,6 +355,10 @@ draw_set_vertex_elements(struct draw_context
>>> *draw,
>>>  {
>>>     assert(count <= PIPE_MAX_ATTRIBS);
>>>
>>> +   /* We could improve this by only flushing the frontend and the
>>> fetch part
>>> +    * of the middle. This would avoid recalculating the emit keys.*/
>>> +   draw_do_flush( draw, DRAW_FLUSH_STATE_CHANGE );
>>> +
>>>     memcpy(draw->pt.vertex_element, elements, count *
>>>     sizeof(elements[0]));
>>>     draw->pt.nr_vertex_elements = count;
>>>  }
>>> @@ -654,6 +658,8 @@ void draw_do_flush( struct draw_context *draw,
>>> unsigned flags )
>>>
>>>        draw_pipeline_flush( draw, flags );
>>>
>>> +      draw_pt_flush( draw, flags );
>>> +
>>>        draw->flushing = FALSE;
>>>     }
>>>  }
>>> diff --git a/src/gallium/auxiliary/draw/draw_private.h
>>> b/src/gallium/auxiliary/draw/draw_private.h
>>> index 1a0286d..c3eca97 100644
>>> --- a/src/gallium/auxiliary/draw/draw_private.h
>>> +++ b/src/gallium/auxiliary/draw/draw_private.h
>>> @@ -63,6 +63,7 @@ struct draw_stage;
>>>  struct vbuf_render;
>>>  struct tgsi_exec_machine;
>>>  struct tgsi_sampler;
>>> +struct draw_pt_front_end;
>>>
>>>
>>>  /**
>>> @@ -137,6 +138,12 @@ struct draw_context
>>>     /* Support prototype passthrough path:
>>>      */
>>>     struct {
>>> +      /* Current active frontend */
>>> +      struct draw_pt_front_end *frontend;
>>> +      unsigned prim;
>>> +      unsigned opt;
>>> +      unsigned eltSize; /* saved eltSize for flushing */
>>> +
>>>        struct {
>>>           struct draw_pt_middle_end *fetch_emit;
>>>           struct draw_pt_middle_end *fetch_shade_emit;
>>> @@ -391,6 +398,7 @@ void draw_remove_extra_vertex_attribs(struct
>>> draw_context *draw);
>>>  boolean draw_pt_init( struct draw_context *draw );
>>>  void draw_pt_destroy( struct draw_context *draw );
>>>  void draw_pt_reset_vertex_ids( struct draw_context *draw );
>>> +void draw_pt_flush( struct draw_context *draw, unsigned flags );
>>>
>>>
>>>  /*******************************************************************************
>>> diff --git a/src/gallium/auxiliary/draw/draw_pt.c
>>> b/src/gallium/auxiliary/draw/draw_pt.c
>>> index 9a017fd..025d539 100644
>>> --- a/src/gallium/auxiliary/draw/draw_pt.c
>>> +++ b/src/gallium/auxiliary/draw/draw_pt.c
>>> @@ -52,7 +52,7 @@ DEBUG_GET_ONCE_BOOL_OPTION(draw_no_fse,
>>> "DRAW_NO_FSE", FALSE)
>>>   *     - backend  -- the vbuf_render provided by the driver.
>>>   */
>>>  static boolean
>>> -draw_pt_arrays(struct draw_context *draw,
>>> +draw_pt_arrays(struct draw_context *draw,
>>>                 unsigned prim,
>>>                 unsigned start,
>>>                 unsigned count)
>>> @@ -106,17 +106,56 @@ draw_pt_arrays(struct draw_context *draw,
>>>           middle = draw->pt.middle.general;
>>>     }
>>>
>>> -   frontend = draw->pt.front.vsplit;
>>> +   frontend = draw->pt.frontend;
>>> +
>>> +   if (frontend ) {
>>> +      if (draw->pt.prim != prim || draw->pt.opt != opt) {
>>> +         /* In certain conditions switching primitives requires us
>>> to flush
>>> +          * and validate the different stages. One example is when
>>> smooth
>>> +          * lines are active but first drawn with triangles and then
>>> with
>>> +          * lines.
>>> +          */
>>> +         draw_do_flush( draw, DRAW_FLUSH_STATE_CHANGE );
>>> +         frontend = NULL;
>>> +      } else if (draw->pt.eltSize != draw->pt.user.eltSize) {
>>> +         /* Flush draw state if eltSize changed.
>>> +          * This could be improved so only the frontend is flushed
>>> since it
>>> +          * converts all indices to ushorts and the fetch part of
>>> the middle
>>> +          * always perpares both linear and indexed.
>>> +          */
>>> +         frontend->flush( frontend, DRAW_FLUSH_STATE_CHANGE );
>>> +         frontend = NULL;
>>> +      }
>>> +   }
>>>
>>> -   frontend->prepare( frontend, prim, middle, opt );
>>> +   if (!frontend) {
>>> +      frontend = draw->pt.front.vsplit;
>>>
>>> -   frontend->run(frontend, start, count);
>>> +      frontend->prepare( frontend, prim, middle, opt );
>>>
>>> -   frontend->finish( frontend );
>>> +      draw->pt.frontend = frontend;
>>> +      draw->pt.eltSize = draw->pt.user.eltSize;
>>> +      draw->pt.prim = prim;
>>> +      draw->pt.opt = opt;
>>> +   }
>>> +
>>> +   frontend->run( frontend, start, count );
>>>
>>>     return TRUE;
>>>  }
>>>
>>> +void draw_pt_flush( struct draw_context *draw, unsigned flags )
>>> +{
>>> +   if (draw->pt.frontend) {
>>> +      draw->pt.frontend->flush( draw->pt.frontend, flags );
>>> +
>>> +      /* don't prepare if we only are flushing the backend */
>>> +      if (!(flags & DRAW_FLUSH_BACKEND))
>>> +         draw->pt.frontend = NULL;
>>> +   }
>>> +}
>>> +
>>> +
>>>
>>>  boolean draw_pt_init( struct draw_context *draw )
>>>  {
>>> diff --git a/src/gallium/auxiliary/draw/draw_pt.h
>>> b/src/gallium/auxiliary/draw/draw_pt.h
>>> index 9a45845..2c2efdc 100644
>>> --- a/src/gallium/auxiliary/draw/draw_pt.h
>>> +++ b/src/gallium/auxiliary/draw/draw_pt.h
>>> @@ -73,7 +73,7 @@ struct draw_pt_front_end {
>>>                  unsigned start,
>>>                  unsigned count );
>>>
>>> -   void (*finish)( struct draw_pt_front_end * );
>>> +   void (*flush)( struct draw_pt_front_end *, unsigned flags );
>>>     void (*destroy)( struct draw_pt_front_end * );
>>>  };
>>>
>>> diff --git a/src/gallium/auxiliary/draw/draw_pt_vsplit.c
>>> b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
>>> index c19dcd9..0fed057 100644
>>> --- a/src/gallium/auxiliary/draw/draw_pt_vsplit.c
>>> +++ b/src/gallium/auxiliary/draw/draw_pt_vsplit.c
>>> @@ -178,11 +178,14 @@ static void vsplit_prepare(struct
>>> draw_pt_front_end *frontend,
>>>  }
>>>
>>>
>>> -static void vsplit_finish(struct draw_pt_front_end *frontend)
>>> +static void vsplit_flush(struct draw_pt_front_end *frontend,
>>> unsigned flags)
>>>  {
>>>     struct vsplit_frontend *vsplit = (struct vsplit_frontend *)
>>>     frontend;
>>> -   vsplit->middle->finish(vsplit->middle);
>>> -   vsplit->middle = NULL;
>>> +
>>> +   if (!(flags & DRAW_FLUSH_BACKEND)) {
>>> +      vsplit->middle->finish(vsplit->middle);
>>> +      vsplit->middle = NULL;
>>> +   }
>>>  }
>>>
>>>
>>> @@ -202,7 +205,7 @@ struct draw_pt_front_end *draw_pt_vsplit(struct
>>> draw_context *draw)
>>>
>>>     vsplit->base.prepare = vsplit_prepare;
>>>     vsplit->base.run     = NULL;
>>> -   vsplit->base.finish  = vsplit_finish;
>>> +   vsplit->base.flush   = vsplit_flush;
>>>     vsplit->base.destroy = vsplit_destroy;
>>>     vsplit->draw = draw;
>>>
>>>
>>> _______________________________________________
>>> mesa-commit mailing list
>>> mesa-commit at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-commit
>>>


More information about the mesa-dev mailing list