Re: [Mesa-dev] [PATCH 05/22] glsl: Add sqrt, rsq, exp, exp2 to get_range
gl_Position.z isn't bound by any range. It doesn't have to be clipped
(it can be clamped instead), but if it's clipped, the value outside of
the range still affects the whole primitive that can be at least
partially visible.
Marek
On Sat, Jan 17, 2015 at 5:07 PM, Thomas Helland
thomashellan...@gmail.com wrote:
I see why you are worried, and I agree 100%.
This just reinforces my impression that expanding this pass does
not give adequate return on investment.
If we had even better coverage we just might get some advantage,
but even then I have a bad feeling about this.
Do you have any suggestions for operations apart from
expressions and constants that we can get a range of?
If so I could work on it some more to figure out if this is
getting us anywhere at all. If I recall correctly the z
component of gl_Position is bound between 0 and 1?
2015-01-09 4:15 GMT+01:00 Connor Abbott cwabbo...@gmail.com:
On Sat, Jan 3, 2015 at 2:18 PM, Thomas Helland
thomashellan...@gmail.com wrote:
Also handle undefined behaviour for sqrt(x) where x 0
and rsq(x) where x = 0.
This gives us some reduction in instruction count on three
Dungeon Defenders shaders as they are doing: max(exp(x), 0)
So initially when you said that Dungeon Defenders was doing
max(exp(x), 0), my thought was wat? but after thinking about it some
more, I can see why it would do this. The GLSL spec doesn't guarantee
that implementations of +, *, exp(), etc. will return NaN when one of
the arguments is NaN, but it also doesn't guarantee that they *won't*;
in other words, if for some strange reason you need the old-style
never-return-NaN functionality, you need to do something like what
this game is doing. For implementations that don't return NaN, this
optimization is just fine, but if you remove it when the HW does
return NaN, then whatever's using the result might get a NaN when it's
not expecting it, leading to Bad Things happening. Maybe it isn't an
issue with this particular game, but in order to be correct here it
seems like we do have to take NaN's into account after all.
There was a related thread (and other discussions) about the behavior
of min/max wrt NaN's:
http://lists.freedesktop.org/archives/mesa-dev/2014-December/073182.html
My conclusion is that basically everyone that actually produces NaN's
follows the IEEE/D3D behavior here, which I'm assuming the Dungeon
Defenders developers were probably depending on.
v2: Change to use new IS_CONSTANT() macro
Fix high unintenionally not being returned
Add some air for readability
Comment on the exploit of undefined behavior
Constify mem_ctx
---
src/glsl/opt_minmax.cpp | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
index 56805c0..2faa3c3 100644
--- a/src/glsl/opt_minmax.cpp
+++ b/src/glsl/opt_minmax.cpp
@@ -274,9 +274,40 @@ get_range(ir_rvalue *rval)
minmax_range r0;
minmax_range r1;
+ void *const mem_ctx = ralloc_parent(rval);
+
+ ir_constant *low = NULL;
+ ir_constant *high = NULL;
+
if (expr) {
switch (expr-operation) {
+ case ir_unop_exp:
+ case ir_unop_exp2:
+ case ir_unop_sqrt:
+ case ir_unop_rsq:
+ r0 = get_range(expr-operands[0]);
+
+ /* The spec says sqrt is undefined if x 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_sqrt
+ IS_CONSTANT(r0.high, , 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* The spec says rsq is undefined if x = 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_rsq
+ IS_CONSTANT(r0.high, =, 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* TODO: If we know, i.e, the lower range of the operand
+ * we can calculate the lower range
+ */
+ low = new(mem_ctx) ir_constant(0.0f);
+ return minmax_range(low, high);
+
case ir_binop_min:
case ir_binop_max:
r0 = get_range(expr-operands[0]);
--
2.2.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/22] glsl: Add sqrt, rsq, exp, exp2 to get_range
I see why you are worried, and I agree 100%.
This just reinforces my impression that expanding this pass does
not give adequate return on investment.
If we had even better coverage we just might get some advantage,
but even then I have a bad feeling about this.
Do you have any suggestions for operations apart from
expressions and constants that we can get a range of?
If so I could work on it some more to figure out if this is
getting us anywhere at all. If I recall correctly the z
component of gl_Position is bound between 0 and 1?
2015-01-09 4:15 GMT+01:00 Connor Abbott cwabbo...@gmail.com:
On Sat, Jan 3, 2015 at 2:18 PM, Thomas Helland
thomashellan...@gmail.com wrote:
Also handle undefined behaviour for sqrt(x) where x 0
and rsq(x) where x = 0.
This gives us some reduction in instruction count on three
Dungeon Defenders shaders as they are doing: max(exp(x), 0)
So initially when you said that Dungeon Defenders was doing
max(exp(x), 0), my thought was wat? but after thinking about it some
more, I can see why it would do this. The GLSL spec doesn't guarantee
that implementations of +, *, exp(), etc. will return NaN when one of
the arguments is NaN, but it also doesn't guarantee that they *won't*;
in other words, if for some strange reason you need the old-style
never-return-NaN functionality, you need to do something like what
this game is doing. For implementations that don't return NaN, this
optimization is just fine, but if you remove it when the HW does
return NaN, then whatever's using the result might get a NaN when it's
not expecting it, leading to Bad Things happening. Maybe it isn't an
issue with this particular game, but in order to be correct here it
seems like we do have to take NaN's into account after all.
There was a related thread (and other discussions) about the behavior
of min/max wrt NaN's:
http://lists.freedesktop.org/archives/mesa-dev/2014-December/073182.html
My conclusion is that basically everyone that actually produces NaN's
follows the IEEE/D3D behavior here, which I'm assuming the Dungeon
Defenders developers were probably depending on.
v2: Change to use new IS_CONSTANT() macro
Fix high unintenionally not being returned
Add some air for readability
Comment on the exploit of undefined behavior
Constify mem_ctx
---
src/glsl/opt_minmax.cpp | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
index 56805c0..2faa3c3 100644
--- a/src/glsl/opt_minmax.cpp
+++ b/src/glsl/opt_minmax.cpp
@@ -274,9 +274,40 @@ get_range(ir_rvalue *rval)
minmax_range r0;
minmax_range r1;
+ void *const mem_ctx = ralloc_parent(rval);
+
+ ir_constant *low = NULL;
+ ir_constant *high = NULL;
+
if (expr) {
switch (expr-operation) {
+ case ir_unop_exp:
+ case ir_unop_exp2:
+ case ir_unop_sqrt:
+ case ir_unop_rsq:
+ r0 = get_range(expr-operands[0]);
+
+ /* The spec says sqrt is undefined if x 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_sqrt
+ IS_CONSTANT(r0.high, , 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* The spec says rsq is undefined if x = 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_rsq
+ IS_CONSTANT(r0.high, =, 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* TODO: If we know, i.e, the lower range of the operand
+ * we can calculate the lower range
+ */
+ low = new(mem_ctx) ir_constant(0.0f);
+ return minmax_range(low, high);
+
case ir_binop_min:
case ir_binop_max:
r0 = get_range(expr-operands[0]);
--
2.2.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/22] glsl: Add sqrt, rsq, exp, exp2 to get_range
Well, max(x, 0) has no effect on NaNs. According to the GLSL
specification, max() should return x if either argument is NaN. So the
correct way to convert NaN to 0 is:
max(0, y)
That implies that min and max aren't commutative if NaNs are supported.
Marek
On Fri, Jan 9, 2015 at 4:15 AM, Connor Abbott cwabbo...@gmail.com wrote:
On Sat, Jan 3, 2015 at 2:18 PM, Thomas Helland
thomashellan...@gmail.com wrote:
Also handle undefined behaviour for sqrt(x) where x 0
and rsq(x) where x = 0.
This gives us some reduction in instruction count on three
Dungeon Defenders shaders as they are doing: max(exp(x), 0)
So initially when you said that Dungeon Defenders was doing
max(exp(x), 0), my thought was wat? but after thinking about it some
more, I can see why it would do this. The GLSL spec doesn't guarantee
that implementations of +, *, exp(), etc. will return NaN when one of
the arguments is NaN, but it also doesn't guarantee that they *won't*;
in other words, if for some strange reason you need the old-style
never-return-NaN functionality, you need to do something like what
this game is doing. For implementations that don't return NaN, this
optimization is just fine, but if you remove it when the HW does
return NaN, then whatever's using the result might get a NaN when it's
not expecting it, leading to Bad Things happening. Maybe it isn't an
issue with this particular game, but in order to be correct here it
seems like we do have to take NaN's into account after all.
There was a related thread (and other discussions) about the behavior
of min/max wrt NaN's:
http://lists.freedesktop.org/archives/mesa-dev/2014-December/073182.html
My conclusion is that basically everyone that actually produces NaN's
follows the IEEE/D3D behavior here, which I'm assuming the Dungeon
Defenders developers were probably depending on.
v2: Change to use new IS_CONSTANT() macro
Fix high unintenionally not being returned
Add some air for readability
Comment on the exploit of undefined behavior
Constify mem_ctx
---
src/glsl/opt_minmax.cpp | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
index 56805c0..2faa3c3 100644
--- a/src/glsl/opt_minmax.cpp
+++ b/src/glsl/opt_minmax.cpp
@@ -274,9 +274,40 @@ get_range(ir_rvalue *rval)
minmax_range r0;
minmax_range r1;
+ void *const mem_ctx = ralloc_parent(rval);
+
+ ir_constant *low = NULL;
+ ir_constant *high = NULL;
+
if (expr) {
switch (expr-operation) {
+ case ir_unop_exp:
+ case ir_unop_exp2:
+ case ir_unop_sqrt:
+ case ir_unop_rsq:
+ r0 = get_range(expr-operands[0]);
+
+ /* The spec says sqrt is undefined if x 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_sqrt
+ IS_CONSTANT(r0.high, , 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* The spec says rsq is undefined if x = 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_rsq
+ IS_CONSTANT(r0.high, =, 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* TODO: If we know, i.e, the lower range of the operand
+ * we can calculate the lower range
+ */
+ low = new(mem_ctx) ir_constant(0.0f);
+ return minmax_range(low, high);
+
case ir_binop_min:
case ir_binop_max:
r0 = get_range(expr-operands[0]);
--
2.2.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/22] glsl: Add sqrt, rsq, exp, exp2 to get_range
Also, you seem to be assuming that IEEE and D3D10 versions of the
min() and max() are the same, which is not true. Both GLSL and D3D10
versions should work according to IEEE754, yet they are different. The
GLSL versions use conditional assignments, which is the same as C. The
D3D10 versions are defined in IEEE as minNum and maxNum and have
different behavior with regard to NaNs (cannot be implemented as one
conditional assignment).
Marek
On Sun, Jan 11, 2015 at 12:57 PM, Marek Olšák mar...@gmail.com wrote:
Well, max(x, 0) has no effect on NaNs. According to the GLSL
specification, max() should return x if either argument is NaN. So the
correct way to convert NaN to 0 is:
max(0, y)
That implies that min and max aren't commutative if NaNs are supported.
Marek
On Fri, Jan 9, 2015 at 4:15 AM, Connor Abbott cwabbo...@gmail.com wrote:
On Sat, Jan 3, 2015 at 2:18 PM, Thomas Helland
thomashellan...@gmail.com wrote:
Also handle undefined behaviour for sqrt(x) where x 0
and rsq(x) where x = 0.
This gives us some reduction in instruction count on three
Dungeon Defenders shaders as they are doing: max(exp(x), 0)
So initially when you said that Dungeon Defenders was doing
max(exp(x), 0), my thought was wat? but after thinking about it some
more, I can see why it would do this. The GLSL spec doesn't guarantee
that implementations of +, *, exp(), etc. will return NaN when one of
the arguments is NaN, but it also doesn't guarantee that they *won't*;
in other words, if for some strange reason you need the old-style
never-return-NaN functionality, you need to do something like what
this game is doing. For implementations that don't return NaN, this
optimization is just fine, but if you remove it when the HW does
return NaN, then whatever's using the result might get a NaN when it's
not expecting it, leading to Bad Things happening. Maybe it isn't an
issue with this particular game, but in order to be correct here it
seems like we do have to take NaN's into account after all.
There was a related thread (and other discussions) about the behavior
of min/max wrt NaN's:
http://lists.freedesktop.org/archives/mesa-dev/2014-December/073182.html
My conclusion is that basically everyone that actually produces NaN's
follows the IEEE/D3D behavior here, which I'm assuming the Dungeon
Defenders developers were probably depending on.
v2: Change to use new IS_CONSTANT() macro
Fix high unintenionally not being returned
Add some air for readability
Comment on the exploit of undefined behavior
Constify mem_ctx
---
src/glsl/opt_minmax.cpp | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
index 56805c0..2faa3c3 100644
--- a/src/glsl/opt_minmax.cpp
+++ b/src/glsl/opt_minmax.cpp
@@ -274,9 +274,40 @@ get_range(ir_rvalue *rval)
minmax_range r0;
minmax_range r1;
+ void *const mem_ctx = ralloc_parent(rval);
+
+ ir_constant *low = NULL;
+ ir_constant *high = NULL;
+
if (expr) {
switch (expr-operation) {
+ case ir_unop_exp:
+ case ir_unop_exp2:
+ case ir_unop_sqrt:
+ case ir_unop_rsq:
+ r0 = get_range(expr-operands[0]);
+
+ /* The spec says sqrt is undefined if x 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_sqrt
+ IS_CONSTANT(r0.high, , 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* The spec says rsq is undefined if x = 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_rsq
+ IS_CONSTANT(r0.high, =, 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* TODO: If we know, i.e, the lower range of the operand
+ * we can calculate the lower range
+ */
+ low = new(mem_ctx) ir_constant(0.0f);
+ return minmax_range(low, high);
+
case ir_binop_min:
case ir_binop_max:
r0 = get_range(expr-operands[0]);
--
2.2.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/22] glsl: Add sqrt, rsq, exp, exp2 to get_range
On Sun, Jan 11, 2015 at 6:57 AM, Marek Olšák mar...@gmail.com wrote:
Well, max(x, 0) has no effect on NaNs. According to the GLSL
specification, max() should return x if either argument is NaN. So the
correct way to convert NaN to 0 is:
max(0, y)
That implies that min and max aren't commutative if NaNs are supported.
Marek
This is exactly why I added the second paragraph... the point is that
basically no-one follows the spec anyways, instead implementing the
minNum and maxNum behavior (fmin() and fmax() in C), hence on every
implementation that does support NaN's, this will force the exp() to
return 0 on NaN. In fact, if you want to be really pedantic, it's
perfectly legal for max(NaN, 0) to return 0 because operations with
NaN inputs don't have to return NaN. Of course, this behavior isn't
required, but it seems to be a de-facto standard because of D3D. It
seems worth preserving this behavior, and more generally being careful
about NaN's when doing range analysis (unless the hardware is known to
never return NaN's), to keep games from potentially breaking on edge
cases like these.
On Fri, Jan 9, 2015 at 4:15 AM, Connor Abbott cwabbo...@gmail.com wrote:
On Sat, Jan 3, 2015 at 2:18 PM, Thomas Helland
thomashellan...@gmail.com wrote:
Also handle undefined behaviour for sqrt(x) where x 0
and rsq(x) where x = 0.
This gives us some reduction in instruction count on three
Dungeon Defenders shaders as they are doing: max(exp(x), 0)
So initially when you said that Dungeon Defenders was doing
max(exp(x), 0), my thought was wat? but after thinking about it some
more, I can see why it would do this. The GLSL spec doesn't guarantee
that implementations of +, *, exp(), etc. will return NaN when one of
the arguments is NaN, but it also doesn't guarantee that they *won't*;
in other words, if for some strange reason you need the old-style
never-return-NaN functionality, you need to do something like what
this game is doing. For implementations that don't return NaN, this
optimization is just fine, but if you remove it when the HW does
return NaN, then whatever's using the result might get a NaN when it's
not expecting it, leading to Bad Things happening. Maybe it isn't an
issue with this particular game, but in order to be correct here it
seems like we do have to take NaN's into account after all.
There was a related thread (and other discussions) about the behavior
of min/max wrt NaN's:
http://lists.freedesktop.org/archives/mesa-dev/2014-December/073182.html
My conclusion is that basically everyone that actually produces NaN's
follows the IEEE/D3D behavior here, which I'm assuming the Dungeon
Defenders developers were probably depending on.
v2: Change to use new IS_CONSTANT() macro
Fix high unintenionally not being returned
Add some air for readability
Comment on the exploit of undefined behavior
Constify mem_ctx
---
src/glsl/opt_minmax.cpp | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
index 56805c0..2faa3c3 100644
--- a/src/glsl/opt_minmax.cpp
+++ b/src/glsl/opt_minmax.cpp
@@ -274,9 +274,40 @@ get_range(ir_rvalue *rval)
minmax_range r0;
minmax_range r1;
+ void *const mem_ctx = ralloc_parent(rval);
+
+ ir_constant *low = NULL;
+ ir_constant *high = NULL;
+
if (expr) {
switch (expr-operation) {
+ case ir_unop_exp:
+ case ir_unop_exp2:
+ case ir_unop_sqrt:
+ case ir_unop_rsq:
+ r0 = get_range(expr-operands[0]);
+
+ /* The spec says sqrt is undefined if x 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_sqrt
+ IS_CONSTANT(r0.high, , 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* The spec says rsq is undefined if x = 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_rsq
+ IS_CONSTANT(r0.high, =, 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* TODO: If we know, i.e, the lower range of the operand
+ * we can calculate the lower range
+ */
+ low = new(mem_ctx) ir_constant(0.0f);
+ return minmax_range(low, high);
+
case ir_binop_min:
case ir_binop_max:
r0 = get_range(expr-operands[0]);
--
2.2.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 05/22] glsl: Add sqrt, rsq, exp, exp2 to get_range
On Sat, Jan 3, 2015 at 2:18 PM, Thomas Helland
thomashellan...@gmail.com wrote:
Also handle undefined behaviour for sqrt(x) where x 0
and rsq(x) where x = 0.
This gives us some reduction in instruction count on three
Dungeon Defenders shaders as they are doing: max(exp(x), 0)
So initially when you said that Dungeon Defenders was doing
max(exp(x), 0), my thought was wat? but after thinking about it some
more, I can see why it would do this. The GLSL spec doesn't guarantee
that implementations of +, *, exp(), etc. will return NaN when one of
the arguments is NaN, but it also doesn't guarantee that they *won't*;
in other words, if for some strange reason you need the old-style
never-return-NaN functionality, you need to do something like what
this game is doing. For implementations that don't return NaN, this
optimization is just fine, but if you remove it when the HW does
return NaN, then whatever's using the result might get a NaN when it's
not expecting it, leading to Bad Things happening. Maybe it isn't an
issue with this particular game, but in order to be correct here it
seems like we do have to take NaN's into account after all.
There was a related thread (and other discussions) about the behavior
of min/max wrt NaN's:
http://lists.freedesktop.org/archives/mesa-dev/2014-December/073182.html
My conclusion is that basically everyone that actually produces NaN's
follows the IEEE/D3D behavior here, which I'm assuming the Dungeon
Defenders developers were probably depending on.
v2: Change to use new IS_CONSTANT() macro
Fix high unintenionally not being returned
Add some air for readability
Comment on the exploit of undefined behavior
Constify mem_ctx
---
src/glsl/opt_minmax.cpp | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
index 56805c0..2faa3c3 100644
--- a/src/glsl/opt_minmax.cpp
+++ b/src/glsl/opt_minmax.cpp
@@ -274,9 +274,40 @@ get_range(ir_rvalue *rval)
minmax_range r0;
minmax_range r1;
+ void *const mem_ctx = ralloc_parent(rval);
+
+ ir_constant *low = NULL;
+ ir_constant *high = NULL;
+
if (expr) {
switch (expr-operation) {
+ case ir_unop_exp:
+ case ir_unop_exp2:
+ case ir_unop_sqrt:
+ case ir_unop_rsq:
+ r0 = get_range(expr-operands[0]);
+
+ /* The spec says sqrt is undefined if x 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_sqrt
+ IS_CONSTANT(r0.high, , 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* The spec says rsq is undefined if x = 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_rsq
+ IS_CONSTANT(r0.high, =, 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* TODO: If we know, i.e, the lower range of the operand
+ * we can calculate the lower range
+ */
+ low = new(mem_ctx) ir_constant(0.0f);
+ return minmax_range(low, high);
+
case ir_binop_min:
case ir_binop_max:
r0 = get_range(expr-operands[0]);
--
2.2.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/22] glsl: Add sqrt, rsq, exp, exp2 to get_range
Also handle undefined behaviour for sqrt(x) where x 0
and rsq(x) where x = 0.
This gives us some reduction in instruction count on three
Dungeon Defenders shaders as they are doing: max(exp(x), 0)
v2: Change to use new IS_CONSTANT() macro
Fix high unintenionally not being returned
Add some air for readability
Comment on the exploit of undefined behavior
Constify mem_ctx
---
src/glsl/opt_minmax.cpp | 31 +++
1 file changed, 31 insertions(+)
diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
index 56805c0..2faa3c3 100644
--- a/src/glsl/opt_minmax.cpp
+++ b/src/glsl/opt_minmax.cpp
@@ -274,9 +274,40 @@ get_range(ir_rvalue *rval)
minmax_range r0;
minmax_range r1;
+ void *const mem_ctx = ralloc_parent(rval);
+
+ ir_constant *low = NULL;
+ ir_constant *high = NULL;
+
if (expr) {
switch (expr-operation) {
+ case ir_unop_exp:
+ case ir_unop_exp2:
+ case ir_unop_sqrt:
+ case ir_unop_rsq:
+ r0 = get_range(expr-operands[0]);
+
+ /* The spec says sqrt is undefined if x 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_sqrt
+ IS_CONSTANT(r0.high, , 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* The spec says rsq is undefined if x = 0
+ * We can use this to set the range to whatever we want
+ */
+ if (expr-operation == ir_unop_rsq
+ IS_CONSTANT(r0.high, =, 0.0f))
+high = new(mem_ctx) ir_constant(0.0f);
+
+ /* TODO: If we know, i.e, the lower range of the operand
+ * we can calculate the lower range
+ */
+ low = new(mem_ctx) ir_constant(0.0f);
+ return minmax_range(low, high);
+
case ir_binop_min:
case ir_binop_max:
r0 = get_range(expr-operands[0]);
--
2.2.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
