Re: [Mesa-dev] [PATCH v4 16/17] glsl linker: compare interface blocks during intrastage linking
On 05/16/2013 05:44 PM, Jordan Justen wrote: Verify that interface blocks match when combining compilation units at the same stage. (For example, when merging all vertex shaders.) Fixes piglit glsl-1.50 test: * linker/interface-blocks-multiple-vs-member-count-mismatch.shader_test Signed-off-by: Jordan Justen I will be sending out a revised version of this patch that incorporates my suggestions. --- src/glsl/Makefile.sources |1 + src/glsl/interface_blocks.cpp | 86 + src/glsl/interface_blocks.h | 32 +++ src/glsl/linker.cpp |7 4 files changed, 126 insertions(+) create mode 100644 src/glsl/interface_blocks.cpp create mode 100644 src/glsl/interface_blocks.h diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources index b9fd283..e39cb1a 100644 --- a/src/glsl/Makefile.sources +++ b/src/glsl/Makefile.sources @@ -26,6 +26,7 @@ LIBGLSL_FILES = \ $(GLSL_SRCDIR)/glsl_types.cpp \ $(GLSL_SRCDIR)/glsl_symbol_table.cpp \ $(GLSL_SRCDIR)/hir_field_selection.cpp \ + $(GLSL_SRCDIR)/interface_blocks.cpp \ $(GLSL_SRCDIR)/ir_basic_block.cpp \ $(GLSL_SRCDIR)/ir_builder.cpp \ $(GLSL_SRCDIR)/ir_clone.cpp \ diff --git a/src/glsl/interface_blocks.cpp b/src/glsl/interface_blocks.cpp new file mode 100644 index 000..6dfd0c4 --- /dev/null +++ b/src/glsl/interface_blocks.cpp I would call this link_interface_blocks.cpp. All the other linker code is named link*.cpp. @@ -0,0 +1,86 @@ +/* + * Copyright (c) 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. + */ + +/** + * \file interface_blocks.cpp + * GLSL interface blocks support + */ + +#include "ir.h" +#include "glsl_symbol_table.h" +#include "main/macros.h" +#include "program/hash_table.h" + +static bool +cross_validate_interface_blocks(const gl_shader **shader_list, +unsigned num_shaders) +{ + bool ok = true; + glsl_symbol_table interfaces; + + for (unsigned int i = 0; ok && i < num_shaders; i++) { + if (shader_list[i] == NULL) + continue; + + foreach_list(node, shader_list[i]->ir) { + ir_variable *var = ((ir_instruction *) node)->as_variable(); + if (!var) +continue; + + const glsl_type *iface_type = var->interface_type; + + if (iface_type == NULL) +continue; + + const char *iface_name = iface_type->name; + + const glsl_type *old_iface_type = +interfaces.get_interface(iface_name, + (enum ir_variable_mode) var->mode); + + /* This is the first time we've seen the interface, so save + * it into our symbol table. + */ + if (old_iface_type == NULL) { +interfaces.add_interface(iface_name, iface_type, + (enum ir_variable_mode) var->mode); +old_iface_type = iface_type; + } + + ok &= old_iface_type == iface_type; I guess C++ booleans are guaranteed to be 0 or 1, so this ought to work, but using bitwise operators on bools is still a bit odd. + if (!ok) +break; You could actually just drop the "ok" variable altogether and change this code to: if (old_iface_type != iface_type) return false; and then "return true" at the bottom. Seems simpler. + } + } + + return ok; +} + +bool +validate_intrastage_interface_blocks(const gl_shader **shader_list, + unsigned num_shaders) +{ + return cross_validate_interface_blocks(shader_list, + num_shaders); +} + diff --git a/src/glsl/interface_blocks.h b/src/glsl/interface_blocks.h new file mode 100644 index 000..4c37c02 --- /dev/null +++ b/src/glsl/interface_blocks.h I would just stuff this in
[Mesa-dev] [PATCH v4 16/17] glsl linker: compare interface blocks during intrastage linking
Verify that interface blocks match when combining compilation units at the same stage. (For example, when merging all vertex shaders.) Fixes piglit glsl-1.50 test: * linker/interface-blocks-multiple-vs-member-count-mismatch.shader_test Signed-off-by: Jordan Justen --- src/glsl/Makefile.sources |1 + src/glsl/interface_blocks.cpp | 86 + src/glsl/interface_blocks.h | 32 +++ src/glsl/linker.cpp |7 4 files changed, 126 insertions(+) create mode 100644 src/glsl/interface_blocks.cpp create mode 100644 src/glsl/interface_blocks.h diff --git a/src/glsl/Makefile.sources b/src/glsl/Makefile.sources index b9fd283..e39cb1a 100644 --- a/src/glsl/Makefile.sources +++ b/src/glsl/Makefile.sources @@ -26,6 +26,7 @@ LIBGLSL_FILES = \ $(GLSL_SRCDIR)/glsl_types.cpp \ $(GLSL_SRCDIR)/glsl_symbol_table.cpp \ $(GLSL_SRCDIR)/hir_field_selection.cpp \ + $(GLSL_SRCDIR)/interface_blocks.cpp \ $(GLSL_SRCDIR)/ir_basic_block.cpp \ $(GLSL_SRCDIR)/ir_builder.cpp \ $(GLSL_SRCDIR)/ir_clone.cpp \ diff --git a/src/glsl/interface_blocks.cpp b/src/glsl/interface_blocks.cpp new file mode 100644 index 000..6dfd0c4 --- /dev/null +++ b/src/glsl/interface_blocks.cpp @@ -0,0 +1,86 @@ +/* + * Copyright (c) 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. + */ + +/** + * \file interface_blocks.cpp + * GLSL interface blocks support + */ + +#include "ir.h" +#include "glsl_symbol_table.h" +#include "main/macros.h" +#include "program/hash_table.h" + +static bool +cross_validate_interface_blocks(const gl_shader **shader_list, +unsigned num_shaders) +{ + bool ok = true; + glsl_symbol_table interfaces; + + for (unsigned int i = 0; ok && i < num_shaders; i++) { + if (shader_list[i] == NULL) + continue; + + foreach_list(node, shader_list[i]->ir) { + ir_variable *var = ((ir_instruction *) node)->as_variable(); + if (!var) +continue; + + const glsl_type *iface_type = var->interface_type; + + if (iface_type == NULL) +continue; + + const char *iface_name = iface_type->name; + + const glsl_type *old_iface_type = +interfaces.get_interface(iface_name, + (enum ir_variable_mode) var->mode); + + /* This is the first time we've seen the interface, so save + * it into our symbol table. + */ + if (old_iface_type == NULL) { +interfaces.add_interface(iface_name, iface_type, + (enum ir_variable_mode) var->mode); +old_iface_type = iface_type; + } + + ok &= old_iface_type == iface_type; + if (!ok) +break; + } + } + + return ok; +} + +bool +validate_intrastage_interface_blocks(const gl_shader **shader_list, + unsigned num_shaders) +{ + return cross_validate_interface_blocks(shader_list, + num_shaders); +} + diff --git a/src/glsl/interface_blocks.h b/src/glsl/interface_blocks.h new file mode 100644 index 000..4c37c02 --- /dev/null +++ b/src/glsl/interface_blocks.h @@ -0,0 +1,32 @@ +/* + * Copyright (c) 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 +