[Mesa-dev] [PATCH] draw: make sure key size is calculated consistently.

[Jose Fonseca]

There may be more vertex elements that used in the shader. But why should the key contain those elements? Won't this cause needless recompilations (e.g., in situations where the state tracker leaves unneeded elements from previous draw?)?

That is, it seems to be that the key should have the number of elements from pipe_vertex_element information, but only copy those that vertex shader uses.

Jose


----- Original Message -----
> From: Roland Scheidegger <sroland at vmware.com>
> 
> Some parts calculated key size by using shader information, others by using
> the pipe_vertex_element information. Since it is perfectly valid to have more
> vertex_elements set than the vertex shader is using those may not be the
> same,
> so we weren't copying over all vertex_element state - this caused the tgsi
> dump
> to assert (iterates over all vertex elements). With some luck it didn't
> crash otherwise even though the llvm generate_fetch code also iterates over
> all vertex elements (probably because llvm threw away the unused inputs
> anyway),
> but if in this situation vertex texturing would be used things would
> definitely
> go wrong (as the sampler information wouldn't be copied).
> So drop the key size calculation using shader information.
> ---
>  src/gallium/auxiliary/draw/draw_llvm.c             |   13 ++++++++-----
>  src/gallium/auxiliary/draw/draw_llvm.h             |    1 -
>  .../draw/draw_pt_fetch_shade_pipeline_llvm.c       |    7 ++++++-
>  src/gallium/auxiliary/draw/draw_vs_llvm.c          |    6 ------
>  4 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c
> b/src/gallium/auxiliary/draw/draw_llvm.c
> index f3bbbbb..df57358 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
> @@ -420,17 +420,20 @@ draw_llvm_destroy(struct draw_llvm *llvm)
>   */
>  struct draw_llvm_variant *
>  draw_llvm_create_variant(struct draw_llvm *llvm,
> -			 unsigned num_inputs,
> -			 const struct draw_llvm_variant_key *key)
> +                         unsigned num_inputs,
> +                         const struct draw_llvm_variant_key *key)
>  {
>     struct draw_llvm_variant *variant;
>     struct llvm_vertex_shader *shader =
>        llvm_vertex_shader(llvm->draw->vs.vertex_shader);
>     LLVMTypeRef vertex_header;
> +   unsigned key_size = draw_llvm_variant_key_size(key->nr_vertex_elements,
> +                                                  MAX2(key->nr_samplers,
> +
> key->nr_sampler_views));
>  
>     variant = MALLOC(sizeof *variant +
> -		    shader->variant_key_size -
> -		    sizeof variant->key);
> +                    key_size -
> +                    sizeof variant->key);
>     if (variant == NULL)
>        return NULL;
>  
> @@ -440,7 +443,7 @@ draw_llvm_create_variant(struct draw_llvm *llvm,
>  
>     create_jit_types(variant);
>  
> -   memcpy(&variant->key, key, shader->variant_key_size);
> +   memcpy(&variant->key, key, key_size);
>  
>     vertex_header = create_jit_vertex_header(variant->gallivm, num_inputs);
>  
> diff --git a/src/gallium/auxiliary/draw/draw_llvm.h
> b/src/gallium/auxiliary/draw/draw_llvm.h
> index 17ca304..b20cee5 100644
> --- a/src/gallium/auxiliary/draw/draw_llvm.h
> +++ b/src/gallium/auxiliary/draw/draw_llvm.h
> @@ -281,7 +281,6 @@ struct draw_llvm_variant
>  struct llvm_vertex_shader {
>     struct draw_vertex_shader base;
>  
> -   unsigned variant_key_size;
>     struct draw_llvm_variant_list_item variants;
>     unsigned variants_created;
>     unsigned variants_cached;
> diff --git a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> index b0c18ed..d7f855f 100644
> --- a/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_pt_fetch_shade_pipeline_llvm.c
> @@ -127,13 +127,18 @@ llvm_middle_end_prepare( struct draw_pt_middle_end
> *middle,
>        struct llvm_vertex_shader *shader = llvm_vertex_shader(vs);
>        char store[DRAW_LLVM_MAX_VARIANT_KEY_SIZE];
>        unsigned i;
> +      unsigned key_size;
>  
>        key = draw_llvm_make_variant_key(fpme->llvm, store);
>  
> +      key_size = draw_llvm_variant_key_size(key->nr_vertex_elements,
> +                                            MAX2(key->nr_samplers,
> +                                                 key->nr_sampler_views));
> +
>        /* Search shader's list of variants for the key */
>        li = first_elem(&shader->variants);
>        while (!at_end(&shader->variants, li)) {
> -         if (memcmp(&li->base->key, key, shader->variant_key_size) == 0) {
> +         if (memcmp(&li->base->key, key, key_size) == 0) {
>              variant = li->base;
>              break;
>           }
> diff --git a/src/gallium/auxiliary/draw/draw_vs_llvm.c
> b/src/gallium/auxiliary/draw/draw_vs_llvm.c
> index ac3999e..50cef79 100644
> --- a/src/gallium/auxiliary/draw/draw_vs_llvm.c
> +++ b/src/gallium/auxiliary/draw/draw_vs_llvm.c
> @@ -98,12 +98,6 @@ draw_create_vs_llvm(struct draw_context *draw,
>  
>     tgsi_scan_shader(state->tokens, &vs->base.info);
>  
> -   vs->variant_key_size =
> -      draw_llvm_variant_key_size(
> -         vs->base.info.file_max[TGSI_FILE_INPUT]+1,
> -         MAX2(vs->base.info.file_max[TGSI_FILE_SAMPLER]+1,
> -              vs->base.info.file_max[TGSI_FILE_SAMPLER_VIEW]+1));
> -
>     vs->base.state.stream_output = state->stream_output;
>     vs->base.draw = draw;
>     vs->base.prepare = vs_llvm_prepare;
> --
> 1.7.9.5
>