On 01/12/2016 12:30 AM, Timothy Arceri wrote:
On Mon, 2016-01-11 at 08:24 +0200, Tapani Pälli wrote:
On 01/08/2016 11:32 AM, Tapani Pälli wrote:
On 01/08/2016 11:17 AM, Timothy Arceri wrote:
On Fri, 2016-01-08 at 08:20 +0200, Tapani Pälli wrote:
Linker missed a check for situation where we exceed max amount
of
uniform locations with explicit + implicit locations. Patch
adds this
check to already existing iteration over uniforms in linker.
Fixes following CTS test:
ES31-CTS.explicit_uniform_location.uniform-loc-negative
-link-max
-num-of-locations
Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
---
src/glsl/linker.cpp | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 7a18523..ef23b36 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -3130,6 +3130,7 @@ check_explicit_uniform_locations(struct
gl_context *ctx,
return;
}
+ unsigned entries_total = 0;
for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
struct gl_shader *sh = prog->_LinkedShaders[i];
@@ -3138,8 +3139,12 @@ check_explicit_uniform_locations(struct
gl_context *ctx,
foreach_in_list(ir_instruction, node, sh->ir) {
ir_variable *var = node->as_variable();
- if (var && (var->data.mode == ir_var_uniform &&
- var->data.explicit_location)) {
+ if (!var || var->data.mode != ir_var_uniform)
+ continue;
+
+ entries_total += var->type->is_array() ? var->type
->length
: 1;
This should be:
entries_total += var->type->uniform_locations();
Otherwise AoA and structs will not be counted correctly.
ah I had forgotten about uniform_locations(), thanks!
+
+ if (var->data.explicit_location) {
bool ret;
if (var->type->is_subroutine())
ret =
reserve_subroutine_explicit_locations(prog, sh,
var);
@@ -3153,6 +3158,14 @@ check_explicit_uniform_locations(struct
gl_context *ctx,
}
}
+ /* Verify that total amount of entries for explicit and
implicit
locations
+ * is less than MAX_UNIFORM_LOCATIONS.
+ */
+ if (entries_total >= ctx
->Const.MaxUserAssignableUniformLocations) {
+ linker_error(prog, "count of uniform locations >=
MAX_UNIFORM_LOCATIONS"
+ "(%u >= %u)", entries_total,
+ ctx
->Const.MaxUserAssignableUniformLocations);
+ }
I think this check would be better in
link_assign_uniform_locations()
because check_explicit_uniform_locations() is called before
arrays are
resized via update_array_sizes()
Also in link_assign_uniform_locations() there is already a count
of
uniform locations.
See: const unsigned num_uniforms =
uniform_size.num_active_uniforms;
That function can't currently fail and it does not know the max no
of
uniforms so it was easier to plug here. I can move it there but it
requires changing the function signature a bit, I guess not that
big
deal though.
I've made a version that counts used locations only at
link_assign_uniform_locations(). Problem with that is that it happens
too late. With the test case an uniform array gets optimized away
(since
it's not used in the shader, however as it is using explicit
location,
those locations must be reserved), therefore this happens too late.
IMO
we must do this calculation as the my first proposal did it.
Hmm, sure they must be reserved but if they are inactive I don't think
there is anything in the spec that disallows using the location for
varyings that don't have an explicit location.
I don't think overlap between these matter as these are distinct
resources, used for different things.
Anyways its not a big deal I guess, if you fix up the issue above you
can have.
Reviewed-by: Timothy Arceri <timothy.arc...@collabora.com>
Thanks for the review!
// Tapani
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
