[Mesa-dev] [PATCH 2/2] st/mesa: fix CTS regression caused by fcbb93e86024
Samuel Pitoiset
samuel.pitoiset at gmail.com
Tue Aug 1 08:07:19 UTC 2017
Don't you think it's just safer to revert the bad commit for 17.2 and
fix it later on?
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