[Mesa-dev] [PATCH] draw: do not use draw_get_option_use_llvm() inside draw execution paths

Brian Paul brianp at vmware.com
Wed May 7 10:29:56 PDT 2014


On 05/07/2014 11:08 AM, sroland at vmware.com wrote:
> From: Roland Scheidegger <sroland at vmware.com>
>
> 1c73e919a4b4dd79166d0633075990056f27fd28 made it possible to not allocate
> the tgsi machine if llvm was used. However, draw_get_option_use_llvm() is
> not reliable after draw context creation, since drivers can explicitly
> request a non-llvm draw context even if draw_get_option_use_llvm() would
> return true (and softpipe does just that) which leads to crashes.
> Thus use draw->llvm to determine if we're using llvm or not instead (and
> make draw->llvm available even if HAVE_LLVM is false so we don't have to put
> even more ifdefs).
>
> Cc: "10.2" <mesa-stable at lists.freedesktop.org>
> ---
>   src/gallium/auxiliary/draw/draw_context.c |  2 ++
>   src/gallium/auxiliary/draw/draw_gs.c      | 10 +++++-----
>   src/gallium/auxiliary/draw/draw_private.h |  4 +---
>   src/gallium/auxiliary/draw/draw_vs.c      |  4 ++--
>   src/gallium/auxiliary/draw/draw_vs_exec.c |  4 ++--
>   5 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/src/gallium/auxiliary/draw/draw_context.c b/src/gallium/auxiliary/draw/draw_context.c
> index ddc305b..d7197fd 100644
> --- a/src/gallium/auxiliary/draw/draw_context.c
> +++ b/src/gallium/auxiliary/draw/draw_context.c
> @@ -1000,6 +1000,8 @@ draw_get_shader_param_no_llvm(unsigned shader, enum pipe_shader_cap param)
>   /**
>    * XXX: Results for PIPE_SHADER_CAP_MAX_TEXTURE_SAMPLERS because there are two
>    * different ways of setting textures, and drivers typically only support one.
> + * Drivers requesting a draw context explicitly without llvm must call
> + * draw_get_shader_param_no_llvm instead.
>    */
>   int
>   draw_get_shader_param(unsigned shader, enum pipe_shader_cap param)
> diff --git a/src/gallium/auxiliary/draw/draw_gs.c b/src/gallium/auxiliary/draw/draw_gs.c
> index 5e503ff..fc4f697 100644
> --- a/src/gallium/auxiliary/draw/draw_gs.c
> +++ b/src/gallium/auxiliary/draw/draw_gs.c
> @@ -597,7 +597,7 @@ int draw_geometry_shader_run(struct draw_geometry_shader *shader,
>
>
>   #ifdef HAVE_LLVM
> -   if (draw_get_option_use_llvm()) {
> +   if (shader->draw->llvm) {
>         shader->gs_output = output_verts->verts;
>         if (max_out_prims > shader->max_out_prims) {
>            unsigned i;
> @@ -674,7 +674,7 @@ int draw_geometry_shader_run(struct draw_geometry_shader *shader,
>   void draw_geometry_shader_prepare(struct draw_geometry_shader *shader,
>                                     struct draw_context *draw)
>   {
> -   boolean use_llvm = draw_get_option_use_llvm();
> +   boolean use_llvm = draw->llvm != NULL;
>      if (!use_llvm && shader && shader->machine->Tokens != shader->state.tokens) {
>         tgsi_exec_machine_bind_shader(shader->machine,
>                                       shader->state.tokens,
> @@ -686,7 +686,7 @@ void draw_geometry_shader_prepare(struct draw_geometry_shader *shader,
>   boolean
>   draw_gs_init( struct draw_context *draw )
>   {
> -   if (!draw_get_option_use_llvm()) {
> +   if (!draw->llvm) {
>         draw->gs.tgsi.machine = tgsi_exec_machine_create();
>         if (!draw->gs.tgsi.machine)
>            return FALSE;
> @@ -715,7 +715,7 @@ draw_create_geometry_shader(struct draw_context *draw,
>                               const struct pipe_shader_state *state)
>   {
>   #ifdef HAVE_LLVM
> -   boolean use_llvm = draw_get_option_use_llvm();
> +   boolean use_llvm = draw->llvm != NULL;
>      struct llvm_geometry_shader *llvm_gs;
>   #endif
>      struct draw_geometry_shader *gs;
> @@ -870,7 +870,7 @@ void draw_delete_geometry_shader(struct draw_context *draw,
>         return;
>      }
>   #ifdef HAVE_LLVM
> -   if (draw_get_option_use_llvm()) {
> +   if (draw->llvm) {
>         struct llvm_geometry_shader *shader = llvm_geometry_shader(dgs);
>         struct draw_gs_llvm_variant_list_item *li;
>
> diff --git a/src/gallium/auxiliary/draw/draw_private.h b/src/gallium/auxiliary/draw/draw_private.h
> index 801d009..783c3ef 100644
> --- a/src/gallium/auxiliary/draw/draw_private.h
> +++ b/src/gallium/auxiliary/draw/draw_private.h
> @@ -47,7 +47,6 @@
>   #include "tgsi/tgsi_scan.h"
>
>   #ifdef HAVE_LLVM
> -struct draw_llvm;
>   struct gallivm_state;
>   #endif
>
> @@ -69,6 +68,7 @@ struct tgsi_exec_machine;
>   struct tgsi_sampler;
>   struct draw_pt_front_end;
>   struct draw_assembler;
> +struct draw_llvm;
>
>
>   /**
> @@ -318,9 +318,7 @@ struct draw_context
>      unsigned start_instance;
>      unsigned start_index;
>
> -#ifdef HAVE_LLVM
>      struct draw_llvm *llvm;
> -#endif
>
>      /** Texture sampler and sampler view state.
>       * Note that we have arrays indexed by shader type.  At this time
> diff --git a/src/gallium/auxiliary/draw/draw_vs.c b/src/gallium/auxiliary/draw/draw_vs.c
> index eb7f4e0..dc50870 100644
> --- a/src/gallium/auxiliary/draw/draw_vs.c
> +++ b/src/gallium/auxiliary/draw/draw_vs.c
> @@ -149,7 +149,7 @@ draw_vs_init( struct draw_context *draw )
>   {
>      draw->dump_vs = debug_get_option_gallium_dump_vs();
>
> -   if (!draw_get_option_use_llvm()) {
> +   if (!draw->llvm) {
>         draw->vs.tgsi.machine = tgsi_exec_machine_create();
>         if (!draw->vs.tgsi.machine)
>            return FALSE;
> @@ -175,7 +175,7 @@ draw_vs_destroy( struct draw_context *draw )
>      if (draw->vs.emit_cache)
>         translate_cache_destroy(draw->vs.emit_cache);
>
> -   if (!draw_get_option_use_llvm())
> +   if (!draw->llvm)
>         tgsi_exec_machine_destroy(draw->vs.tgsi.machine);
>   }
>
> diff --git a/src/gallium/auxiliary/draw/draw_vs_exec.c b/src/gallium/auxiliary/draw/draw_vs_exec.c
> index 6a18d8c..277d739 100644
> --- a/src/gallium/auxiliary/draw/draw_vs_exec.c
> +++ b/src/gallium/auxiliary/draw/draw_vs_exec.c
> @@ -63,7 +63,7 @@ vs_exec_prepare( struct draw_vertex_shader *shader,
>   {
>      struct exec_vertex_shader *evs = exec_vertex_shader(shader);
>
> -   debug_assert(!draw_get_option_use_llvm());
> +   debug_assert(!draw->llvm);
>      /* Specify the vertex program to interpret/execute.
>       * Avoid rebinding when possible.
>       */
> @@ -97,7 +97,7 @@ vs_exec_run_linear( struct draw_vertex_shader *shader,
>      unsigned slot;
>      boolean clamp_vertex_color = shader->draw->rasterizer->clamp_vertex_color;
>
> -   debug_assert(!draw_get_option_use_llvm());
> +   debug_assert(!shader->draw->llvm);
>      tgsi_exec_set_constant_buffers(machine, PIPE_MAX_CONSTANT_BUFFERS,
>                                     constants, const_size);
>
>

Reviewed-by: Brian Paul <brianp at vmware.com>

Thanks, Roland!

-Brian



More information about the mesa-dev mailing list