Mesa (staging/19.0): glsl/linker: location aliasing requires types to have the same width
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Sat Apr 13 00:26:31 UTC 2019
Module: Mesa
Branch: staging/19.0
Commit: 332da02f275752ee523d01256073ee1b7ff15430
URL: http://cgit.freedesktop.org/mesa/mesa/commit/?id=332da02f275752ee523d01256073ee1b7ff15430
Author: Andres Gomez <agomez at igalia.com>
Date: Fri Mar 29 12:38:34 2019 +0200
glsl/linker: location aliasing requires types to have the same width
>From the OpenGL 4.60.5 spec, section 4.4.1 Input Layout Qualifiers,
Page 67, (Location aliasing):
" Further, when location aliasing, the aliases sharing the location
must have the same underlying numerical type and bit
width (floating-point or integer, 32-bit versus 64-bit, etc.) and
the same auxiliary storage and interpolation qualification."
Additionally, we have improved the linker error descriptions.
Specifically, when taking structs into account we were producing a
linker error because we assumed that all components in each location
were used and that would cause component aliasing. This is not
accurate of the actual problem. Now, the failure specifies that the
underlying numerical type incompatibility is the cause for the
failure.
Fixes the following piglit test:
tests/spec/arb_enhanced_layouts/linker/component-layout/vs-to-fs-width-mismatch-double-float.shader_test
v2:
- Do not assert if we see invalid numerical types. These come
straight from shader code, so we should produce linker errors if
shaders attempt to do location aliasing on variables that are not
numerical such as records.
- While we are at it, improve error reporting for the case of
numerical type mismatch to include the shader stage.
v3:
- Allow location aliasing of images and samplers. If we get these
it means bindless support is active and they should be handled
as 64-bit integers (Ilia)
- Make sure we produce link errors for any non-numerical type
for which we attempt location aliasing, not just structs.
v4:
- Rebased with minor fixes (Andres).
- Added fixing tag to the commit log (Andres).
v5:
- Remove the helper function and check individually for the
underlying numerical type and bit width (Timothy).
- Implicitly, assume that any non-treated type which is checked for
its underlying numerical type is either integer or
float and has a defined bit width (Timothy).
- Implicitly, assume that structs are the only non-treated
non-numerical type (Timothy).
- Improve the linker error descriptions and commit log (Andres).
Fixes: 13652e7516a ("glsl/linker: Fix type checks for location aliasing")
Cc: Ilia Mirkin <imirkin at alum.mit.edu>
Cc: Timothy Arceri <tarceri at itsqueeze.com>
Cc: Iago Toral Quiroga <itoral at igalia.com>
Signed-off-by: Andres Gomez <agomez at igalia.com>
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
(cherry picked from commit 75a3dd97aaeb5fea72ab432c8e9f4bd4e50877ed)
[Andres Gomez: is_record() instead of is_struct() and brought glsl_base_type_get_bit_size]
Signed-off-by: Andres Gomez <agomez at igalia.com>
---
src/compiler/glsl/link_varyings.cpp | 115 ++++++++++++++++++++++++------------
src/compiler/glsl_types.h | 37 ++++++++++++
src/compiler/nir_types.h | 32 +---------
3 files changed, 114 insertions(+), 70 deletions(-)
diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 54ad3d67f6a..28187e2f0a4 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -424,28 +424,14 @@ compute_variable_location_slot(ir_variable *var, gl_shader_stage stage)
struct explicit_location_info {
ir_variable *var;
- unsigned numerical_type;
+ bool base_type_is_integer;
+ unsigned base_type_bit_size;
unsigned interpolation;
bool centroid;
bool sample;
bool patch;
};
-static inline unsigned
-get_numerical_type(const glsl_type *type)
-{
- /* From the OpenGL 4.6 spec, section 4.4.1 Input Layout Qualifiers, Page 68,
- * (Location aliasing):
- *
- * "Further, when location aliasing, the aliases sharing the location
- * must have the same underlying numerical type (floating-point or
- * integer)
- */
- if (type->is_float() || type->is_double())
- return GLSL_TYPE_FLOAT;
- return GLSL_TYPE_INT;
-}
-
static bool
check_location_aliasing(struct explicit_location_info explicit_locations[][4],
ir_variable *var,
@@ -461,14 +447,23 @@ check_location_aliasing(struct explicit_location_info explicit_locations[][4],
gl_shader_stage stage)
{
unsigned last_comp;
- if (type->without_array()->is_record()) {
- /* The component qualifier can't be used on structs so just treat
- * all component slots as used.
+ unsigned base_type_bit_size;
+ const glsl_type *type_without_array = type->without_array();
+ const bool base_type_is_integer =
+ glsl_base_type_is_integer(type_without_array->base_type);
+ const bool is_struct = type_without_array->is_record();
+ if (is_struct) {
+ /* structs don't have a defined underlying base type so just treat all
+ * component slots as used and set the bit size to 0. If there is
+ * location aliasing, we'll fail anyway later.
*/
last_comp = 4;
+ base_type_bit_size = 0;
} else {
- unsigned dmul = type->without_array()->is_64bit() ? 2 : 1;
- last_comp = component + type->without_array()->vector_elements * dmul;
+ unsigned dmul = type_without_array->is_64bit() ? 2 : 1;
+ last_comp = component + type_without_array->vector_elements * dmul;
+ base_type_bit_size =
+ glsl_base_type_get_bit_size(type_without_array->base_type);
}
while (location < location_limit) {
@@ -478,8 +473,22 @@ check_location_aliasing(struct explicit_location_info explicit_locations[][4],
&explicit_locations[location][comp];
if (info->var) {
- /* Component aliasing is not alloed */
- if (comp >= component && comp < last_comp) {
+ if (info->var->type->without_array()->is_record() || is_struct) {
+ /* Structs cannot share location since they are incompatible
+ * with any other underlying numerical type.
+ */
+ linker_error(prog,
+ "%s shader has multiple %sputs sharing the "
+ "same location that don't have the same "
+ "underlying numerical type. Struct variable '%s', "
+ "location %u\n",
+ _mesa_shader_stage_to_string(stage),
+ var->data.mode == ir_var_shader_in ? "in" : "out",
+ is_struct ? var->name : info->var->name,
+ location);
+ return false;
+ } else if (comp >= component && comp < last_comp) {
+ /* Component aliasing is not allowed */
linker_error(prog,
"%s shader has multiple %sputs explicitly "
"assigned to location %d and component %d\n",
@@ -488,27 +497,52 @@ check_location_aliasing(struct explicit_location_info explicit_locations[][4],
location, comp);
return false;
} else {
- /* For all other used components we need to have matching
- * types, interpolation and auxiliary storage
+ /* From the OpenGL 4.60.5 spec, section 4.4.1 Input Layout
+ * Qualifiers, Page 67, (Location aliasing):
+ *
+ * " Further, when location aliasing, the aliases sharing the
+ * location must have the same underlying numerical type
+ * and bit width (floating-point or integer, 32-bit versus
+ * 64-bit, etc.) and the same auxiliary storage and
+ * interpolation qualification."
+ */
+
+ /* If the underlying numerical type isn't integer, implicitly
+ * it will be float or else we would have failed by now.
*/
- if (info->numerical_type !=
- get_numerical_type(type->without_array())) {
+ if (info->base_type_is_integer != base_type_is_integer) {
linker_error(prog,
- "Varyings sharing the same location must "
- "have the same underlying numerical type. "
- "Location %u component %u\n",
- location, comp);
+ "%s shader has multiple %sputs sharing the "
+ "same location that don't have the same "
+ "underlying numerical type. Location %u "
+ "component %u.\n",
+ _mesa_shader_stage_to_string(stage),
+ var->data.mode == ir_var_shader_in ?
+ "in" : "out", location, comp);
+ return false;
+ }
+
+ if (info->base_type_bit_size != base_type_bit_size) {
+ linker_error(prog,
+ "%s shader has multiple %sputs sharing the "
+ "same location that don't have the same "
+ "underlying numerical bit size. Location %u "
+ "component %u.\n",
+ _mesa_shader_stage_to_string(stage),
+ var->data.mode == ir_var_shader_in ?
+ "in" : "out", location, comp);
return false;
}
if (info->interpolation != interpolation) {
linker_error(prog,
- "%s shader has multiple %sputs at explicit "
- "location %u with different interpolation "
- "settings\n",
+ "%s shader has multiple %sputs sharing the "
+ "same location that don't have the same "
+ "interpolation qualification. Location %u "
+ "component %u.\n",
_mesa_shader_stage_to_string(stage),
var->data.mode == ir_var_shader_in ?
- "in" : "out", location);
+ "in" : "out", location, comp);
return false;
}
@@ -516,17 +550,20 @@ check_location_aliasing(struct explicit_location_info explicit_locations[][4],
info->sample != sample ||
info->patch != patch) {
linker_error(prog,
- "%s shader has multiple %sputs at explicit "
- "location %u with different aux storage\n",
+ "%s shader has multiple %sputs sharing the "
+ "same location that don't have the same "
+ "auxiliary storage qualification. Location %u "
+ "component %u.\n",
_mesa_shader_stage_to_string(stage),
var->data.mode == ir_var_shader_in ?
- "in" : "out", location);
+ "in" : "out", location, comp);
return false;
}
}
} else if (comp >= component && comp < last_comp) {
info->var = var;
- info->numerical_type = get_numerical_type(type->without_array());
+ info->base_type_is_integer = base_type_is_integer;
+ info->base_type_bit_size = base_type_bit_size;
info->interpolation = interpolation;
info->centroid = centroid;
info->sample = sample;
diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
index bd22fe0dc10..4767d197449 100644
--- a/src/compiler/glsl_types.h
+++ b/src/compiler/glsl_types.h
@@ -31,6 +31,7 @@
#include "shader_enums.h"
#include "blob.h"
#include "c11/threads.h"
+#include "util/macros.h"
#ifdef __cplusplus
#include "main/config.h"
@@ -114,6 +115,42 @@ static inline bool glsl_base_type_is_integer(enum glsl_base_type type)
type == GLSL_TYPE_IMAGE;
}
+static inline unsigned int
+glsl_base_type_get_bit_size(const enum glsl_base_type base_type)
+{
+ switch (base_type) {
+ case GLSL_TYPE_BOOL:
+ return 1;
+
+ case GLSL_TYPE_INT:
+ case GLSL_TYPE_UINT:
+ case GLSL_TYPE_FLOAT: /* TODO handle mediump */
+ case GLSL_TYPE_SUBROUTINE:
+ return 32;
+
+ case GLSL_TYPE_FLOAT16:
+ case GLSL_TYPE_UINT16:
+ case GLSL_TYPE_INT16:
+ return 16;
+
+ case GLSL_TYPE_UINT8:
+ case GLSL_TYPE_INT8:
+ return 8;
+
+ case GLSL_TYPE_DOUBLE:
+ case GLSL_TYPE_INT64:
+ case GLSL_TYPE_UINT64:
+ case GLSL_TYPE_IMAGE:
+ case GLSL_TYPE_SAMPLER:
+ return 64;
+
+ default:
+ unreachable("unknown base type");
+ }
+
+ return 0;
+}
+
enum glsl_sampler_dim {
GLSL_SAMPLER_DIM_1D = 0,
GLSL_SAMPLER_DIM_2D,
diff --git a/src/compiler/nir_types.h b/src/compiler/nir_types.h
index 8f419817d8c..eb5cdf0a089 100644
--- a/src/compiler/nir_types.h
+++ b/src/compiler/nir_types.h
@@ -97,37 +97,7 @@ unsigned glsl_atomic_size(const struct glsl_type *type);
static inline unsigned
glsl_get_bit_size(const struct glsl_type *type)
{
- switch (glsl_get_base_type(type)) {
- case GLSL_TYPE_BOOL:
- return 1;
-
- case GLSL_TYPE_INT:
- case GLSL_TYPE_UINT:
- case GLSL_TYPE_FLOAT: /* TODO handle mediump */
- case GLSL_TYPE_SUBROUTINE:
- return 32;
-
- case GLSL_TYPE_FLOAT16:
- case GLSL_TYPE_UINT16:
- case GLSL_TYPE_INT16:
- return 16;
-
- case GLSL_TYPE_UINT8:
- case GLSL_TYPE_INT8:
- return 8;
-
- case GLSL_TYPE_DOUBLE:
- case GLSL_TYPE_INT64:
- case GLSL_TYPE_UINT64:
- case GLSL_TYPE_IMAGE:
- case GLSL_TYPE_SAMPLER:
- return 64;
-
- default:
- unreachable("unknown base type");
- }
-
- return 0;
+ return glsl_base_type_get_bit_size(glsl_get_base_type(type));
}
bool glsl_type_is_16bit(const struct glsl_type *type);
More information about the mesa-commit
mailing list