[Mesa-dev] [PATCH 3/4] anv: Use central api generation scripts.

[Dylan Baker]

Quoting Bas Nieuwenhuizen (2018-08-13 09:25:01)
> On Mon, Aug 13, 2018 at 5:54 PM, Dylan Baker <dylan at pnwbakers.com> wrote:
> > Quoting Emil Velikov (2018-08-13 03:38:13)
> >> On 10 August 2018 at 00:57, Dylan Baker <dylan at pnwbakers.com> wrote:
> >> > Quoting Chad Versace (2018-08-09 10:37:33)
> >> >> On Tue 07 Aug 2018, Dylan Baker wrote:
> >> >> > Quoting Bas Nieuwenhuizen (2018-08-07 16:14:33)
> >> >> > >
> >> >> > >  anv_extensions_c = custom_target(
> >> >> > > @@ -36,10 +37,11 @@ anv_extensions_c = custom_target(
> >> >> > >    input : ['anv_extensions_gen.py', vk_api_xml],
> >> >> > >    output : 'anv_extensions.c',
> >> >> > >    command : [
> >> >> > > +   'env', 'PYTHONPATH=@0@'.format(join_paths(meson.source_root(), 'src/vulkan/util/')),
> >> >> >
> >> >> > This is really gross, you're adding a dependency on a unix console command. I
> >> >> > know that anv is only built on Unix-like oses, but this will eventually end up
> >> >> > being used in some code that needs to run on Windows (or mac, does mac have
> >> >> > env?).
> >> >> >
> >> >> > I know that some people will object, but IMHO a better solution than mucking
> >> >> > with the python path (either through sys.path or through PYTHONPATH, is to
> >> >> > put all of the generators in a src/generators directory and be done with it.
> >> >> > Sure the intel specific bits (for example) aren't in the src/intel folder,
> >> >> > that's a small price to avoid having to call env just to run a python script.
> >> >>
> >> >> Dylan, I think we should avoid introducing complexity in the build
> >> >> system for the benefit of operating systems not supported by the driver.
> >> >> That feels like a serious premature optimazation, to me.  Anvil's usage
> >> >> of ioctls is highly specific to Linux/Unix, will not work on MacOS, and
> >> >> definitely does not work on Windows.
> >> >
> >> > I agree completely. I think where we disagree is on whether mucking with
> >> > PYTHONPATH and using env is more complex or putting our generators in a single
> >> > directory is more complex. I think using env is extremely gross and complex, I
> >> > think mucking with PYTHONPATH is extremely gross and complex, and I think having
> >> > to reference a file from another directory is a *lot* less gross and *much* less
> >> > complex.
> >> >
> >> Can we reuse the NIR approach seen here [1] [2]?
> >> Moving files around just to appease python feels wrong on many levels :-(
> >>
> >> Thanks
> >> Emil
> >>
> >> [1] https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/intel/compiler/meson.build#L120
> >> [2] https://gitlab.freedesktop.org/mesa/mesa/blob/master/src/gallium/drivers/freedreno/meson.build#L21
> >
> > I would begrudgingly accept that.
> >
> > I really don't understand why this is such a big deal. Imagine for a moment that
> > a there was a driver for "Magical Unicorn" hardware existed. Imagine it had some
> > very odd instructions and the developers of the Magic Unicorn driver wanted some
> > special NIR passes for their hardware. Now lets imagine that they insisted that
> > those passes must live in src/unicorn, but should be linked into libnir. No one
> > would accept that, they would be told either to keep the passes in their backend
> > or to move them into src/compiler/nir. I believe this is an analogous situation.
> > We want to share code, so we're putting it in src/util/vulkan, but then instead
> > of calling a generator out of src/util/vulkan in src/intel and src/amd (we
> > already call python scripts from different directories all over the build
> > system), we do some elaborate environment variable manipulation which can have
> > unforseen side effects (like that google sets PYTHONPATH in their distro) and
> > have to wrap our actual callable with `env`, just so that we could put the
> > callable in src/intel and src/amd instead of having a callable in
> > src/util/vulkan. Does no one else think that this is a bit crazy?
> 
> For the main functions of the generators scripts I agree and can merge
> them a bit further. However the one where I think moving it is
> somewhat crazy is the {radv,anv}_extensions.py, in particular the list
> of supported extensions and the (driver-specific) conditions in which
> they should be enabled. That seems way driver-specific to put in a
> shared directory.

Okay, that's a fair point.

What about having a file that is a list of extensions that could live in
src/$vendor, which the script takes as an argument?

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180813/d9d34426/attachment.sig>