[Mesa-dev] [PATCH v3 (part2) 09/56] glsl: Add parser/compiler support for unsized array's length()

Jordan Justen jordan.l.justen at intel.com
Tue Aug 4 16:04:51 PDT 2015


On 2015-08-04 15:12:06, Jordan Justen wrote:
> On 2015-07-14 00:46:11, Iago Toral Quiroga wrote:
> > From: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> > 
> > It also creates unop and triop expressions to tell the driver to
> > calculate the unsized array length.
> > 
> > It is needed two expressions to do the calculation:
> 
> "Two expressions are needed to do the calculation:"
> 
> > * The unop expression saves the ir_rvalue* whose length should be
> >   calculated.
> > * Afterwards, this unop is going to be processed by a lowering pass
> > that will convert it to a triop that includes the block index,
> 
> line 'that' up with 'Afterwards'.
> 
> > offset of the variable inside the shader storage block and the array
> > stride. All of them are needed for length() calculation following
> > GL_ARB_shader_storage_buffer spec:
> > 
> >    array.length() =
> >       max((buffer_object_size - offset_of_array) / stride_of_array, 0)
> > 
> > Signed-off-by: Samuel Iglesias Gonsalvez <siglesias at igalia.com>
> > ---
> >  src/glsl/hir_field_selection.cpp                      | 15 +++++++++++----
> >  src/glsl/ir.cpp                                       |  9 +++++++++
> >  src/glsl/ir.h                                         | 19 ++++++++++++++++++-

Hmm, I added an r-b below, but I had a question.

Rather than IR nodes, should this be handled with intrinsics like the
SSBO references?

-Jordan

> >  src/glsl/ir_validate.cpp                              | 13 +++++++++++++
> >  src/glsl/link_uniforms.cpp                            |  8 +++++++-
> >  .../drivers/dri/i965/brw_fs_channel_expressions.cpp   |  2 ++
> >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp        |  8 ++++++++
> >  src/mesa/program/ir_to_mesa.cpp                       |  2 ++
> >  src/mesa/state_tracker/st_glsl_to_tgsi.cpp            |  5 +++++
> >  9 files changed, 75 insertions(+), 6 deletions(-)
> > 
> > diff --git a/src/glsl/hir_field_selection.cpp b/src/glsl/hir_field_selection.cpp
> > index 0fa9768..fedbef0 100644
> > --- a/src/glsl/hir_field_selection.cpp
> > +++ b/src/glsl/hir_field_selection.cpp
> > @@ -71,10 +71,17 @@ _mesa_ast_field_selection_to_hir(const ast_expression *expr,
> >              _mesa_glsl_error(&loc, state, "length method takes no arguments");
> >  
> >           if (op->type->is_array()) {
> > -            if (op->type->is_unsized_array())
> > -               _mesa_glsl_error(&loc, state, "length called on unsized array");
> > -
> > -            result = new(ctx) ir_constant(op->type->array_size());
> > +            if (op->type->is_unsized_array()) {
> > +               if (!state->ARB_shader_storage_buffer_object_enable) {
> > +                  _mesa_glsl_error(&loc, state, "length called on unsized array"
> > +                                                " only available with "
> > +                                                "ARB_shader_storage_buffer_object");
> > +               }
> > +               /* Calculate length of an unsized array in run-time */
> > +               result = new(ctx) ir_expression(ir_unop_ssbo_unsized_array_length, op);
> > +            } else {
> > +               result = new(ctx) ir_constant(op->type->array_size());
> > +            }
> >           } else if (op->type->is_vector()) {
> >              if (state->ARB_shading_language_420pack_enable) {
> >                 /* .length() returns int. */
> > diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
> > index 390e8f3..48c91ee 100644
> > --- a/src/glsl/ir.cpp
> > +++ b/src/glsl/ir.cpp
> > @@ -340,6 +340,9 @@ ir_expression::ir_expression(int op, ir_rvalue *op0)
> >        this->type = glsl_type::get_instance(GLSL_TYPE_INT,
> >                                            op0->type->vector_elements, 1);
> >        break;
> > +   case ir_unop_ssbo_unsized_array_length:
> > +      this->type = glsl_type::int_type;
> > +      break;
> >  
> >     default:
> >        assert(!"not reached: missing automatic type setup for ir_expression");
> > @@ -471,6 +474,10 @@ ir_expression::ir_expression(int op, ir_rvalue *op0, ir_rvalue *op1,
> >        this->type = op1->type;
> >        break;
> >  
> > +   case ir_triop_ssbo_unsized_array_length:
> > +      this->type = glsl_type::int_type;
> > +      break;
> > +
> >     default:
> >        assert(!"not reached: missing automatic type setup for ir_expression");
> >        this->type = glsl_type::float_type;
> > @@ -569,6 +576,7 @@ static const char *const operator_strs[] = {
> >     "frexp_exp",
> >     "noise",
> >     "interpolate_at_centroid",
> > +   "ssbo_unsized_array_length_parser",
> >     "+",
> >     "-",
> >     "*",
> > @@ -609,6 +617,7 @@ static const char *const operator_strs[] = {
> >     "csel",
> >     "bfi",
> >     "bitfield_extract",
> > +   "ssbo_unsized_array_length",
> >     "vector_insert",
> >     "bitfield_insert",
> >     "vector",
> > diff --git a/src/glsl/ir.h b/src/glsl/ir.h
> > index e677b89..b2e94a7 100644
> > --- a/src/glsl/ir.h
> > +++ b/src/glsl/ir.h
> > @@ -1406,9 +1406,17 @@ enum ir_expression_operation {
> >     ir_unop_interpolate_at_centroid,
> >  
> >     /**
> > +    * Calculate length of an unsized array inside a buffer block.
> > +    * This opcode is going to be replaced in a lowering pass inside the linker.
> 
> line length too long
> 
> > +    *
> > +    * operand0 is the unsized array's ir_value for the calculation of its lenght.
> 
> line length too long
> 
> also, typo in 'length'
> 
> Reviewed-by: Jordan Justen <jordan.l.justen at intel.com>
> 
> > +    */
> > +   ir_unop_ssbo_unsized_array_length,
> > +
> > +   /**
> >      * A sentinel marking the last of the unary operations.
> >      */
> > -   ir_last_unop = ir_unop_interpolate_at_centroid,
> > +   ir_last_unop = ir_unop_ssbo_unsized_array_length,
> >  
> >     ir_binop_add,
> >     ir_binop_sub,
> > @@ -1580,6 +1588,15 @@ enum ir_expression_operation {
> >     ir_triop_bitfield_extract,
> >  
> >     /**
> > +    * Calculate length of an unsized array inside a buffer block.
> > +    *
> > +    * operand0 is the ir_constant buffer block index in the linked shader.
> > +    * operand1 is a byte offset of the unsized array inside the buffer block
> > +    * operand2 is the array stride if the unsized array were sized.
> > +    */
> > +   ir_triop_ssbo_unsized_array_length,
> > +
> > +   /**
> >      * Generate a value with one field of a vector changed
> >      *
> >      * operand0 is the vector
> > diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
> > index 684bef2..728163f 100644
> > --- a/src/glsl/ir_validate.cpp
> > +++ b/src/glsl/ir_validate.cpp
> > @@ -409,6 +409,12 @@ ir_validate::visit_leave(ir_expression *ir)
> >        assert(ir->operands[0]->type->is_float());
> >        break;
> >  
> > +   case ir_unop_ssbo_unsized_array_length:
> > +      assert(ir->type == glsl_type::int_type);
> > +      assert(ir->operands[0]->type->is_array());
> > +      assert(ir->operands[0]->type->is_unsized_array());
> > +      break;
> > +
> >     case ir_unop_d2f:
> >        assert(ir->operands[0]->type->base_type == GLSL_TYPE_DOUBLE);
> >        assert(ir->type->base_type == GLSL_TYPE_FLOAT);
> > @@ -654,6 +660,13 @@ ir_validate::visit_leave(ir_expression *ir)
> >        assert(ir->operands[3]->type == glsl_type::int_type);
> >        break;
> >  
> > +   case ir_triop_ssbo_unsized_array_length:
> > +      assert(ir->type == glsl_type::int_type);
> > +      assert(ir->operands[0]->type == glsl_type::uint_type);
> > +      assert(ir->operands[1]->type == glsl_type::uint_type);
> > +      assert(ir->operands[2]->type == glsl_type::uint_type);
> > +      break;
> > +
> >     case ir_quadop_vector:
> >        /* The vector operator collects some number of scalars and generates a
> >         * vector from them.
> > diff --git a/src/glsl/link_uniforms.cpp b/src/glsl/link_uniforms.cpp
> > index e786ddc..131e76c 100644
> > --- a/src/glsl/link_uniforms.cpp
> > +++ b/src/glsl/link_uniforms.cpp
> > @@ -221,7 +221,13 @@ program_resource_visitor::recursion(const glsl_type *t, char **name,
> >        if (record_type == NULL && t->fields.array->is_record())
> >           record_type = t->fields.array;
> >  
> > -      for (unsigned i = 0; i < t->length; i++) {
> > +      unsigned length = t->length;
> > +      /* Shader storage block unsized arrays: add subscript [0] to variable
> > +       * names */
> > +      if (t->is_unsized_array())
> > +         length = 1;
> > +
> > +      for (unsigned i = 0; i < length; i++) {
> >          size_t new_length = name_length;
> >  
> >          /* Append the subscript to the current variable name */
> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp b/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> > index d0f6122..fb06c10 100644
> > --- a/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_fs_channel_expressions.cpp
> > @@ -378,6 +378,7 @@ ir_channel_expressions_visitor::visit_leave(ir_assignment *ir)
> >     }
> >  
> >     case ir_binop_ubo_load:
> > +   case ir_triop_ssbo_unsized_array_length:
> >        unreachable("not yet supported");
> >  
> >     case ir_triop_fma:
> > @@ -429,6 +430,7 @@ ir_channel_expressions_visitor::visit_leave(ir_assignment *ir)
> >     case ir_triop_vector_insert:
> >     case ir_quadop_bitfield_insert:
> >     case ir_quadop_vector:
> > +   case ir_unop_ssbo_unsized_array_length:
> >        unreachable("should have been lowered");
> >  
> >     case ir_unop_unpack_half_2x16_split_x:
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > index 0c8b0bd..d95b7c8 100644
> > --- a/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp
> > @@ -1555,6 +1555,10 @@ vec4_visitor::visit(ir_expression *ir)
> >     case ir_unop_noise:
> >        unreachable("not reached: should be handled by lower_noise");
> >  
> > +   case ir_unop_ssbo_unsized_array_length:
> > +      unreachable("not reached: should be handled by lower_ubo_reference");
> > +      break;
> > +
> >     case ir_binop_add:
> >        emit(ADD(result_dst, op[0], op[1]));
> >        break;
> > @@ -1924,6 +1928,10 @@ vec4_visitor::visit(ir_expression *ir)
> >        emit(BFE(result_dst, op[2], op[1], op[0]));
> >        break;
> >  
> > +   case ir_triop_ssbo_unsized_array_length:
> > +      unreachable("not reached: not implemented");
> > +      break;
> > +
> >     case ir_triop_vector_insert:
> >        unreachable("should have been lowered by lower_vector_insert");
> >  
> > diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> > index 2bd212e..ff7115c 100644
> > --- a/src/mesa/program/ir_to_mesa.cpp
> > +++ b/src/mesa/program/ir_to_mesa.cpp
> > @@ -1342,9 +1342,11 @@ ir_to_mesa_visitor::visit(ir_expression *ir)
> >     case ir_unop_dFdx_fine:
> >     case ir_unop_dFdy_coarse:
> >     case ir_unop_dFdy_fine:
> > +   case ir_triop_ssbo_unsized_array_length:
> >        assert(!"not supported");
> >        break;
> >  
> > +   case ir_unop_ssbo_unsized_array_length:
> >     case ir_quadop_vector:
> >        /* This operation should have already been handled.
> >         */
> > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > index 25e30c7..244d897 100644
> > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
> > @@ -2172,10 +2172,15 @@ glsl_to_tgsi_visitor::visit(ir_expression *ir)
> >     case ir_triop_vector_insert:
> >     case ir_binop_carry:
> >     case ir_binop_borrow:
> > +   case ir_unop_ssbo_unsized_array_length:
> >        /* This operation is not supported, or should have already been handled.
> >         */
> >        assert(!"Invalid ir opcode in glsl_to_tgsi_visitor::visit()");
> >        break;
> > +
> > +   case ir_triop_ssbo_unsized_array_length:
> > +      assert(!"Not implemented yet");
> > +      break;
> >     }
> >  
> >     this->result = result_src;
> > -- 
> > 1.9.1
> > 


More information about the mesa-dev mailing list