<div dir="ltr"> <div class="gmail_extra">Thank you for reviewing Nicolai!</div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Apr 18, 2017 at 4:17 AM, Nicolai Hähnle <<a href="mailto:nhaehnle@gmail.com" target="_blank">nhaehnle@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On 18.04.2017 03:25, Plamena Manolova wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 <<a href="mailto:plamena.manolova@intel.com" target="_blank">plamena.manolova@intel.com</a>> --- 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(-) </blockquote></div></div> [snip]<div><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> @@ -651,6 +655,86 @@ ast_type_qualifier::merge_into_in_qualifier(YYLTYPE *loc, r = false; } + if (state->in_qualifier->flags.q.pixel_interlock_ordered) { + if (state->in_qualifier->flags.q.pixel_interlock_unordered || + state->in_qualifier->flags.q.sample_interlock_ordered || + state->in_qualifier->flags.q.sample_interlock_unordered) { + _mesa_glsl_error(loc, state, "only one interlock mode can be used at " + "any time."); + r = false; + } else { + if (!state->ctx->Multisample.Enabled) { + state->fs_pixel_interlock_ordered = true; + state->in_qualifier->flags.q.pixel_interlock_ordered = false; + } else { + _mesa_glsl_error(loc, state, + "pixel_interlock_ordered can only be used when " + "multisampling is disabled."); + r = false; + } + } + } + + if (state->in_qualifier->flags.q.pixel_interlock_unordered) { + if (state->in_qualifier->flags.q.pixel_interlock_ordered || + state->in_qualifier->flags.q.sample_interlock_ordered || + state->in_qualifier->flags.q.sample_interlock_unordered) { + _mesa_glsl_error(loc, state, "only one interlock mode can be used at " + "any time."); + r = false; + } else { + if (!state->ctx->Multisample.Enabled) { + state->fs_pixel_interlock_unordered = true; + state->in_qualifier->flags.q.pixel_interlock_unordered = false; + } else { + _mesa_glsl_error(loc, state, + "pixel_interlock_unordered can only be used when " + "multisampling is disabled."); + r = false; + } + } + } + + if (state->in_qualifier->flags.q.sample_interlock_ordered) { + if (state->in_qualifier->flags.q.pixel_interlock_ordered || + state->in_qualifier->flags.q.pixel_interlock_unordered || + state->in_qualifier->flags.q.sample_interlock_unordered) { + _mesa_glsl_error(loc, state, "only one interlock mode can be used at " + "any time."); + r = false; + } else { + if (state->ctx->Multisample.Enabled) { + state->fs_sample_interlock_ordered = true; + state->in_qualifier->flags.q.sample_interlock_ordered = false; + } else { + _mesa_glsl_error(loc, state, + "sample_interlock_ordered can only be used when " + "multisampling is enabled."); + r = false; + } + } + } + + if (state->in_qualifier->flags.q.sample_interlock_unordered) { + if (state->in_qualifier->flags.q.pixel_interlock_ordered || + state->in_qualifier->flags.q.pixel_interlock_unordered || + state->in_qualifier->flags.q.sample_interlock_ordered) { + _mesa_glsl_error(loc, state, "only one interlock mode can be used at " + "any time."); + r = false; + } else { + if (state->ctx->Multisample.Enabled) { + state->fs_sample_interlock_unordered = true; + state->in_qualifier->flags.q.sample_interlock_unordered = false; + } else { + _mesa_glsl_error(loc, state, + "sample_interlock_unordered can only be used when " + "multisampling is enabled.\n"); + r = false; + } + } + } </blockquote> </div></div> There is a lot of code (almost-)duplication here. At the very least, the "only one interlock mode" check should be simplified by just checking if (state->in_qualifier->flags.q.sample_interlock_ordered + state->in_qualifier->flags.q.sample_interlock_unordered + state->in_qualifier->flags.q.pixel_interlock_ordered + state->in_qualifier->flags.q.pixel_interlock_unordered > 1) ... The remaining duplication could be reduced by temporarily defining a preprocessor macro. Personally, that's what I would do, but I'm okay with other people have different stylistic preferences. </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> [snip]</blockquote><div> </div><div>Those if-statements could definitely be simplified, I'll go with your approach.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy index e703073..1e23fd2 100644 --- a/src/compiler/glsl/glsl_parser.yy +++ b/src/compiler/glsl/glsl_parser.yy @@ -1456,6 +1456,115 @@ layout_qualifier_id: } } + $$.flags.q.pixel_interlock_ordered = match_layout_qualifier($1, + "pixel_interlock_ordered", state) == 0 ? 1 : 0; + $$.flags.q.pixel_interlock_unordered = match_layout_qualifier($1, + "pixel_interlock_unordered", state) == 0 ? 1 : 0; + $$.flags.q.sample_interlock_ordered = match_layout_qualifier($1, + "sample_interlock_ordered", state) == 0 ? 1 : 0; + $$.flags.q.sample_interlock_unordered = match_layout_qualifier($1, + "sample_interlock_unordered", state) == 0 ? 1 : 0; + + if ($$.flags.q.pixel_interlock_ordered == 1) { + if (state->stage != MESA_SHADER_FRAGMENT) { + _mesa_glsl_error(& @1, state, + "pixel_interlock_ordered layout qualifier only " + "valid in fragment shaders."); + $$.flags.q.pixel_interlock_ordered = 0; + } + + if ($$.flags.q.pixel_interlock_unordered == 1 || + $$.flags.q.sample_interlock_ordered == 1 || + $$.flags.q.sample_interlock_unordered == 1) { + _mesa_glsl_error(& @1, state, + "only one interlock mode can be used at any " + "time."); + $$.flags.q.pixel_interlock_ordered = 0; + } + + if (state->ctx->Multisample.Enabled) { + _mesa_glsl_error(& @1, state, + "pixel_interlock_ordered can only be used when " + "multisampling is disabled."); + $$.flags.q.pixel_interlock_ordered = 0; + } + } </blockquote> </div></div> Are these checks redundant with the checks above? Cheers, Nicolai <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> + + if ($$.flags.q.pixel_interlock_unordered == 1) { + if (state->stage != MESA_SHADER_FRAGMENT) { + _mesa_glsl_error(& @1, state, + "pixel_interlock_unordered layout qualifier only " + "valid in fragment shaders."); + $$.flags.q.pixel_interlock_unordered = 0; + } + + if ($$.flags.q.pixel_interlock_ordered == 1 || + $$.flags.q.sample_interlock_ordered == 1 || + $$.flags.q.sample_interlock_unordered == 1) { + _mesa_glsl_error(& @1, state, + "only one interlock mode can be used at any " + "time."); + $$.flags.q.pixel_interlock_unordered = 0; + } + + if (state->ctx->Multisample.Enabled) { + _mesa_glsl_error(& @1, state, + "pixel_interlock_unordered can only be used when " + "multisampling is disabled."); + $$.flags.q.pixel_interlock_unordered = 0; + } + } + + if ($$.flags.q.sample_interlock_ordered == 1) { + if (state->stage != MESA_SHADER_FRAGMENT) { + _mesa_glsl_error(& @1, state, + "sample_interlock_ordered layout qualifier only " + "valid in fragment shaders."); + $$.flags.q.sample_interlock_ordered = 0; + } + + if ($$.flags.q.pixel_interlock_ordered == 1 || + $$.flags.q.pixel_interlock_unordered == 1 || + $$.flags.q.sample_interlock_unordered == 1) { + _mesa_glsl_error(& @1, state, + "only one interlock mode can be used at any " + "time."); + $$.flags.q.sample_interlock_ordered = 0; + } + + if (!state->ctx->Multisample.Enabled) { + _mesa_glsl_error(& @1, state, + "sample_interlock_ordered can only be used when " + "multisampling is enabled."); + $$.flags.q.sample_interlock_ordered = 0; + } + } + + if ($$.flags.q.sample_interlock_unordered == 1) { + if (state->stage != MESA_SHADER_FRAGMENT) { + _mesa_glsl_error(& @1, state, + "sample_interlock_unordered layout qualifier only " + "valid in fragment shaders."); + $$.flags.q.sample_interlock_unordered = 0; + } + + if ($$.flags.q.pixel_interlock_ordered == 1 || + $$.flags.q.pixel_interlock_unordered == 1 || + $$.flags.q.sample_interlock_ordered == 1) { + _mesa_glsl_error(& @1, state, + "only one interlock mode can be used at any " + "time."); + $$.flags.q.sample_interlock_unordered = 0; + } + + if (!state->ctx->Multisample.Enabled) { + _mesa_glsl_error(& @1, state, + "sample_interlock_unordered can only be used when " + "multisampling is enabled."); + $$.flags.q.sample_interlock_unordered = 0; + } + } + /* Layout qualifiers for tessellation evaluation shaders. */ if (!$$.flags.i) { static const struct { diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp index eb12eff..68ebd5c 100644 --- a/src/compiler/glsl/glsl_parser_extras.cpp +++ b/src/compiler/glsl/glsl_parser_extras.cpp @@ -300,6 +300,10 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx, this->fs_early_fragment_tests = false; this->fs_inner_coverage = false; this->fs_post_depth_coverage = false; + this->fs_pixel_interlock_ordered = false; + this->fs_pixel_interlock_unordered = false; + this->fs_sample_interlock_ordered = false; + this->fs_sample_interlock_unordered = false; this->fs_blend_support = 0; memset(this->atomic_counter_offsets, 0, sizeof(this->atomic_counter_offsets)); @@ -619,6 +623,7 @@ static const _mesa_glsl_extension _mesa_glsl_supported_extensions[] = { EXT(ARB_explicit_uniform_location), EXT(ARB_fragment_coord_conventions), EXT(ARB_fragment_layer_viewport), + EXT(ARB_fragment_shader_interlock), EXT(ARB_gpu_shader5), EXT(ARB_gpu_shader_fp64), EXT(ARB_gpu_shader_int64), @@ -1720,6 +1725,10 @@ set_shader_inout_layout(struct gl_shader *shader, assert(!state->fs_early_fragment_tests); assert(!state->fs_inner_coverage); assert(!state->fs_post_depth_coverage); + assert(!state->fs_pixel_interlock_ordered); + assert(!state->fs_pixel_interlock_unordered); + assert(!state->fs_sample_interlock_ordered); + assert(!state->fs_sample_interlock_unordered); } for (unsigned i = 0; i < MAX_FEEDBACK_BUFFERS; i++) { @@ -1841,6 +1850,10 @@ set_shader_inout_layout(struct gl_shader *shader, shader->EarlyFragmentTests = state->fs_early_fragment_tests; shader->InnerCoverage = state->fs_inner_coverage; shader->PostDepthCoverage = state->fs_post_depth_coverage; + shader->PixelInterlockOrdered = state->fs_pixel_interlock_ordered; + shader->PixelInterlockUnordered = state->fs_pixel_interlock_unordered; + shader->SampleInterlockOrdered = state->fs_sample_interlock_ordered; + shader->SampleInterlockUnordered = state->fs_sample_interlock_unordered; shader->BlendSupport = state->fs_blend_support; break; diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h index 6c3bc8a..56218cc 100644 --- a/src/compiler/glsl/glsl_parser_extras.h +++ b/src/compiler/glsl/glsl_parser_extras.h @@ -616,6 +616,8 @@ struct _mesa_glsl_parse_state { bool ARB_fragment_coord_conventions_warn; bool ARB_fragment_layer_viewport_enable; bool ARB_fragment_layer_viewport_warn; + bool ARB_fragment_shader_interlock_enable; + bool ARB_fragment_shader_interlock_warn; bool ARB_gpu_shader5_enable; bool ARB_gpu_shader5_warn; bool ARB_gpu_shader_fp64_enable; @@ -810,6 +812,11 @@ struct _mesa_glsl_parse_state { bool fs_post_depth_coverage; + bool fs_pixel_interlock_ordered; + bool fs_pixel_interlock_unordered; + bool fs_sample_interlock_ordered; + bool fs_sample_interlock_unordered; + unsigned fs_blend_support; /** diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index 870d457..82b872c 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -734,6 +734,12 @@ nir_visitor::visit(ir_call *ir) case ir_intrinsic_shader_clock: op = nir_intrinsic_shader_clock; break; + case ir_intrinsic_begin_invocation_interlock_ARB: + op = nir_intrinsic_begin_invocation_interlock_ARB; + break; + case ir_intrinsic_end_invocation_interlock_ARB: + op = nir_intrinsic_end_invocation_interlock_ARB; + break; case ir_intrinsic_group_memory_barrier: op = nir_intrinsic_group_memory_barrier; break; @@ -934,6 +940,12 @@ nir_visitor::visit(ir_call *ir) instr->num_components = 2; nir_builder_instr_insert(&b, &instr->instr); break; + case nir_intrinsic_begin_invocation_interlock_ARB: + nir_builder_instr_insert(&b, &instr->instr); + break; + case nir_intrinsic_end_invocation_interlock_ARB: + nir_builder_instr_insert(&b, &instr->instr); + break; case nir_intrinsic_store_ssbo: { exec_node *param = ir->actual_parameters.get_head(); ir_rvalue *block = ((ir_instruction *)param)->as_rvalue(); diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h index d7a81c5..a404d00 100644 --- a/src/compiler/glsl/ir.h +++ b/src/compiler/glsl/ir.h @@ -1097,6 +1097,8 @@ enum ir_intrinsic_id { ir_intrinsic_memory_barrier_buffer, ir_intrinsic_memory_barrier_image, ir_intrinsic_memory_barrier_shared, + ir_intrinsic_begin_invocation_interlock_ARB, + ir_intrinsic_end_invocation_interlock_ARB, ir_intrinsic_shared_load, ir_intrinsic_shared_store = MAKE_INTRINSIC_FOR_TYPE(store, shared), diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp index 7ace01d..f06eaf7 100644 --- a/src/compiler/glsl/linker.cpp +++ b/src/compiler/glsl/linker.cpp @@ -1895,6 +1895,14 @@ link_fs_inout_layout_qualifiers(struct gl_shader_program *prog, linked_shader->Program->info.fs.inner_coverage |= shader->InnerCoverage; linked_shader->Program->info.fs.post_depth_coverage |= shader->PostDepthCoverage; + linked_shader->Program->info.fs.pixel_interlock_ordered |= + shader->PixelInterlockOrdered; + linked_shader->Program->info.fs.pixel_interlock_unordered |= + shader->PixelInterlockUnordered; + linked_shader->Program->info.fs.sample_interlock_ordered |= + shader->SampleInterlockOrdered; + linked_shader->Program->info.fs.sample_interlock_unordered |= + shader->SampleInterlockUnordered; linked_shader->Program->sh.fs.BlendSupport |= shader->BlendSupport; } diff --git a/src/compiler/nir/nir_intrinsics.h b/src/compiler/nir/nir_intrinsics.h index 3a519a7..82dac3d 100644 --- a/src/compiler/nir/nir_intrinsics.h +++ b/src/compiler/nir/nir_intrinsics.h @@ -104,6 +104,8 @@ BARRIER(memory_barrier_buffer) BARRIER(memory_barrier_image) BARRIER(memory_barrier_shared) +BARRIER(begin_invocation_interlock_ARB) +BARRIER(end_invocation_interlock_ARB) /** A conditional discard, with a single boolean source. */ INTRINSIC(discard_if, 1, ARR(1), false, 0, 0, 0, xx, xx, xx, 0) diff --git a/src/compiler/shader_info.h b/src/compiler/shader_info.h index a670841..3d7282f 100644 --- a/src/compiler/shader_info.h +++ b/src/compiler/shader_info.h @@ -127,6 +127,11 @@ typedef struct shader_info { bool post_depth_coverage; + bool pixel_interlock_ordered; + bool pixel_interlock_unordered; + bool sample_interlock_ordered; + bool sample_interlock_unordered; + /** gl_FragDepth layout for ARB_conservative_depth. */ enum gl_frag_depth_layout depth_layout; } fs; diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h index d11cb0f..e64a88a 100644 --- a/src/mesa/main/extensions_table.h +++ b/src/mesa/main/extensions_table.h @@ -67,6 +67,7 @@ EXT(ARB_fragment_layer_viewport , ARB_fragment_layer_viewport EXT(ARB_fragment_program , ARB_fragment_program , GLL, x , x , x , 2002) EXT(ARB_fragment_program_shadow , ARB_fragment_program_shadow , GLL, x , x , x , 2003) EXT(ARB_fragment_shader , ARB_fragment_shader , GLL, GLC, x , x , 2002) +EXT(ARB_fragment_shader_interlock , ARB_fragment_shader_interlock , GLL, GLC, x , x , 2015) EXT(ARB_framebuffer_no_attachments , ARB_framebuffer_no_attachments , GLL, GLC, x , x , 2012) EXT(ARB_framebuffer_object , ARB_framebuffer_object , GLL, GLC, x , x , 2005) EXT(ARB_framebuffer_sRGB , EXT_framebuffer_sRGB , GLL, GLC, x , x , 1998) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index c4fab9d..608345d 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -2558,6 +2558,10 @@ struct gl_shader bool uses_gl_fragcoord; bool PostDepthCoverage; + bool PixelInterlockOrdered; + bool PixelInterlockUnordered; + bool SampleInterlockOrdered; + bool SampleInterlockUnordered; bool InnerCoverage; /** @@ -3967,6 +3971,7 @@ struct gl_extensions GLboolean ARB_fragment_shader; GLboolean ARB_framebuffer_no_attachments; GLboolean ARB_framebuffer_object; + GLboolean ARB_fragment_shader_interlock; GLboolean ARB_enhanced_layouts; GLboolean ARB_explicit_attrib_location; GLboolean ARB_explicit_uniform_location; -- 2.9.3 </div></div> _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote> -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. </blockquote></div> </div></div>