On 2015-11-20 06:48:27, Iago Toral Quiroga wrote:
> Improves register pressure, since otherwise we end up emitting
> loads for all the elements in the RHS and them emitting
> stores for all elements in the LHS.
>
> Fixes the following piglit test:
> tests/spec/arb_shader_storage_buffer_object/execution/large-field-copy.shader_test
> ---
>
> Jordan, this fixes the link failure for the the test you provided. Needs more
> testing and I have to check if I need to do something for structs that contain
> arrays too, etc I'll do that on Monday unless someone else comes with a better
> idea to fix this.
>
> src/glsl/lower_ubo_reference.cpp | 61
> ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
>
> diff --git a/src/glsl/lower_ubo_reference.cpp
> b/src/glsl/lower_ubo_reference.cpp
> index b74aa3d..7d48960 100644
> --- a/src/glsl/lower_ubo_reference.cpp
> +++ b/src/glsl/lower_ubo_reference.cpp
> @@ -154,6 +154,7 @@ public:
> ir_call *ssbo_load(const struct glsl_type *type,
> ir_rvalue *offset);
>
> + bool check_for_ssbo_array_copy(ir_assignment *ir);
> void check_for_ssbo_store(ir_assignment *ir);
> void write_to_memory(ir_dereference *deref,
> ir_variable *var,
> @@ -1133,9 +1134,69 @@
> lower_ubo_reference_visitor::check_for_ssbo_store(ir_assignment *ir)
> }
>
>
> +bool
> +lower_ubo_reference_visitor::check_for_ssbo_array_copy(ir_assignment *ir)
> +{
> + if (!ir || !ir->lhs || !ir->rhs)
> + return false;
> +
> + ir_dereference *rhs_deref = ir->rhs->as_dereference();
> + if (!rhs_deref)
> + return false;
> +
> + ir_dereference *lhs_deref = ir->lhs->as_dereference();
> + if (!lhs_deref)
> + return false;
> +
> + /* LHS and RHS must be SSBO variables */
Must they? In fact, the issue that prompted this was a copy from an
SSBO to a CS shared variable in the ES3.1 CTS. Maybe just the source
or dest being an SSBO is good enough?
Also, I just re-sent a piglit patch. In this case I just focused on
testing that the shader linked. I tried the 'array' version with this
patch, and it hit an assertion in nir_validate.
The copy 'struct' version still fails to register allocate.
-Jordan
> + ir_variable *lhs_var = ir->lhs->variable_referenced();
> + if (!lhs_var || !lhs_var->is_in_shader_storage_block())
> + return false;
> +
> + ir_variable *rhs_var = ir->rhs->variable_referenced();
> + if (!rhs_var || !rhs_var->is_in_shader_storage_block())
> + return false;
> +
> + /* LHS and RHS must be variable dereferences.
> + * FIXME: arrays of arrays?
> + */
> + if (!ir->lhs->as_dereference_variable() ||
> + !ir->rhs->as_dereference_variable())
> + return false;
> +
> + /* LHS and RHS must be arrays */
> + if (!rhs_var->type->is_array() || !lhs_var->type->is_array())
> + return false;
> +
> + assert(lhs_deref->type->length == rhs_deref->type->length);
> +
> + for (unsigned i = 0; i < lhs_deref->type->length; i++) {
> + ir_dereference *lhs_i =
> + new(mem_ctx) ir_dereference_array(lhs_deref->clone(mem_ctx, NULL),
> + new(mem_ctx) ir_constant(i));
> +
> + ir_dereference *rhs_i =
> + new(mem_ctx) ir_dereference_array(rhs_deref->clone(mem_ctx, NULL),
> + new(mem_ctx) ir_constant(i));
> + ir->insert_after(assign(lhs_i, rhs_i));
> + }
> +
> + ir->remove();
> + return true;
> +}
> +
> ir_visitor_status
> lower_ubo_reference_visitor::visit_enter(ir_assignment *ir)
> {
> + /* Array copies could involve large amounts of SSBO load/store
> + * operations. To improve register pressure we want to special-case
> + * this and split the array copy into many individual element copies.
> + * This way we avoid emitting all the loads for the RHS first and
> + * all the writes for the LHS second.
> + */
> + if (check_for_ssbo_array_copy(ir))
> + return visit_continue_with_parent;
> +
> check_ssbo_unsized_array_length_assignment(ir);
> check_for_ssbo_store(ir);
> return rvalue_visit(ir);
> --
> 1.9.1
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
