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.roman...@intel.com> >> Signed-off-by: Kenneth Graunke <kenn...@whitecape.org> >> Cc: mesa-sta...@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-sta...@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-stable > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev