[Mesa-dev] [PATCH 15/16] glsl: Correctly determine when the field of a UBO is row-major

[Matt Turner]

On Mon, Jul 21, 2014 at 2:04 PM, Ian Romanick <idr at freedesktop.org> wrote:
> From: Ian Romanick <ian.d.romanick at intel.com>
>
> Previously if a field of an block with an instance name was marked
> row-major (but block itself was not), we would think the field (and it's
> sub-fields) were column-major.
>
> Fixes gles3conform failures in:
>
> ES3-CTS.shaders.uniform_block.random.basic_types.7
> ES3-CTS.shaders.uniform_block.random.basic_types.9
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.1
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.3
> ES3-CTS.shaders.uniform_block.random.nested_structs.3
> ES3-CTS.shaders.uniform_block.random.nested_structs.5
> ES3-CTS.shaders.uniform_block.random.nested_structs.8
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays.3
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays.6
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays.7
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays.8
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays.9
> ES3-CTS.shaders.uniform_block.random.nested_structs_instance_arrays.0
> ES3-CTS.shaders.uniform_block.random.nested_structs_instance_arrays.1
> ES3-CTS.shaders.uniform_block.random.nested_structs_instance_arrays.2
> ES3-CTS.shaders.uniform_block.random.nested_structs_instance_arrays.3
> ES3-CTS.shaders.uniform_block.random.nested_structs_instance_arrays.4
> ES3-CTS.shaders.uniform_block.random.nested_structs_instance_arrays.6
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays_instance_arrays.0
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays_instance_arrays.1
> ES3-CTS.shaders.uniform_block.random.nested_structs_arrays_instance_arrays.5
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.0
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.4
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.7
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.8
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.12
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.14
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.15
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.16
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.1
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.8
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.9
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.10
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.11
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.13
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.14
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.15
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.16
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.17
>
> Fixes gles3conform failures (caused by previous commits) in:
>
> ES3-CTS.shaders.uniform_block.random.basic_types.8
> ES3-CTS.shaders.uniform_block.random.basic_arrays.3
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.0
> ES3-CTS.shaders.uniform_block.random.basic_instance_arrays.2
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.9
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.13
> ES3-CTS.shaders.uniform_block.random.all_per_block_buffers.18
> ES3-CTS.shaders.uniform_block.random.all_shared_buffer.4
>
> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  src/glsl/lower_ubo_reference.cpp | 137 ++++++++++++++++++++++++++++++++++-----
>  1 file changed, 122 insertions(+), 15 deletions(-)
>
> diff --git a/src/glsl/lower_ubo_reference.cpp b/src/glsl/lower_ubo_reference.cpp
> index 99bfe97..0ee4e8a 100644
> --- a/src/glsl/lower_ubo_reference.cpp
> +++ b/src/glsl/lower_ubo_reference.cpp
> @@ -40,6 +40,98 @@
>
>  using namespace ir_builder;
>
> +/**
> + * Determine if a thing being dereferenced is row-major
> + *
> + * There is some trickery here.
> + *
> + * If the thing being dereferenced is a member of uniform block \b without an
> + * instance name, then the name of the \c ir_variable is the field name of an
> + * interface type.  If this field is row-major, then the thing referenced is
> + * row-major.
> + *
> + * If the thing being dereferenced is a member of uniform block \b with an
> + * instance name, then the last dereference in the tree will be an
> + * \c ir_dereference_record.  If that record field is row-major, then the
> + * thing referenced is row-major.
> + */
> +static bool
> +is_dereferenced_thing_row_major(const ir_dereference *deref)
> +{
> +   bool matrix = false;
> +   const ir_rvalue *ir = deref;
> +
> +   while (true) {
> +      if (ir->type->is_matrix()
> +          || (ir->type->is_array() && ir->type->fields.array->is_matrix()))

Use your new function here.

> +         matrix = true;
> +
> +      switch (ir->ir_type) {
> +      case ir_type_dereference_array: {
> +         const ir_dereference_array *const array_deref =
> +            (const ir_dereference_array *) ir;
> +
> +         ir = array_deref->array;
> +         break;
> +      }
> +
> +      case ir_type_dereference_record: {
> +         const ir_dereference_record *const record_deref =
> +            (const ir_dereference_record *) ir;
> +
> +         ir = record_deref->record;
> +
> +         const int idx = ir->type->field_index(record_deref->field);
> +         assert(idx >= 0);
> +
> +         const enum glsl_matrix_layout matrix_layout =
> +            glsl_matrix_layout(ir->type->fields.structure[idx].matrix_layout);
> +
> +         switch (matrix_layout) {
> +         case GLSL_MATRIX_LAYOUT_DEFAULT:
> +            break;
> +         case GLSL_MATRIX_LAYOUT_COLUMN_MAJOR:
> +            return false;
> +         case GLSL_MATRIX_LAYOUT_ROW_MAJOR:
> +            return matrix || deref->type->is_record();
> +         }
> +
> +         break;
> +      }
> +
> +      case ir_type_dereference_variable: {
> +         const ir_dereference_variable *const var_deref =
> +            (const ir_dereference_variable *) ir;
> +
> +         const enum glsl_matrix_layout matrix_layout =
> +            glsl_matrix_layout(var_deref->var->data.matrix_layout);
> +
> +         switch (matrix_layout) {
> +         case GLSL_MATRIX_LAYOUT_DEFAULT:
> +            assert(!matrix);
> +            return false;
> +         case GLSL_MATRIX_LAYOUT_COLUMN_MAJOR:
> +            return false;
> +         case GLSL_MATRIX_LAYOUT_ROW_MAJOR:
> +            return matrix || deref->type->is_record();
> +         }
> +
> +         unreachable("invalid matrix layout");
> +         break;

No need for break after unreachable.

> +      }
> +
> +      default:
> +         return false;
> +      }
> +   }
> +
> +   /* The tree must have ended with a dereference that wasn't an
> +    * ir_dereference_variable.  That is invalid, and it should be impossible.
> +    */
> +   unreachable("invalid dereference tree");
> +   return false;

Or return after unreachable.