[Mesa-dev] [PATCH 2/2] st/mesa: fix CTS regression caused by fcbb93e86024

Timothy Arceri tarceri at itsqueeze.com
Tue Aug 1 09:37:01 UTC 2017


On 01/08/17 18:07, Samuel Pitoiset wrote:
> Don't you think it's just safer to revert the bad commit for 17.2 and 
> fix it later on?

I'm not overly worried. If you really want to go that way we can, but I 
don't think it's necessary.

> 
> On 08/01/2017 09:35 AM, Timothy Arceri wrote:
>> When generation the storage offset for struct members we need
>> to skip opaque types as they no longer have backing storage.
>>
>> Fixes: fcbb93e86024 ("mesa: stop assigning unused storage for 
>> non-bindless opaque types")
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101983
>> ---
>>   src/mesa/Makefile.sources                  |   2 +
>>   src/mesa/state_tracker/st_glsl_to_tgsi.cpp |  16 ++++-
>>   src/mesa/state_tracker/st_glsl_types.cpp   | 105 
>> +++++++++++++++++++++++++++++
>>   src/mesa/state_tracker/st_glsl_types.h     |  44 ++++++++++++
>>   4 files changed, 165 insertions(+), 2 deletions(-)
>>   create mode 100644 src/mesa/state_tracker/st_glsl_types.cpp
>>   create mode 100644 src/mesa/state_tracker/st_glsl_types.h
>>
>> diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources
>> index 97c8287acb..2c557c3952 100644
>> --- a/src/mesa/Makefile.sources
>> +++ b/src/mesa/Makefile.sources
>> @@ -500,20 +500,22 @@ STATETRACKER_FILES = \
>>       state_tracker/st_extensions.c \
>>       state_tracker/st_extensions.h \
>>       state_tracker/st_format.c \
>>       state_tracker/st_format.h \
>>       state_tracker/st_gen_mipmap.c \
>>       state_tracker/st_gen_mipmap.h \
>>       state_tracker/st_gl_api.h \
>>       state_tracker/st_glsl_to_nir.cpp \
>>       state_tracker/st_glsl_to_tgsi.cpp \
>>       state_tracker/st_glsl_to_tgsi.h \
>> +-       state_tracker/st_glsl_types.cpp \
>> +-       state_tracker/st_glsl_types.h \
>>       state_tracker/st_manager.c \
>>       state_tracker/st_manager.h \
>>       state_tracker/st_mesa_to_tgsi.c \
>>       state_tracker/st_mesa_to_tgsi.h \
>>       state_tracker/st_nir.h \
>>       state_tracker/st_nir_lower_builtin.c \
>>       state_tracker/st_nir_lower_tex_src_plane.c \
>>       state_tracker/st_pbo.c \
>>       state_tracker/st_pbo.h \
>>       state_tracker/st_program.c \
>> diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp 
>> b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> index 2fca398a51..54a749d388 100644
>> --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp
>> @@ -42,20 +42,21 @@
>>   #include "main/shaderapi.h"
>>   #include "main/shaderimage.h"
>>   #include "program/prog_instruction.h"
>>   #include "pipe/p_context.h"
>>   #include "pipe/p_screen.h"
>>   #include "tgsi/tgsi_ureg.h"
>>   #include "tgsi/tgsi_info.h"
>>   #include "util/u_math.h"
>>   #include "util/u_memory.h"
>> +#include "st_glsl_types.h"
>>   #include "st_program.h"
>>   #include "st_mesa_to_tgsi.h"
>>   #include "st_format.h"
>>   #include "st_nir.h"
>>   #include "st_shader_cache.h"
>>   #include "util/hash_table.h"
>>   #include <algorithm>
>>   #define PROGRAM_ANY_CONST ((1 << PROGRAM_STATE_VAR) |    \
>> @@ -2825,22 +2826,30 @@ shrink_array_declarations(struct inout_decl 
>> *decls, unsigned count,
>>               *usage_mask |= BITFIELD64_BIT(decl->mesa_index + j);
>>         }
>>      }
>>   }
>>   void
>>   glsl_to_tgsi_visitor::visit(ir_dereference_array *ir)
>>   {
>>      ir_constant *index;
>>      st_src_reg src;
>> -   int element_size = type_size(ir->type);
>>      bool is_2D = false;
>> +   ir_variable *var = ir->variable_referenced();
>> +
>> +   /* We only need the logic provided by st_glsl_storage_type_size()
>> +    * for arrays of structs. Indirect sampler and image indexing is 
>> handled
>> +    * elsewhere.
>> +    */
>> +   int element_size = ir->type->without_array()->is_record() ?
>> +      st_glsl_storage_type_size(ir->type, var->data.bindless) :
>> +      type_size(ir->type);
>>      index = ir->array_index->constant_expression_value();
>>      ir->array->accept(this);
>>      src = this->result;
>>      if (ir->array->ir_type != ir_type_dereference_array) {
>>         switch (this->prog->Target) {
>>         case GL_TESS_CONTROL_PROGRAM_NV:
>>            is_2D = (src.file == PROGRAM_INPUT || src.file == 
>> PROGRAM_OUTPUT) &&
>> @@ -2916,28 +2925,31 @@ 
>> glsl_to_tgsi_visitor::visit(ir_dereference_array *ir)
>>      src.type = ir->type->base_type;
>>      this->result = src;
>>   }
>>   void
>>   glsl_to_tgsi_visitor::visit(ir_dereference_record *ir)
>>   {
>>      unsigned int i;
>>      const glsl_type *struct_type = ir->record->type;
>> +   ir_variable *var = ir->record->variable_referenced();
>>      int offset = 0;
>>      ir->record->accept(this);
>> +   assert(var);
>>      for (i = 0; i < struct_type->length; i++) {
>>         if (strcmp(struct_type->fields.structure[i].name, ir->field) 
>> == 0)
>>            break;
>> -      offset += type_size(struct_type->fields.structure[i].type);
>> +      const glsl_type *member_type = 
>> struct_type->fields.structure[i].type;
>> +      offset += st_glsl_storage_type_size(member_type, 
>> var->data.bindless);
>>      }
>>      /* If the type is smaller than a vec4, replicate the last channel 
>> out. */
>>      if (ir->type->is_scalar() || ir->type->is_vector())
>>         this->result.swizzle = 
>> swizzle_for_size(ir->type->vector_elements);
>>      else
>>         this->result.swizzle = SWIZZLE_NOOP;
>>      this->result.index += offset;
>>      this->result.type = ir->type->base_type;
>> diff --git a/src/mesa/state_tracker/st_glsl_types.cpp 
>> b/src/mesa/state_tracker/st_glsl_types.cpp
>> new file mode 100644
>> index 0000000000..50936025d9
>> --- /dev/null
>> +++ b/src/mesa/state_tracker/st_glsl_types.cpp
>> @@ -0,0 +1,105 @@
>> +/*
>> + * Copyright (C) 2005-2007  Brian Paul   All Rights Reserved.
>> + * Copyright (C) 2008  VMware, Inc.   All Rights Reserved.
>> + * Copyright © 2010 Intel Corporation
>> + * Copyright © 2011 Bryan Cain
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial portions 
>> of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#include "st_glsl_types.h"
>> +
>> +/**
>> + * Returns the number of places to offset the uniform index, given 
>> the type of
>> + * a struct member. We use this because samplers and images have backing
>> + * storeage only when they are bindless.
>> + */
>> +int
>> +st_glsl_storage_type_size(const struct glsl_type *type, bool 
>> is_bindless)
>> +{
>> +   unsigned int i;
>> +   int size;
>> +
>> +   switch (type->base_type) {
>> +   case GLSL_TYPE_UINT:
>> +   case GLSL_TYPE_INT:
>> +   case GLSL_TYPE_FLOAT:
>> +   case GLSL_TYPE_BOOL:
>> +      if (type->is_matrix()) {
>> +         return type->matrix_columns;
>> +      } else {
>> +         /* Regardless of size of vector, it gets a vec4. This is bad
>> +          * packing for things like floats, but otherwise arrays 
>> become a
>> +          * mess.  Hopefully a later pass over the code can pack scalars
>> +          * down if appropriate.
>> +          */
>> +         return 1;
>> +      }
>> +      break;
>> +   case GLSL_TYPE_DOUBLE:
>> +      if (type->is_matrix()) {
>> +         if (type->vector_elements <= 2)
>> +            return type->matrix_columns;
>> +         else
>> +            return type->matrix_columns * 2;
>> +      } else {
>> +         /* For doubles if we have a double or dvec2 they fit in one
>> +          * vec4, else they need 2 vec4s.
>> +          */
>> +         if (type->vector_elements <= 2)
>> +            return 1;
>> +         else
>> +            return 2;
>> +      }
>> +      break;
>> +   case GLSL_TYPE_UINT64:
>> +   case GLSL_TYPE_INT64:
>> +      if (type->vector_elements <= 2)
>> +         return 1;
>> +      else
>> +         return 2;
>> +   case GLSL_TYPE_ARRAY:
>> +      assert(type->length > 0);
>> +      return st_glsl_storage_type_size(type->fields.array, 
>> is_bindless) *
>> +         type->length;
>> +   case GLSL_TYPE_STRUCT:
>> +      size = 0;
>> +      for (i = 0; i < type->length; i++) {
>> +         size += 
>> st_glsl_storage_type_size(type->fields.structure[i].type,
>> +                                               is_bindless);
>> +      }
>> +      return size;
>> +   case GLSL_TYPE_SAMPLER:
>> +   case GLSL_TYPE_IMAGE:
>> +      if (!is_bindless)
>> +         return 0;
>> +      /* fall through */
>> +   case GLSL_TYPE_SUBROUTINE:
>> +      return 1;
>> +   case GLSL_TYPE_ATOMIC_UINT:
>> +   case GLSL_TYPE_INTERFACE:
>> +   case GLSL_TYPE_VOID:
>> +   case GLSL_TYPE_ERROR:
>> +   case GLSL_TYPE_FUNCTION:
>> +      assert(!"Invalid type in type_size");
>> +      break;
>> +   }
>> +   return 0;
>> +}
>> diff --git a/src/mesa/state_tracker/st_glsl_types.h 
>> b/src/mesa/state_tracker/st_glsl_types.h
>> new file mode 100644
>> index 0000000000..915816d1fa
>> --- /dev/null
>> +++ b/src/mesa/state_tracker/st_glsl_types.h
>> @@ -0,0 +1,44 @@
>> +/*
>> + * Copyright (C) 2005-2007  Brian Paul   All Rights Reserved.
>> + * Copyright (C) 2008  VMware, Inc.   All Rights Reserved.
>> + * Copyright © 2010 Intel Corporation
>> + * Copyright © 2011 Bryan Cain
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial portions 
>> of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
>> SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
>> + * DEALINGS IN THE SOFTWARE.
>> + */
>> +
>> +#ifndef __ST_GLSL_TYPES_H__
>> +#define __ST_GLSL_TYPES_H__
>> +
>> +#include "compiler/glsl_types.h"
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +int st_glsl_storage_type_size(const struct glsl_type *type,
>> +                              bool is_bindless);
>> +
>> +
>> +#ifdef __cplusplus
>> +}
>> +#endif
>> +
>> +#endif /* __ST_GLSL_TYPES_H__ */
>>


More information about the mesa-dev mailing list