[Mesa-dev] [RFCv2 03/13] nir: allow pre-resolved sampler uniform locations

[Rob Clark]

On Sun, Nov 8, 2015 at 7:58 PM, Timothy Arceri <t_arceri at yahoo.com.au> wrote:
> On Sun, 2015-11-08 at 15:12 -0500, Rob Clark wrote:
>> From: Rob Clark <robclark at freedesktop.org>
>>
>> With TGSI, the ir_variable::data.location gets fixed up to be a stage
>> local location (rather than program global).  In this case we need to
>> skip the UniformStorage[location] lookup.
>> ---
>>  src/glsl/nir/nir_lower_samplers.c | 23 ++++++++++++++++-------
>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/src/glsl/nir/nir_lower_samplers.c
>> b/src/glsl/nir/nir_lower_samplers.c
>> index 5df79a6..d99ba4c 100644
>> --- a/src/glsl/nir/nir_lower_samplers.c
>> +++ b/src/glsl/nir/nir_lower_samplers.c
>> @@ -130,14 +130,18 @@ lower_sampler(nir_tex_instr *instr, const struct
>> gl_shader_program *shader_progr
>>        instr->sampler_array_size = array_elements;
>>     }
>>
>> -   if (location > shader_program->NumUniformStorage - 1 ||
>> -       !shader_program->UniformStorage[location].opaque[stage].active) {
>> -      assert(!"cannot return a sampler");
>> -      return;
>> -   }
>> +   if (!shader_program) {
>> +      instr->sampler_index = location;
>> +   } else {
>> +      if (location > shader_program->NumUniformStorage - 1 ||
>> +          !shader_program->UniformStorage[location].opaque[stage].active) {
>> +         assert(!"cannot return a sampler");
>> +         return;
>> +      }
>>
>> -   instr->sampler_index +=
>> -      shader_program->UniformStorage[location].opaque[stage].index;
>> +      instr->sampler_index =
>> +         shader_program->UniformStorage[location].opaque[stage].index;
>
> Hi Rob,
>
> This will break arrays as instr->sampler_index is increamented inside
>  calc_sampler_offsets()

oh, whoops, I didn't notice that.. ok, that part is easy enough to fix..

> calc_sampler_offsets() also modifies the value of location is this what you
> want? I would assume not as we are counting uniforms not just samplers here.

hmm, tbh I'm not entirely sure..  offhand, what piglit's should I
check?  I guess it would be easier to debug if it worked correctly
with glsl_to_tgsi, but I guess I could try to get the non-indirect
case working..

BR,
-R

> The other thing to note is that glsl to tgsi doesn't handle indirects on
> structs or arrays of arrays correctly (Ilia was trying to fix this).
>
> Tim
>
>
>
>> +   }
>>
>>     instr->sampler = NULL;
>>  }
>> @@ -177,6 +181,11 @@ lower_impl(nir_function_impl *impl, const struct
>> gl_shader_program *shader_progr
>>     nir_foreach_block(impl, lower_block_cb, &state);
>>  }
>>
>> +/* Call with a null 'shader_program' if uniform locations are
>
> uniform locations -> sampler indices?
>
>> + * already local to the shader, ie. skipping the
>> + * shader_program->UniformStorage[location].opaque[stage].index
>> + * lookup
>> + */
>>  void
>>  nir_lower_samplers(nir_shader *shader,
>>                     const struct gl_shader_program *shader_program)