Am 19.02.2013 15:57, schrieb 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?)?
I don't think the state tracker would leave unneeded elements like that
(that is I think the nr_elements would be adjusted if the state tracker
has to figure it out on its own, causing recompiles in any case).
But yes if you set different pipe_vertex_element which only differ in
the unused elements then it will cause unnecessary recompile (I don't
think that's really something which matters here).
>
> 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.
That doesn't sound very good. If we want to dump the
pipe_vertex_elements like we do now either we need to fix up the
nr_elements or copy all of them. Also vs_generate function seems to
create code for all pipe_vertex_elements, not just those used by the shader.
I guess that instead of using nr_elements we could just use the
information from the shader instead consistently, though I'm actually
unsure this works always - is it somehow possible to only use
vertex_element nr 2 and 4 for instance?
So I think you're suggesting instead of this fix that key->nr_elements
wouldn't be used for anything except the key comparison itself, and
everything else (calculating sampler offset in the key, tgsi dump, code
generation) would use the shader information?
Roland
>
> Jose
>
>
> ----- Original Message -----
>> From: Roland Scheidegger <srol...@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
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
