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

[Rob Clark]

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 ;-)

BR,
-R

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