[Mesa-dev] [PATCH 2/2] st/mesa: fix CTS regression caused by fcbb93e86024
Timothy Arceri
tarceri at itsqueeze.com
Tue Aug 8 05:34:07 UTC 2017
On 01/08/17 19:37, Timothy Arceri wrote:
> 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.
So how do we move forward on this? Can we have the revert applied to
17.2 and commit this to master?
Ccing Emil for answers :)
>
>>
>> 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__ */
>>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list