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.
On Wed, Aug 22, 2018 at 7:51 AM Danylo Piliaiev <[email protected]> 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 <[email protected]> > --- > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
