On Sat, 2015-11-14 at 13:43 -0800, Jordan Justen wrote:
> Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Samuel Iglesias Gonsalvez <sigles...@igalia.com>
> Cc: Iago Toral Quiroga <ito...@igalia.com>
> ---
> src/glsl/lower_ubo_reference.cpp | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/src/glsl/lower_ubo_reference.cpp
> b/src/glsl/lower_ubo_reference.cpp
> index b74aa3d..41012db 100644
> --- a/src/glsl/lower_ubo_reference.cpp
> +++ b/src/glsl/lower_ubo_reference.cpp
> @@ -162,6 +162,14 @@ public:
> ir_call *ssbo_store(ir_rvalue *deref, ir_rvalue *offset,
> unsigned write_mask);
>
> + enum {
> + ubo_load_access,
> + ssbo_load_access,
> + ssbo_store_access,
> + ssbo_get_array_length,
ssbo_get_array_length misses that is for "unsized" arrays and does not
include the "access" prefix that the other enum values have, which makes
it a bit inconsistent. How about we name this
'ssbo_unsized_array_length_access'? or maybe 'ssbo_unsized_array_access'
if we think the former is too long.
> + ssbo_atomic_access,
> + } buffer_access_type;
> +
> void emit_access(bool is_write, ir_dereference *deref,
> ir_variable *base_offset, unsigned int deref_offset,
> bool row_major, int matrix_columns,
> @@ -189,7 +197,6 @@ public:
> struct gl_uniform_buffer_variable *ubo_var;
> ir_rvalue *uniform_block;
> bool progress;
> - bool is_shader_storage;
> };
>
> /**
> @@ -339,10 +346,9 @@
> lower_ubo_reference_visitor::setup_for_load_or_store(ir_variable *var,
> deref, &nonconst_block_index);
>
> /* Locate the block by interface name */
> - this->is_shader_storage = var->is_in_shader_storage_block();
> unsigned num_blocks;
> struct gl_uniform_block **blocks;
> - if (this->is_shader_storage) {
> + if (buffer_access_type != ubo_load_access) {
I think this file generally uses 'this->' to refer to class members (or
at least this function does), so maybe we should keep that for
consistency. The same in the other places where you use
buffer_access_type.
That said, right now it seems that we only ever use buffer_access_type
here and you always assign its value right before calling
setup_for_load_or_store() so maybe it is better to just make it a
function parameter instead of a class member? setup_for_load_or_store()
already has a large number of parameters, so I am not super happy about
the idea, but it looks more natural to me. What do you think?
> num_blocks = shader->NumShaderStorageBlocks;
> blocks = shader->ShaderStorageBlocks;
> } else {
> @@ -552,6 +558,10 @@ lower_ubo_reference_visitor::handle_rvalue(ir_rvalue
> **rvalue)
> int matrix_columns;
> unsigned packing = var->get_interface_type()->interface_packing;
>
> + buffer_access_type =
> + var->is_in_shader_storage_block() ?
> + ssbo_load_access : ubo_load_access;
> +
> /* Compute the offset to the start if the dereference as well as other
> * information we need to configure the write
> */
> @@ -795,7 +805,7 @@ lower_ubo_reference_visitor::emit_access(bool is_write,
> if (is_write)
> base_ir->insert_after(ssbo_store(deref, offset, write_mask));
> else {
> - if (!this->is_shader_storage) {
> + if (buffer_access_type == ubo_load_access) {
> base_ir->insert_before(assign(deref->clone(mem_ctx, NULL),
> ubo_load(deref->type, offset)));
> } else {
> @@ -862,7 +872,7 @@ lower_ubo_reference_visitor::emit_access(bool is_write,
>
> base_ir->insert_after(ssbo_store(swizzle(deref, i, 1),
> chan_offset, 1));
> } else {
> - if (!this->is_shader_storage) {
> + if (buffer_access_type == ubo_load_access) {
> base_ir->insert_before(assign(deref->clone(mem_ctx, NULL),
> ubo_load(deref_type,
> chan_offset),
> (1U << i)));
> @@ -891,6 +901,8 @@
> lower_ubo_reference_visitor::write_to_memory(ir_dereference *deref,
> int matrix_columns;
> unsigned packing = var->get_interface_type()->interface_packing;
>
> + buffer_access_type = ssbo_store_access;
> +
> /* Compute the offset to the start if the dereference as well as other
> * information we need to configure the write
> */
> @@ -1068,6 +1080,8 @@
> lower_ubo_reference_visitor::process_ssbo_unsized_array_length(ir_rvalue
> **rvalu
> unsigned packing = var->get_interface_type()->interface_packing;
> int unsized_array_stride = calculate_unsized_array_stride(deref, packing);
>
> + buffer_access_type = ssbo_get_array_length;
> +
> /* Compute the offset to the start if the dereference as well as other
> * information we need to calculate the length.
> */
> @@ -1181,6 +1195,8 @@
> lower_ubo_reference_visitor::lower_ssbo_atomic_intrinsic(ir_call *ir)
> int matrix_columns;
> unsigned packing = var->get_interface_type()->interface_packing;
>
> + buffer_access_type = ssbo_atomic_access;
> +
> setup_for_load_or_store(var, deref,
> &offset, &const_offset,
> &row_major, &matrix_columns,
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
