[Mesa-dev] [PATCH 9/9] i965: Use NIR by default for Fragment shaders on SNB+

Jason Ekstrand jason at jlekstrand.net
Wed Mar 18 10:39:54 PDT 2015


On Wed, Mar 18, 2015 at 9:52 AM, Matt Turner <mattst88 at gmail.com> wrote:
> On Wed, Mar 18, 2015 at 8:27 AM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Wed, Mar 18, 2015 at 7:22 AM, Connor Abbott <cwabbott0 at gmail.com> wrote:
>>> On Wed, Mar 18, 2015 at 1:56 AM, Matt Turner <mattst88 at 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. :-)


More information about the mesa-dev mailing list