<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> Hi Jason, <div class="moz-cite-prefix">On 8/22/18 7:07 PM, Jason Ekstrand wrote: </div> <blockquote type="cite" cite="mid:CAOFGe96JDeA8Z1aMAkKobXw_NXgGez5xp2EnETxfaNL2df6F2Q@mail.gmail.com"> <meta http-equiv="content-type" content="text/html; charset=UTF-8"> <div dir="ltr">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. </div> </blockquote> 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 <tt>linker_optimisation_loop</tt>) is done after '<tt>link_intrastage_shaders</tt>' (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 - <tt>cross_validate_globals</tt> is called in <tt>link_intrastage_shaders</tt> 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 '<tt>link_intrastage_shaders</tt>' 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. <blockquote type="cite" cite="mid:CAOFGe96JDeA8Z1aMAkKobXw_NXgGez5xp2EnETxfaNL2df6F2Q@mail.gmail.com"> <div class="gmail_quote"> <div dir="ltr">On Wed, Aug 22, 2018 at 7:51 AM Danylo Piliaiev <<a href="mailto:danylo.piliaiev@gmail.com" moz-do-not-send="true">danylo.piliaiev@gmail.com</a>> wrote: </div> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=100316" rel="noreferrer" target="_blank" moz-do-not-send="true">https://bugs.freedesktop.org/show_bug.cgi?id=100316</a> Signed-off-by: Danylo Piliaiev <<a href="mailto:danylo.piliaiev@globallogic.com" target="_blank" moz-do-not-send="true">danylo.piliaiev@globallogic.com</a>> --- 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 </blockquote> </div> </blockquote> </body> </html>