Hi,
I went ahead and sent the patch which implements my proposed solution:
"[PATCH] glsl: Cross validate variable's invariance by explicit
invariance only".
In relation to the new patch this one becomes cosmetic since while the
new patch solves linking errors, it doesn't remove invariant qualifier
from uniforms.
- Danil
On 8/23/18 2:49 PM, Danylo Piliaiev wrote:
Hi Jason,
On 8/22/18 7:07 PM, Jason Ekstrand wrote:
I think this would be correct to do and would fix the bug at hand but
there may be other bugs lurking deeper. In particular, if you have
multiple shaders for the same stage that share some global variables
and get linked together, I think you could run into the same issue
only with a read-write global. If this is indeed possible, then I'm
not sure what to do. We have to run this pass prior to doing any
optimization because we depend on it to get precise and invariant
correct. On the other hand, it has to be run after linking. :-/
Maybe we just have to assume precise and invariant for all
pre-linking optimizations and then run this post-linking.
I didn't check this case, thanks for pointing it out. Now I have
looked at relevant code and saw that invariant propagation
(linker.cpp:5024 in linker_optimisation_loop) is done after
'link_intrastage_shaders' (linker.cpp:4868) which combines a group of
shaders for a single stage. Actually invariant propagation is also
done once before linking. In addition I've done some tests and
invariance from variable in one vertex shader is propagated on
variables directly used only in other vertex shader.
However I have found other issue - cross_validate_globals is called in
link_intrastage_shaders and throws error if 'invariant' qualifier
doesn't match between shaders in same stage which happens when
variable in one shader becomes invariant before the
'link_intrastage_shaders' call. This could be fixed separately.
Or there is another way to solve these issues: divide 'invariant'
qualifier into explicit and implicit 'invariant', where explicit is
for 'invariant' set by user in shader and implicit is for 'invariant'
propagated from other invariants. Thus in interface checking we will
be able to check only for explicit invariance which besides two
previous issues with solve another one: if out variable is marked as
invariant due to invariance propagation this variable will be required
to have invariant qualifier in the next stage even though user never
marked it as such anywhere. And when the invariance will actually
matter we will check both implicit and explicit one. What do you think
about such solution?
- Danil.
On Wed, Aug 22, 2018 at 7:51 AM Danylo Piliaiev
<danylo.pilia...@gmail.com <mailto:danylo.pilia...@gmail.com>> wrote:
Read-only variables could be considered inherently invariant and
precise
since no expression in shader affects them.
Explicitly marking them as such is unnecessary and can cause issues,
e.g. uniform marked as invariant may require the same uniform in
other
stage to be invariant too but uniforms cannot have "invariant"
qualifier.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=100316
Signed-off-by: Danylo Piliaiev <danylo.pilia...@globallogic.com
<mailto:danylo.pilia...@globallogic.com>>
---
My assumption is that variable being read_only means that it either
loads a value from an external source or is initialized from
constant value.
From what I saw in code it's true but it may be wrong to depend
on it.
src/compiler/glsl/propagate_invariance.cpp | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/src/compiler/glsl/propagate_invariance.cpp
b/src/compiler/glsl/propagate_invariance.cpp
index b3f1d810cd..0e4233aa9f 100644
--- a/src/compiler/glsl/propagate_invariance.cpp
+++ b/src/compiler/glsl/propagate_invariance.cpp
@@ -96,6 +96,16 @@
ir_invariance_propagation_visitor::visit(ir_dereference_variable *ir)
if (this->dst_var == NULL)
return visit_continue;
+ /* Read-only variables could be considered inherently
+ * invariant and precise since no expression in shader
affects them.
+ * Explicitly marking them as such is unnecessary and can cause
+ * issues, e.g. uniform marked as invariant may require the same
+ * uniform in other stage to be invariant too but uniforms cannot
+ * have "invariant" qualifier.
+ */
+ if (ir->var->data.read_only)
+ return visit_continue;
+
if (this->dst_var->data.invariant) {
if (!ir->var->data.invariant)
this->progress = true;
--
2.18.0
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
