On 06/25/2015 05:24 PM, Emil Velikov wrote:
Hi gents,
On 9 June 2015 at 14:09, Francisco Jerez <curroje...@riseup.net> wrote:
Francisco Jerez <curroje...@riseup.net> writes:
Tapani Pälli <tapani.pa...@intel.com> writes:
Desktop GLSL < 130 and GLSL ES < 300 allow sampler array indexing where
index can contain a loop induction variable. This extra check will warn
during linking if some of the indexes could not be turned in to constant
expressions.
v2: warning instead of error for backends that did not enable
UnrollSamplerArrayDynamicIndexing option (have dynamic indexing)
Signed-off-by: Tapani Pälli <tapani.pa...@intel.com>
Cc: "10.5" and "10.6" <mesa-sta...@lists.freedesktop.org>
---
src/glsl/linker.cpp | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 9978380..27d7c18 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -346,6 +346,39 @@ private:
bool uses_non_zero_stream;
};
+/* Class that finds array derefs and check if indexes are dynamic. */
+class dynamic_sampler_array_indexing_visitor : public ir_hierarchical_visitor
+{
+public:
+ dynamic_sampler_array_indexing_visitor() :
+ dynamic_sampler_array_indexing(false)
+ {
+ }
+
+ ir_visitor_status visit_enter(ir_dereference_array *ir)
+ {
+ if (!ir->variable_referenced())
+ return visit_continue;
+
+ if (!ir->variable_referenced()->type->contains_sampler())
+ return visit_continue;
+
+ if (!ir->array_index->constant_expression_value()) {
+ dynamic_sampler_array_indexing = true;
+ return visit_stop;
+ }
+ return visit_continue;
+ }
+
+ bool uses_dynamic_sampler_array_indexing()
+ {
+ return dynamic_sampler_array_indexing;
+ }
+
+private:
+ bool dynamic_sampler_array_indexing;
+};
+
} /* anonymous namespace */
void
@@ -2736,6 +2769,40 @@ build_program_resource_list(struct gl_context *ctx,
*/
}
+/**
+ * This check is done to make sure we allow only constant expression
+ * indexing and "constant-index-expression" (indexing with an expression
+ * that includes loop induction variable).
+ */
+static bool
+validate_sampler_array_indexing(struct gl_context *ctx,
+ struct gl_shader_program *prog)
+{
+ dynamic_sampler_array_indexing_visitor v;
+ for (unsigned i = 0; i < MESA_SHADER_STAGES; i++) {
+ if (prog->_LinkedShaders[i] == NULL)
+ continue;
+
+ bool no_dynamic_indexing =
+ ctx->Const.ShaderCompilerOptions[i].UnrollSamplerArrayDynamicIndexing;
+
+ /* Search for array derefs in shader. */
+ v.run(prog->_LinkedShaders[i]->ir);
+ if (v.uses_dynamic_sampler_array_indexing()) {
+ const char *msg = "sampler arrays indexed with non-constant "
+ "expressions is forbidden in GLSL %s %u";
For the sake of clarity, maybe add that it's sampler array indexing with
*general* non-constant expressions what is forbidden, loop induction
variables are allowed and they are technically a kind of non-constant
expression.
+ /* Backend has indicated that it has no dynamic indexing support. */
+ if (no_dynamic_indexing) {
+ linker_error(prog, msg, prog->IsES ? "ES" : "", prog->Version);
+ return false;
+ } else {
+ linker_warning(prog, msg, prog->IsES ? "ES" : "", prog->Version);
It seems a bit mean to spam the user with another warning at link time,
you've already warned in PATCH 01 that this feature will be removed in
more recent GLSL versions. If you drop the warning:
Er, nevermind, the warning here is indeed subtly different (you are
doing a kind of indexing not considered under the
constant-index-expression wording), disregard my comment about dropping
the warning, it seems fine to warn the user twice.
Reviewed-by: Francisco Jerez <curroje...@riseup.net>
If I understand things correctly the series (patches 1/4 and 4/4 from
stable pov) are reviewed but are not in master. Are there any
obstacles/objections against doing so ?
IMO nothing, they can be applied as is. The extra compiler option and
drivers using that can be considered as additions on top. I will do
these later (patch 3 + some changes on top) on but I've been busy with
other work currently.
Thanks
Emil
// Tapani
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
