On 19/04/17 11:12, Manolova, Plamena wrote:
Thank you for reviewing Timothy!
On Mon, Apr 17, 2017 at 7:10 PM, Timothy Arceri <tarc...@itsqueeze.com
<mailto:tarc...@itsqueeze.com>> wrote:
On 18/04/17 11:25, Plamena Manolova wrote:
This extension provides new GLSL built-in functions
beginInvocationInterlockARB() and endInvocationInterlockARB()
that delimit a critical section of fragment shader code. For
pairs of shader invocations with "overlapping" coverage in a
given pixel, the OpenGL implementation will guarantee that the
critical section of the fragment shader will be executed for
only one fragment at a time.
Signed-off-by: Plamena Manolova <plamena.manol...@intel.com
<mailto:plamena.manol...@intel.com>>
---
src/compiler/glsl/ast.h | 10 +++
src/compiler/glsl/ast_to_hir.cpp | 21 ++++++
src/compiler/glsl/ast_type.cpp | 90
++++++++++++++++++++++++-
src/compiler/glsl/builtin_functions.cpp | 79
++++++++++++++++++++++
src/compiler/glsl/glsl_parser.yy | 109
+++++++++++++++++++++++++++++++
src/compiler/glsl/glsl_parser_extras.cpp | 13 ++++
src/compiler/glsl/glsl_parser_extras.h | 7 ++
src/compiler/glsl/glsl_to_nir.cpp | 12 ++++
src/compiler/glsl/ir.h | 2 +
src/compiler/glsl/linker.cpp | 8 +++
src/compiler/nir/nir_intrinsics.h | 2 +
src/compiler/shader_info.h | 5 ++
src/mesa/main/extensions_table.h | 1 +
src/mesa/main/mtypes.h | 5 ++
14 files changed, 363 insertions(+), 1 deletion(-)
diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
index 455cb81..f2f7e9e 100644
--- a/src/compiler/glsl/ast.h
+++ b/src/compiler/glsl/ast.h
@@ -617,6 +617,16 @@ struct ast_type_qualifier {
* Flag set if GL_ARB_post_depth_coverage layout
qualifier is used.
*/
unsigned post_depth_coverage:1;
+
+ /**
+ * Flags for the layout qualifers added by
ARB_fragment_shader_interlock
+ */
+
+ unsigned pixel_interlock_ordered:1;
+ unsigned pixel_interlock_unordered:1;
+ unsigned sample_interlock_ordered:1;
+ unsigned sample_interlock_unordered:1;
Samuel spent a bunch of time freeing up 4 bits of this flag to be
able to implement ARB_bindless_texture. This change will use up all
those free bits.
These only apply to the default in right? I wonder if it's possible
to split those qualifiers off from the layout qualifiers applied to
varyings?
I think it should be possible to split them out, I can have a go at that.
I expect it will be tricky, but worth it if we can. Good luck. :)
+
/**
* Flag set if GL_INTEL_conservartive_rasterization
layout qualifier
* is used.
diff --git a/src/compiler/glsl/ast_to_hir.cpp
b/src/compiler/glsl/ast_to_hir.cpp
index 9ea37f4..71c52ad 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3717,6 +3717,27 @@ apply_layout_qualifier_to_variable(const
struct ast_type_qualifier *qual,
_mesa_glsl_error(loc, state, "post_depth_coverage layout
qualifier only "
"valid in fragment shader input layout
declaration.");
}
+
+ if (qual->flags.q.pixel_interlock_ordered) {
+ _mesa_glsl_error(loc, state, "pixel_interlock_ordered
layout qualifier only "
+ "valid in fragment shader input layout
declaration.");
+ }
+
+ if (qual->flags.q.pixel_interlock_unordered) {
+ _mesa_glsl_error(loc, state, "pixel_interlock_unordered
layout qualifier only "
+ "valid in fragment shader input layout
declaration.");
+ }
+
+
+ if (qual->flags.q.pixel_interlock_ordered) {
+ _mesa_glsl_error(loc, state, "sample_interlock_ordered
layout qualifier only "
+ "valid in fragment shader input layout
declaration.");
+ }
+
+ if (qual->flags.q.pixel_interlock_unordered) {
+ _mesa_glsl_error(loc, state, "sample_interlock_unordered
layout qualifier only "
+ "valid in fragment shader input layout
declaration.");
+ }
Here and below we duplicated the validation done in glsl_parser.yy.
Does the parser validation not catch everything?
Also there seems to be no link time validation to check that "only
one interlock mode can be used at any time" when we are compiling a
program that contains multiple fragment shaders.
Do you have any piglit tests to go with this series?
Yes there's a test here: https://patchwork.freedesktop.org/patch/151133
I'll look into whether we actually need validation in both
glsl_parser.yy and ast_type.cpp
That is just an execution test. Please also provide compile/link tests
the exercise both valid and invalid shaders. With those you will easily
be able to tell if the additional validation is required or not.
Thanks,
Tim
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
