[Mesa-dev] [PATCH 1/2] mesa: Add GL/GLSL plumbing for ARB_fragment_shader_interlock.

Nicolai Hähnle nhaehnle at gmail.com
Tue Apr 18 11:17:45 UTC 2017


On 18.04.2017 03: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.manolova at 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(-)
>
[snip]
> @@ -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;
> +         }
> +      }
> +   }

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.


[snip]
> 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;
> +         }
> +      }

Are these checks redundant with the checks above?

Cheers,
Nicolai


> +
> +      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
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>


-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.


More information about the mesa-dev mailing list