On 06.01.2017 00:26, 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
This will just become max(a, b), right?
The problem is that both branches are treated the same: descending in
the left branch will prune the constant, and then descending the right
branch will prune the constant there as well, because limits[0] wasn't
updated to take the change on the left branch into account, and so we
still get [3,\infty) as baserange.
With your change, nothing will be done at all. That's fine as far as I'm
concerned. I don't see a clean way of updating the limits on-the-fly.
Thanks for the piglit test. An explanatory comment would be nice, but
either way, this patch is
Reviewed-by: Nicolai Hähnle <nicolai.haeh...@amd.com>
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
