[Mesa-dev] [Mesa-stable] [PATCH 1/2] i965: Skip allocating UNIFORM file storage for uniforms of size 0.

[Ian Romanick]

On 09/12/2014 07:54 AM, Ian Romanick wrote:
> On 09/11/2014 10:07 PM, Kenneth Graunke wrote:
>> Samplers take up zero slots and therefore don't exist in the params
>> array, nor are they included in stage_prog_data->nr_params.  There's no
>> need to store their size in param_size, as it's only used for dealing
>> with arrays of "real" uniforms (ones uploaded as shader constants).
>>
>> We run into all kinds of problems trying to refer to the uniform storage
>> for variables that don't have uniform storage.  For one, we may use some
>> other variable's index, or access out of bounds in arrays.  In the FS
>> backend, our extra 2 * MaxSamplerImageUnits params for texture rectangle
>> rescaling paper over a lot of problems.  In the VS backend, we claim
>> samplers take up a slot, which also papers over problems.
>>
>> Instead, just skip allocating storage for variables that don't have any.
> 
> At least until we get to bindless, this seems right.  Are there any bad
> interactions with ARB_gpu_shader5 dynamic index of sampler arrays?

Assuming that's fine, the series is

Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>

>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> Cc: mesa-stable at lists.freedesktop.org
>> ---
>>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp   | 6 +++---
>>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 6 +++---
>>  2 files changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> index 5d2e7c8..2d5318a 100644
>> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
>> @@ -109,10 +109,10 @@ fs_visitor::visit(ir_variable *ir)
>>         * ir_binop_ubo_load expressions and not ir_dereference_variable for UBO
>>         * variables, so no need for them to be in variable_ht.
>>         *
>> -       * Atomic counters take no uniform storage, no need to do
>> -       * anything here.
>> +       * Some uniforms, such as samplers and atomic counters, have no actual
>> +       * storage, so we should ignore them.
>>         */
>> -      if (ir->is_in_uniform_block() || ir->type->contains_atomic())
>> +      if (ir->is_in_uniform_block() || type_size(ir->type) == 0)
>>           return;
>>  
>>        if (dispatch_width == 16) {
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> index 1e823da..d504e2e 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
>> @@ -1023,10 +1023,10 @@ vec4_visitor::visit(ir_variable *ir)
>>         * ir_binop_ubo_load expressions and not ir_dereference_variable for UBO
>>         * variables, so no need for them to be in variable_ht.
>>         *
>> -       * Atomic counters take no uniform storage, no need to do
>> -       * anything here.
>> +       * Some uniforms, such as samplers and atomic counters, have no actual
>> +       * storage, so we should ignore them.
>>         */
>> -      if (ir->is_in_uniform_block() || ir->type->contains_atomic())
>> +      if (ir->is_in_uniform_block() || type_size(ir->type) == 0)
>>           return;
>>  
>>        /* Track how big the whole uniform variable is, in case we need to put a
>>
> 
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
>