[Mesa-dev] [PATCH 3/4] nir: support to flatten_all in peephole-select

[Rob Clark]

On Thu, Apr 2, 2015 at 3:19 PM, Connor Abbott <cwabbott0 at gmail.com> wrote:
> On Thu, Apr 2, 2015 at 1:32 PM, Rob Clark <robdclark at gmail.com> wrote:
>> On Thu, Apr 2, 2015 at 1:24 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>>> On Thu, Apr 2, 2015 at 1:20 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
>>>> On Wednesday, April 01, 2015 07:22:40 PM Connor Abbott wrote:
>>>>> I think it might be better here if we had a callback that backends can
>>>>> fill in that tells you when an instruction can be pulled out by the
>>>>> sel peephole. As Jason noted, you won't be able to do this for
>>>>> everything (notably, output writes and variable writes) and we'll
>>>>> probably need special handling for predicating discards if we want to
>>>>> be able to flatten everything. There are also a few cases where on
>>>>> i965 we aren't activating it when we could. Even then, I didn't think
>>>>> we'd need something this general, but with different backends with
>>>>> such varying needs I guess it makes more sense to go with the more
>>>>> general solution.
>>>>
>>>> NIR does have nir_intrinsic_discard_if, which performs a conditional
>>>> discard.  Assignments can be handled by conditional selects.  In the
>>>> absence of other memory writes (atomics, images, etc), that's probably
>>>> sufficient, no?
>>>
>>> Yes, that should be sufficient assuming array accesses can be
>>> flattened etc.  However, the NIR pass shouldn't flatten things that
>>> can't be flattened so we don't generate wrong NIR.  Instead, in those
>>> cases, we should just let the backend die on control-flow.
>>
>> right..  I'll have a go at hoisting things out that can be moved (like
>> kill -> kill_if and change writes to output to writes to var and then
>> select before write to output).. for things that can't be moved, let
>> the backend fail.. doubtful a backend that doesn't support flow
>> control will support atomics and other 'fancy stuff' anyways ;-)
>
> Well, the one other thing is relative addressing... how does that work
> with your HW? I know you support it, but if I do something like:

well, currently I doubt it works correctly currently.  We don't have
writemask or predicated instructions or anything like that, so it
would end up getting flattened and executed unconditionally.  Quite
possibly none of the variable-addressing piglit tests test this.

Maybe for now I'll just pull a hacked up copy of peephole-select into
my driver.  I can tolerate a bit more some failing edge cases, since
it wouldn't actually be a regression.  The whole point was to be able
to switch over to NIR first, before implementing proper flow control,
so a bit of chicken-and-egg problem..

BR,
-R

> if(...) {
>     foo[i] = bar;
> }
>
> ? Is it just a conditional move of all the elements of foo? In any
> case, we don't lower indirect addressing to SSA in NIR, so there's no
> way we could flatten it there.
>
>>
>> (which.. does makes me wonder how I'll eventually deal w/ atomics vs
>> instruction scheduling..  maybe I can split them out into a separate
>> unconditional successor basic block since at that point I'd be doing
>> instruction scheduling at the basic block level.  I think.)
>>
>> BR,
>> -R
>>
>>
>>> --Jason
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev