[Mesa-dev] [PATCH 1/6] glsl: When constructing a variable with an interface type, set interface_type
Paul Berry
stereotype441 at gmail.com
Fri Oct 18 22:48:03 CEST 2013
On 18 October 2013 12:17, Paul Berry <stereotype441 at gmail.com> wrote:
> On 11 October 2013 11:18, Ian Romanick <idr at freedesktop.org> wrote:
>
>> From: Ian Romanick <ian.d.romanick at intel.com>
>>
>> Ever since the addition of interface blocks with instance names, we have
>> had an implicit invariant:
>>
>> var->type->is_interface() == (var->type == var->interface_type)
>>
>> A similar invariant exists if var->type->fields.array->is_interface().
>>
>
> Can we please spell this out in more detail? As written, it seems to be
> implying that the following two invariants both hold simultaneously:
>
> var->type->is_interface() == (var->type == var->interface_type)
> AND
> var->type->fields.array->is_interface() == ...
>
> Which is impossible because you can't access "fields.array" of an
> interface type.
>
> I think the invariant you mean is something like this:
>
> If var->type->is_interface(), then var->interface_type == var->type.
> If var->type->is_array() and var->type->fields.array->is_interface(), then
> var->interface_type == var->type->fields.array.
>
> With that fixed, this patch is:
>
> Reviewed-by: Paul Berry <stereotype441 at gmail.com>
>
One other thing: would you mind adding "general-ir-test" to
src/glsl/tests/.gitignore? Thanks.
>
>
>>
>>
>> However, the ir_variable constructor doesn't maintain either invariant.
>> That seems kind of silly... and I tripped over it while writing some
>> other code. This patch makes the constructor do the right thing, and it
>> introduces some tests to verify that behavior.
>>
>> Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
>> ---
>> src/glsl/Makefile.am | 16 +++++++
>> src/glsl/ast_to_hir.cpp | 1 -
>> src/glsl/builtin_variables.cpp | 1 -
>> src/glsl/ir.cpp | 11 ++++-
>> src/glsl/tests/general_ir_test.cpp | 89
>> ++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 114 insertions(+), 4 deletions(-)
>> create mode 100644 src/glsl/tests/general_ir_test.cpp
>>
>> diff --git a/src/glsl/Makefile.am b/src/glsl/Makefile.am
>> index cbf253c..f43f49d 100644
>> --- a/src/glsl/Makefile.am
>> +++ b/src/glsl/Makefile.am
>> @@ -32,6 +32,7 @@ AM_CXXFLAGS = $(VISIBILITY_CXXFLAGS)
>> include Makefile.sources
>>
>> TESTS = glcpp/tests/glcpp-test \
>> + tests/general-ir-test \
>> tests/optimization-test \
>> tests/ralloc-test \
>> tests/sampler-types-test \
>> @@ -45,12 +46,27 @@ noinst_LTLIBRARIES = libglsl.la libglcpp.la
>> check_PROGRAMS = \
>> glcpp/glcpp \
>> glsl_test \
>> + tests/general-ir-test \
>> tests/ralloc-test \
>> tests/sampler-types-test \
>> tests/uniform-initializer-test
>>
>> noinst_PROGRAMS = glsl_compiler
>>
>> +tests_general_ir_test_SOURCES = \
>> + $(top_srcdir)/src/mesa/main/hash_table.c \
>> + $(top_srcdir)/src/mesa/main/imports.c \
>> + $(top_srcdir)/src/mesa/program/prog_hash_table.c\
>> + $(top_srcdir)/src/mesa/program/symbol_table.c \
>> + $(GLSL_SRCDIR)/standalone_scaffolding.cpp \
>> + tests/general_ir_test.cpp
>> +tests_general_ir_test_CFLAGS = \
>> + $(PTHREAD_CFLAGS)
>> +tests_general_ir_test_LDADD = \
>> + $(top_builddir)/src/gtest/libgtest.la \
>> + $(top_builddir)/src/glsl/libglsl.la \
>> + $(PTHREAD_LIBS)
>> +
>> tests_uniform_initializer_test_SOURCES = \
>> $(top_srcdir)/src/mesa/main/hash_table.c \
>> $(top_srcdir)/src/mesa/main/imports.c \
>> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
>> index dfa32d9..b644b22 100644
>> --- a/src/glsl/ast_to_hir.cpp
>> +++ b/src/glsl/ast_to_hir.cpp
>> @@ -4876,7 +4876,6 @@ ast_interface_block::hir(exec_list *instructions,
>> var_mode);
>> }
>>
>> - var->init_interface_type(block_type);
>> if (state->target == geometry_shader && var_mode ==
>> ir_var_shader_in)
>> handle_geometry_shader_input_decl(state, loc, var);
>>
>> diff --git a/src/glsl/builtin_variables.cpp
>> b/src/glsl/builtin_variables.cpp
>> index ae0a03f..9d78b2e 100644
>> --- a/src/glsl/builtin_variables.cpp
>> +++ b/src/glsl/builtin_variables.cpp
>> @@ -858,7 +858,6 @@ builtin_variable_generator::generate_varyings()
>> this->per_vertex_in.construct_interface_instance();
>> ir_variable *var = add_variable("gl_in", array(per_vertex_in_type,
>> 0),
>> ir_var_shader_in, -1);
>> - var->init_interface_type(per_vertex_in_type);
>> }
>> if (state->target == vertex_shader || state->target ==
>> geometry_shader) {
>> const glsl_type *per_vertex_out_type =
>> diff --git a/src/glsl/ir.cpp b/src/glsl/ir.cpp
>> index de9613e..54a8e40 100644
>> --- a/src/glsl/ir.cpp
>> +++ b/src/glsl/ir.cpp
>> @@ -1603,8 +1603,15 @@ ir_variable::ir_variable(const struct glsl_type
>> *type, const char *name,
>> this->depth_layout = ir_depth_layout_none;
>> this->used = false;
>>
>> - if (type && type->base_type == GLSL_TYPE_SAMPLER)
>> - this->read_only = true;
>> + if (type != NULL) {
>> + if (type->base_type == GLSL_TYPE_SAMPLER)
>> + this->read_only = true;
>> +
>> + if (type->is_interface())
>> + this->init_interface_type(type);
>> + else if (type->is_array() && type->fields.array->is_interface())
>> + this->init_interface_type(type->fields.array);
>> + }
>> }
>>
>>
>
>> diff --git a/src/glsl/tests/general_ir_test.cpp
>> b/src/glsl/tests/general_ir_test.cpp
>> new file mode 100644
>> index 0000000..862fa19
>> --- /dev/null
>> +++ b/src/glsl/tests/general_ir_test.cpp
>> @@ -0,0 +1,89 @@
>> +/*
>> + * Copyright © 2013 Intel Corporation
>> + *
>> + * 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 <gtest/gtest.h>
>> +#include "main/compiler.h"
>> +#include "main/mtypes.h"
>> +#include "main/macros.h"
>> +#include "ralloc.h"
>> +#include "ir.h"
>> +
>> +TEST(ir_variable_constructor, interface)
>> +{
>> + void *mem_ctx = ralloc_context(NULL);
>> +
>> + static const glsl_struct_field f[] = {
>> + {
>> + glsl_type::vec(4),
>> + "v",
>> + false
>> + }
>> + };
>> +
>> + const glsl_type *const interface =
>> + glsl_type::get_interface_instance(f,
>> + ARRAY_SIZE(f),
>> + GLSL_INTERFACE_PACKING_STD140,
>> + "simple_interface");
>> +
>> + static const char name[] = "named_instance";
>> +
>> + ir_variable *const v =
>> + new(mem_ctx) ir_variable(interface, name, ir_var_uniform);
>> +
>> + EXPECT_STREQ(name, v->name);
>> + EXPECT_NE(name, v->name);
>> + EXPECT_EQ(interface, v->type);
>> + EXPECT_EQ(interface, v->get_interface_type());
>> +}
>> +
>> +TEST(ir_variable_constructor, interface_array)
>> +{
>> + void *mem_ctx = ralloc_context(NULL);
>> +
>> + static const glsl_struct_field f[] = {
>> + {
>> + glsl_type::vec(4),
>> + "v",
>> + false
>> + }
>> + };
>> +
>> + const glsl_type *const interface =
>> + glsl_type::get_interface_instance(f,
>> + ARRAY_SIZE(f),
>> + GLSL_INTERFACE_PACKING_STD140,
>> + "simple_interface");
>> +
>> + const glsl_type *const interface_array =
>> + glsl_type::get_array_instance(interface, 2);
>> +
>> + static const char name[] = "array_instance";
>> +
>> + ir_variable *const v =
>> + new(mem_ctx) ir_variable(interface_array, name, ir_var_uniform);
>> +
>> + EXPECT_STREQ(name, v->name);
>> + EXPECT_NE(name, v->name);
>> + EXPECT_EQ(interface_array, v->type);
>> + EXPECT_EQ(interface, v->get_interface_type());
>> +}
>> --
>> 1.8.1.4
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131018/b4a5447a/attachment-0001.html>
More information about the mesa-dev
mailing list