On Wed, Mar 18, 2015 at 9:52 AM, Matt Turner <matts...@gmail.com> wrote:
> On Wed, Mar 18, 2015 at 8:27 AM, Jason Ekstrand <ja...@jlekstrand.net> wrote:
>> On Wed, Mar 18, 2015 at 7:22 AM, Connor Abbott <cwabbo...@gmail.com> wrote:
>>> On Wed, Mar 18, 2015 at 1:56 AM, Matt Turner <matts...@gmail.com> wrote:
>>>> Thanks. Looked through stats and at some of the regressions.
>>>>
>>>> Some of the areas I noticed we were doing worse:
>>>>
>>>> We generate two CMPs for discard_if; only one without NIR. I think you
>>>> had an idea about solving this.
>>>>
>>>> SEL peephole interference -- we knew about this in general, but I
>>>> found an interesting failure mode:
>>>>
>>>> NIR generates
>>>>
>>>> (+f0) if(8) JIP: 4 UIP: 5
>>>> else(8) JIP: 3
>>>> mov(8) g124<1>F -g124<8,8,1>F
>>>> endif(8) JIP: 6
>>>>
>>>> We should of course improve dead-control-flow-elimination to remove
>>>> the else and set the if's predicate_inverse, but non-NIR generates
>>>>
>>>> (+f0) sel(8) g124<1>F g25<8,8,1>F -g25<8,8,1>F
>>>>
>>>> which is probably better even than a predicated MOV since it's not a
>>>> partial write and subject to optimization restrictions. The assembly
>>>> matches the NIR exactly, so I'm assuming NIR is removing a
>>>> self-assignment in the if-then block, preventing it from turning it
>>>> into a conditional select.
>>>
>>> I think this is because the sel peephole (along with all the other
>>> optimizations) in NIR only works before we lower to source mods, where
>>> this code probably looked something like:
>>>
>>> ssa_1 = ...;
>>> if ... {
>>> } else {
>>> ssa_2 = fneg ssa_1;
>>> }
>>> ssa_3 = phi ssa_1, ssa_2;
>>
>> More specifically, we probably got
>>
>> b = foo ? a : -a
>>
>> from the GLSL source which NIR turns into
>>
>> if (foo) {
>> ssa_1 = a;
>> } else {
>> ssa_2 = -a;
>> }
>> ssa_3 = phi(ssa_1, ssa_2)
>>
>> which gets copy-propagated to what Connor mentioned above.
>>
>>> So we can probably fix it just by adding fneg/ineg to
>>> block_check_for_allowed_instrs().
>>
>> That was my first thought but we're starting to get into the world of
>> backend-specific knowledge there. Maybe this is where we just need to
>> pass in Eric's backend info struct and predicate doing so on whether
>> or not neg is free? But I guess the only compiler we care about at
>> the moment where that isn't true is vc4 which can't do control-flow
>> right now anyway. Thoughts?
>
> I like the plan of allowing fneg/ineg. It's trivial to recognize the
> quoted SEL and turn it into something different if your hardware
> doesn't like negate.
Ok, in that case, let's add fneg/ineg to the list of allowed instructions.
>> We may also want to simply make the backend's peephole smarter.
>
> This is harder than it sounds.
I know. Why do you think I haven't done it. :-)
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
