[Mesa-dev] [PATCH 05/22] glsl: Add sqrt, rsq, exp, exp2 to get_range
Marek Olšák
maraeo at gmail.com
Sun Jan 11 03:57:45 PST 2015
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 <cwabbott0 at gmail.com> wrote:
> On Sat, Jan 3, 2015 at 2:18 PM, Thomas Helland
> <thomashelland90 at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list