diff options
author | Iago Toral Quiroga <[email protected]> | 2016-10-18 14:15:36 +0200 |
---|---|---|
committer | Iago Toral Quiroga <[email protected]> | 2016-10-20 08:26:51 +0200 |
commit | 3da08e166415a745139c1127040a24e8a45dc553 (patch) | |
tree | 01ac2a05f799cbd2c81c2d079f7c274ae9c73fe8 /src/compiler/glsl/link_uniform_blocks.cpp | |
parent | cd45d758ff87305ceecca899fe7325779bb6755b (diff) |
glsl: Indirect array indexing on non-last SSBO member must fail compilation
After the changes in comit 5b2675093e863a52, we moved this check to the
linker, but the spec expects this to be checked at compile-time. There are
dEQP tests that expect an error at compile time and the spec seems to confirm
that expectation:
"Except for the last declared member of a shader storage block (section 4.3.9
“Interface Blocks”), the size of an array must be declared (explicitly sized)
before it is indexed with anything other than an integral constant expression.
The size of any array must be declared before passing it as an argument to a
function. Violation of any of these rules result in compile-time errors. It
is legal to declare an array without a size (unsized) and then later
redeclare the same name as an array of the same type and specify a size, or
index it only with integral constant expressions (implicitly sized)."
Commit 5b2675093e863a52 tries to take care of the case where we have implicitly
sized arrays in SSBOs and it does so by checking the max_array_access field
in ir_variable during linking. In this patch we change the approach: we look
for indirect access on SSBO arrays, and when we find one, we emit a
compile-time error if the accessed member is not the last in the SSBO
definition.
There is a corner case that the specs do not address directly though and that
dEQP checks for: the case of an unsized array in an SSBO definition that is
not defined last but is never used in the shader code either. The following
dEQP tests expect a compile-time error in this scenario:
dEQP-GLES31.functional.debug.negative_coverage.callbacks.shader.compile_compute_shader
dEQP-GLES31.functional.debug.negative_coverage.get_error.shader.compile_compute_shader
dEQP-GLES31.functional.debug.negative_coverage.log.shader.compile_compute_shader
However, since the unsized array is never used it is never indexed with a
non-constant expression, so by the spec quotation above, it should be valid and
the tests are probably incorrect.
Reviewed-by: Nicolai Hähnle <[email protected]>
Diffstat (limited to 'src/compiler/glsl/link_uniform_blocks.cpp')
-rw-r--r-- | src/compiler/glsl/link_uniform_blocks.cpp | 8 |
1 files changed, 1 insertions, 7 deletions
diff --git a/src/compiler/glsl/link_uniform_blocks.cpp b/src/compiler/glsl/link_uniform_blocks.cpp index 5b0dff6aa19..bb423c55410 100644 --- a/src/compiler/glsl/link_uniform_blocks.cpp +++ b/src/compiler/glsl/link_uniform_blocks.cpp @@ -150,13 +150,7 @@ private: */ const glsl_type *type_for_size = type; if (type->is_unsized_array()) { - if (!last_field) { - linker_error(prog, "unsized array `%s' definition: " - "only last member of a shader storage block " - "can be defined as unsized array", - name); - } - + assert(last_field); type_for_size = type->without_array(); } |