[Mesa-dev] [PATCH 1/6] glsl: Optimize pow(x, 2) into x * x.

[Erik Faye-Lund]

On Wed, Mar 12, 2014 at 1:32 AM, Eric Anholt <eric at anholt.net> wrote:
> Erik Faye-Lund <kusmabite at gmail.com> writes:
>
>> On Wed, Mar 12, 2014 at 12:00 AM, Eric Anholt <eric at anholt.net> wrote:
>>> Erik Faye-Lund <kusmabite at gmail.com> writes:
>>>
>>>> On Tue, Mar 11, 2014 at 7:27 PM, Eric Anholt <eric at anholt.net> wrote:
>>>>> Erik Faye-Lund <kusmabite at gmail.com> writes:
>>>>>
>>>>>> On Tue, Mar 11, 2014 at 2:50 PM, Erik Faye-Lund <kusmabite at gmail.com> wrote:
>>>>>>> On Mon, Mar 10, 2014 at 11:54 PM, Matt Turner <mattst88 at gmail.com> wrote:
>>>>>>>> Cuts two instructions out of SynMark's Gl32VSInstancing benchmark.
>>>>>>>> ---
>>>>>>>>  src/glsl/opt_algebraic.cpp | 8 ++++++++
>>>>>>>>  1 file changed, 8 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/src/glsl/opt_algebraic.cpp b/src/glsl/opt_algebraic.cpp
>>>>>>>> index 5c49a78..8494bd9 100644
>>>>>>>> --- a/src/glsl/opt_algebraic.cpp
>>>>>>>> +++ b/src/glsl/opt_algebraic.cpp
>>>>>>>> @@ -528,6 +528,14 @@ ir_algebraic_visitor::handle_expression(ir_expression *ir)
>>>>>>>>        if (is_vec_two(op_const[0]))
>>>>>>>>           return expr(ir_unop_exp2, ir->operands[1]);
>>>>>>>>
>>>>>>>> +      if (is_vec_two(op_const[1])) {
>>>>>>>> +         ir_variable *x = new(ir) ir_variable(ir->operands[1]->type, "x",
>>>>>>>> +                                              ir_var_temporary);
>>>>>>>> +         base_ir->insert_before(x);
>>>>>>>> +         base_ir->insert_before(assign(x, ir->operands[0]));
>>>>>>>> +         return mul(x, x);
>>>>>>>> +      }
>>>>>>>> +
>>>>>>>
>>>>>>> Is this safe? Since many GPUs implement pow(x, y) as exp2(log2(x) *
>>>>>>> y), this will give different results for if y comes from a uniform vs
>>>>>>> if it's a constant, no?
>>>>>
>>>>> Yes, but that wouldn't be covered by the "invariant" keyword.
>>>>>
>>>>>> To be a bit more clear: I don't think this is valid for expressions
>>>>>> writing to variables marked as invariant (or expressions taking part
>>>>>> in the calculations that leads up to invariant variable writes).
>>>>>>
>>>>>> I can't find anything allowing variance like this in the invariance
>>>>>> section of the GLSL 3.30 specifications. In particular, the list
>>>>>> following "To guarantee invariance of a particular output variable
>>>>>> across two programs, the following must also be true" doesn't seem to
>>>>>> require the values to be passed from the same source, only that the
>>>>>> same values are passed. And in this case, the value 2.0 is usually
>>>>>> exactly representable no matter what path it took there.
>>>>>>
>>>>>> Perhaps I'm being a bit too pedantic here, though.
>>>>>
>>>>> This file would do the same thing on the same expression tree in two
>>>>> different programs, so "invariant" is fine (we've probably got other
>>>>> problems with invariant, though).  The keyword you're probably thinking
>>>>> of is "precise", which isn't in GLSL we implement yet.
>>>>
>>>> Are you saying that this only rewrites "x = pow(y, 2.0)" and not
>>>> "const float e = 2.0; x = pow(y, e);"? If so, my point is moot,
>>>> indeed. But if that's *not* the case, then I think we're in trouble
>>>> still.
>>>
>>> The second would also get rewritten, because other passes will move the
>>> 2.0 into the pow.
>>>
>>> I thought I understood your objection, but now I don't.  I think you'll
>>> have to lay out the pair of shaders involving the invariant keyword that
>>> you think that would be broken by this pass.
>>
>> My understanding is that
>> ---8<---
>> invariant varying float v;
>> attribute float a;
>> const float e = 2.0;
>> void main()
>> {
>> v = pow(a, e);
>> }
>> ---8<---
>> and
>> ---8<---
>> invariant varying float v;
>> attribute float a;
>> uniform float e;
>> void main()
>> {
>> v = pow(a, e);
>> }
>> ---8<---
>> ...should produce the exact same result, as long as the latter is
>> passed 2.0 as the uniform e.
>>
>> Because v is marked as invariant, the expressions writing to v are the
>> same, and the values passed in are the same.
>>
>> If we rewrite the first one to do "a * a", we get a different result
>> on implementations that do "exp2(log2(a) * 2.0)" for the latter, due
>> to floating-point normalization in the intermediate steps.
>
> I don't think that's what the spec authors intended from the keyword.  I
> think what they intended was that if you had uniform float e in both
> cases, but different code for setting *other* lvalues, that you'd still
> get the same result in v.

I think that *might* be correct, but it doesn't seem to be what's
actually defined. Invariance comes from ESSL, and FWIW, I was one of
the spec authors in this case. But I don't remember the details down
to this level, and the spec doesn't seem to clarify either.

However, since constant expressions are given some slack wrt how it's
evaluated, I'm inclined to think that you're right about the spirit of
the spec.

AFAIR, we introduced "invariant" to get rid of ftransform (since we'd
already gotten rid of fixed-function state), so that multi-pass
rendering algorthms could be guaranteed that all passes ended up
covering the exact same fragments at the exact same depth coordinate.
And in cases like that, the inputs would really be of the same kind
all the time, I guess.

So yeah, perhaps. But I wouldn't feel safe about optimizations like
this without a clarification from Khronos.