On 03/12/2014 01:29 AM, Erik Faye-Lund wrote: > On Wed, Mar 12, 2014 at 1:32 AM, Eric Anholt <e...@anholt.net> wrote: >> Erik Faye-Lund <kusmab...@gmail.com> writes: >> >>> On Wed, Mar 12, 2014 at 12:00 AM, Eric Anholt <e...@anholt.net> wrote: >>>> Erik Faye-Lund <kusmab...@gmail.com> writes: >>>> >>>>> On Tue, Mar 11, 2014 at 7:27 PM, Eric Anholt <e...@anholt.net> wrote: >>>>>> Erik Faye-Lund <kusmab...@gmail.com> writes: >>>>>> >>>>>>> On Tue, Mar 11, 2014 at 2:50 PM, Erik Faye-Lund <kusmab...@gmail.com> >>>>>>> wrote: >>>>>>>> On Mon, Mar 10, 2014 at 11:54 PM, Matt Turner <matts...@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.
I believe the intention was to prevent inter-expression optimizations from causing different shaders from producing different results. Relative to this example, you can imagine: attribute float a, b; uniform c; void main() { a = pow(c, 3.0); b = pow(c, 5.0); } //---------- attribute float a, b; uniform c; void main() { a = pow(c, 9.0); b = pow(c, 5.0); } In the first shader, we might flatten the calculate of a and b to multiplies. a would be c * c * c, and b would be a * c * c. The second shader might generate two POW instructions. The invariant keyword would (hypothetically) force the calculation of b to be the same in both. If you had a similar situation involving the calculation of gl_Position, you could get mis-aligned geometry. Right now we do barely more than ignore the invariant keyword. I don't feel like this (or the other proposed) optimization is going to make this worth. I was chatting with Matt on IRC, and I feel like we should revisit our poor treatment of invariance when we start implementing the precise keyword with tessellation shaders. FWIW... there are zero occurances of invariant in shader-db. It's hard to get excited about doing work to support a feature that doesn't see much (if any!) use. :( > So yeah, perhaps. But I wouldn't feel safe about optimizations like > this without a clarification from Khronos. > _______________________________________________ > 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