Hi Emil;
I got r-b now for all the patches considering this series. At least one
of these (i965: use EmitNoIndirectSampler for gen < 7) does not apply as
is to the 10.6 tree as it is due to other changes.
How does the process go, should I sent a separate patch for 10.6 tree
(there the change should happen in a different file) or do we try to
pull in other patches to make this work?
Thanks;
// Tapani
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 ?
Thanks
Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
