Hi Tim,
it's been a while, so off the top of my head I don't have any
particular suggestions or the capacity to tell whether your fix is
correct or not :-/.
I'll try to spend some time re-acquainting myself with that code on
Monday and see if I can see what is going on here.
Iago
On Fri, 2017-01-06 at 22:08 +1100, Timothy Arceri wrote:
> Hi Iago/Petri,
>
> Just Ccing you guys in case you have a better solution (I know its
> been
> a while since this was written).
>
> This is causing incorrect shaders in at least Serious Sam 3 possibly
> others. I've sent some piglit tests to reproduce it to the piglit
> list.
>
> Thanks,
> Tim
>
> On Fri, 2017-01-06 at 10:26 +1100, Timothy Arceri wrote:
> >
> > Marking operations as redundant if they are equal to the base
> > range is fine when the tree structure is something like this:
> >
> > max
> > / \
> > max b
> > / \
> > 3 max
> > / \
> > 3 a
> >
> > But the opt falls apart with a tree like this:
> >
> > max
> > / \
> > max max
> > / \ / \
> > 3 a b 3
> >
> > I'm not 100% sure what is going wrong as a tree like:
> >
> > max
> > / \
> > max max
> > / \ / \
> > 3 a b 4
> >
> > Will remove the right hand side max just fine. But not marking
> > limits that equal the base range seems to fix the problem.
> >
> > NIR algebraic opt will clean up the first tree for anyway,
> > hopefully
> > other backends are smart enough to do this also.
> >
> > Cc: "13.0" <mesa-sta...@lists.freedesktop.org>
> > ---
> > src/compiler/glsl/opt_minmax.cpp | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/compiler/glsl/opt_minmax.cpp
> > b/src/compiler/glsl/opt_minmax.cpp
> > index 29482ee..9f64db9 100644
> > --- a/src/compiler/glsl/opt_minmax.cpp
> > +++ b/src/compiler/glsl/opt_minmax.cpp
> > @@ -355,7 +355,7 @@
> > ir_minmax_visitor::prune_expression(ir_expression
> > *expr, minmax_range baserange)
> > */
> > if (!is_redundant && limits[i].low && baserange.high) {
> > cr = compare_components(limits[i].low,
> > baserange.high);
> > - if (cr >= EQUAL && cr != MIXED)
> > + if (cr > EQUAL && cr != MIXED)
> > is_redundant = true;
> > }
> > } else {
> > @@ -373,7 +373,7 @@
> > ir_minmax_visitor::prune_expression(ir_expression
> > *expr, minmax_range baserange)
> > */
> > if (!is_redundant && limits[i].high && baserange.low) {
> > cr = compare_components(limits[i].high,
> > baserange.low);
> > - if (cr <= EQUAL)
> > + if (cr < EQUAL)
> > is_redundant = true;
> > }
> > }
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
