[Mesa-dev] [PATCH 11/16] glsl: Add ir_binop_pow to get_range

[Thomas Helland]

2014-11-17 18:28 GMT+01:00 Thomas Helland <thomashelland90 at gmail.com>:
> 2014-11-17 15:48 GMT+01:00 Bruno Jimenez <brunojimen at gmail.com>:
>> On Mon, 2014-11-17 at 02:51 +0100, Thomas Helland wrote:
>>> The spec states that pow is undefined for x < 0.
>>> Just set the range to correspond to a constant 0
>>> if this is the case.
>>> ---
>>>  src/glsl/opt_minmax.cpp | 11 +++++++++++
>>>  1 file changed, 11 insertions(+)
>>>
>>> diff --git a/src/glsl/opt_minmax.cpp b/src/glsl/opt_minmax.cpp
>>> index 9852dd9..ad8c88a 100644
>>> --- a/src/glsl/opt_minmax.cpp
>>> +++ b/src/glsl/opt_minmax.cpp
>>> @@ -335,6 +335,17 @@ get_range(ir_rvalue *rval)
>>>              high = add(r0.high, r1.high)->constant_expression_value();
>>>           return minmax_range(low, high);
>>>
>>> +      case ir_binop_pow:
>>> +         r0 = get_range(expr->operands[0]);
>>> +         if (is_greater_than_or_equal_zero(r0.low))
>>                   ^^^^^^
>>
>> Hi,
>>
>> I think that you meant 'less' here.
>>
>> If not, sorry for the noise.
>>
>> - Bruno
>
> Hi,
>
> I think you've just fast-read it, what I meant here was:
> For x^y, if x is positive then we know that the result will always be positive.
>
> I guess you where thinking of the undefined behavior?
> That's handled in the part below.
>
> When I think about it we could cut a line of code here
> by saying that the low bound is always 0.0,
> since x is demanded to be >= 0,0 and it's undefined otherwise.
>
> - Thomas

Forgot to reply to list, sorry for that.
Looking at the commit message I guess that was what threw you off.
It's a bit centered around the undefined behavior,
without mentioning that it also handles the "correct" case.
That should probably be fixed before committing.

- Thomas

>
>>
>>> +            low = new(mem_ctx) ir_constant(0.0f);
>>> +         // Result is undefined so we can set the range to bikeshed.
>>> +         if (is_less_than_zero(r0.high)) {
>>> +            low = new(mem_ctx) ir_constant(0.0f);
>>> +            high = new(mem_ctx) ir_constant(0.0f);
>>> +         }
>>> +         return minmax_range(low, high);
>>> +
>>>        default:
>>>           break;
>>>        }
>>
>>