On 30/04/18 20:03, Alejandro Piñeiro wrote:
On 30/04/18 11:53, Timothy Arceri wrote:
<snip>
I'll reply to the above snipped section in a separate email.
Ignoring the above issue I'm fairly certain adding a dependency on
core mesa GL to NIR is a no no so either way these files don't
belong in src/compiler/nir
But we already have that dependency, right? I mean gl_program have a
reference to nir_shader for some time now.
This is largely historical when NIR was created we had no Vulkan
drivers and it originally lived in src/glsl/nir I have a feeling we
can get rid of most of these old dependencies by moving some GL only
helpers out of the src/compiler/nir dir, I might give this a go.
Ok, thanks.
FYI
https://patchwork.freedesktop.org/patch/219624/
And in any case, the implementation of ARB_gl_spirv is based on NIR,
as right now we don't have an alternative to process SPIR-V. So for
example, the following patch (already merged on master) [3]:
mesa/glspirv: Add a _mesa_spirv_to_nir() function
This is basically a wrapper around spirv_to_nir() that includes
arguments setup and post-conversion validation.
Is at main/glspirv.c, and calls directly spirv_to_nir.
Although it is true that code would not be called unless the
extension is enabled, it depends on NIR and it is included on core
mesa (main/gl_spirv.c).
Well, unless I'm misunderstanding what do you mean for core mesa. I'm
including mesa/main as core mesa too.
A dependency on NIR from Mesa is fine, I'm saying NIR (the core
library) shouldn't have dependencies on core Mesa.
Ok, thanks for the explanation.
In any case, I'm not against moving those files to a different
directory if needed, just trying to understand why doesn't belong there.
In patch 12 & 16 you are creating a "util" so you can call things from
the helpers you are placing in the nir dir but these are GL only
concepts. The util is not a real util that is shared by the vulkan and
gl compiler but rather you are working around a dependency problem you
have created by locating the uniform linker and other helpers inside
the nir directory. You are simply bundling code into builds of Vulkan
drivers that will never be used.
Good point, with this directory structure, Vulkan drivers are getting
code that will not use at all.
If you simply put these files inside src/compiler/glsl/ then you don't
need this util at all.
True, but (and sorry for being nitpicky), it sounds strange to me to put
code on a glsl directory that are not really related to glsl.
It would probably make more sense now that this extension has come
along if the dir was actually src/compiler/gl/ but its not worth the
hassle of changing that.
Ok, I see your point. Yes, changing the name would be a hassle, and
adding a new directory (so having both src/compiler/gl and
src/compiler/glsl) confusing. So unless someone complains or have a
better idea, we would go for your suggestion. But if you don't mind I
would wait to do the change until we get review on the code itself, and
not only on the placement.
Not a problem.
<snip>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
