[Mesa-dev] [PATCH 00/11] nir: Add a pass management framework

[Connor Abbott]

On Sat, Oct 31, 2015 at 10:55 AM, Rob Clark <robdclark at gmail.com> wrote:
> On Fri, Oct 30, 2015 at 6:42 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>> On Fri, Oct 30, 2015 at 2:12 PM, Ian Romanick <idr at freedesktop.org> wrote:
>>> On 10/28/2015 10:01 PM, Jason Ekstrand wrote:
>>>>
>>>> On Oct 28, 2015 9:12 PM, "Kenneth Graunke" <kenneth at whitecape.org
>>>> <mailto:kenneth at whitecape.org>> wrote:
>>>>>
>>>>> On Wednesday, October 28, 2015 02:58:07 PM Kristian Høgsberg wrote:
>>>>> > On Wed, Oct 28, 2015 at 2:34 PM, Jason Ekstrand
>>>> <jason at jlekstrand.net <mailto:jason at jlekstrand.net>> wrote:
>>>>> > > On Wed, Oct 28, 2015 at 2:32 PM, Jason Ekstrand
>>>> <jason at jlekstrand.net <mailto:jason at jlekstrand.net>> wrote:
>>>>> > >> This series adds a nir_pass datastructure and some helpers for
>>>> managing
>>>>> > >> optimization and lowering passes.  I've been meaning to get
>>>> around to this
>>>>> > >> for some time.  There are a couple of primary benifits to this:
>>>>> > >>
>>>>> > >> First, this gives us a central place to put things such as
>>>> validating the
>>>>> > >> shader, printing it if it changes, etc.  Right now, the i965
>>>> backend calls
>>>>> > >> nir_validate_shader after each pass.  We would also like to add
>>>> something
>>>>> > >> like we have in the i965 backend where it can be set to dump the
>>>> IR to a
>>>>> > >> file after every pass that changess it.
>>>>> > >>
>>>>> > >> Mor importantly, though, it moves metadata out of the passes them
>>>> selves
>>>>> > >> and into the runner.  In the process of putting this series
>>>> together, I
>>>>> > >> found at least 3 or 4 optimization passes that don't properly
>>>> invalidate
>>>>> > >> metadata.  By putting a metadata_preserved field in nir_pass and
>>>> handling
>>>>> > >> metadata in the pass runner, we make it much less likely that a
>>>> pass will
>>>>> > >> get this wrong.  LLVM has a similar optimization pass
>>>> architecture for
>>>>> > >> precicely this reason.
>>>>> > >>
>>>>> > >> As a nice little side-benifit, we no longer have to iterate over
>>>> all of the
>>>>> > >> overloads with non-NULL impl pointers in each pass.
>>>>> > >
>>>>> > > Once again, git-send-email failed to send the last patch for whatever
>>>>> > > reason.  The entire series can be found here:
>>>>> > >
>>>>> > > http://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/nir-pass
>>>>> >
>>>>> > Nice. Series,
>>>>> >
>>>>> > Reviewed-by: Kristian Høgsberg <krh at bitplanet.net
>>>> <mailto:krh at bitplanet.net>>
>>>>>
>>>>> I plan to review this as well, so please hold off on pushing it for a
>>>>> little while.  Thanks!
>>>>
>>>> By all means, go ahead.  It'd also be nice, while you're at it to weigh
>>>> in on how to handle passing arguments to passes.  There are a number of
>>>> ideas thrown back-and-forth between Connor and myself on how to do it
>>>
>>> Is it even necessary to sort that out now?  Are there passes that
>>> haven't been ported to this infrastructure that need any of the extra
>>> features that you and Connor were discussing?  I will give keithp credit
>>> for teaching me not to engineer in features that you don't need yet.
>>> When you do need them, the thing you spent a bunch of time putting in
>>> probably won't be what you need.
>>
>> Strictly speaking, no.  There are passes that haven't been converted
>> that will need it.  The issue is how do you pass arguments into
>> passes.  Most optimization passes don't care but there are a few that
>> will have non-trivial arguments.
>
> random drive-by comment:  in addition to making it easier to have pass
> specific args (which *could* I suppose be moved into
> nir_shader_compiler_options at the expense of making that not a const
> thing anymore, since some of the options depend on shader variant
> key), having old-fashioned code to call opt passes (instead of a table
> of passes) makes it easier to insert nir_print_shader() calls in
> strategic spots while debugging ;-)

But with this, you can implement something like INTEL_DEBUG=opt where
you dump a file in between each pass and then you can diff them, which
is even better :).

>
> BR,
> -R
>
>> --Jason
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev