On 20/11/17 19:00, Eduardo Lima Mitev wrote:
On 11/16/2017 12:23 AM, Timothy Arceri wrote:
On 16/11/17 00:22, Eduardo Lima Mitev wrote:
This will be used by the linker code to dfferentiate between
programs made out of SPIR-V or GLSL shaders.
So far everywhere this is used it seems you could just do something like:
if (shProg->_LinkedShaders[stage]->spirv_data)
... spriv stuff ...
else
... glsl stuff ...
This flag is a per-program variable (as oppose to a per-shader one).
While it would be possible to know the type of program (spirv vs. glsl)
indirectly by looking at its linked shaders, I think it is clearer and
more readable (and more formal) to have a flag in the program data for
this.
The flag is per-program but as I said above everywhere you use it you
have access to the shader so it's totally unneeded. Adding bools
everywhere just ends up with two ways to get to the same solution making
things less clear and less readable because the code always ends up
using a bool here and a NULL check there. It's hardly a stretch to see
that if you have spirv_data than its a spriv shader.
As the person that spent a bunch of time cleaning up all these structs
at then end of last year (I believe I landed around 100 patches to clean
up the mess) I'm very much against adding unnecessary fields back in here.
This also allows for releasing the memory of the spirv_data structure
when we don't need it, and still being able to know what kind of program
we have.
Why do you need this though? The backend shouldn't care where it came
from once you have compiled it right? You also can't free it until
gl_shader is freed in which case the API can't query it anymore either
so as far as I can see it's not needed.
I would personally keep this flag.
Are there more opinions about this?
Thanks for reviewing, Timothy.
Edu
---
src/mesa/main/mtypes.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index d624f2cbd19..db9c2e1deaa 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2902,6 +2902,12 @@ struct gl_shader_program_data
/* Mask of stages this program was linked against */
unsigned linked_stages;
+
+ /* Whether the shaders of this program are loaded from SPIR-V
binaries
+ * (all have the SPIR_V_BINARY_ARB state). This was introduced by
the
+ * ARB_gl_spirv extension.
+ */
+ bool spirv;
};
/**
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
