[Mesa-dev] [PATCH] glsl: Do not propagate 'precise' and 'invariant' on read-only variables

[Jason Ekstrand]

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 <danylo.piliaiev at 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.piliaiev at 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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180822/5f3bb2f9/attachment.html>