[Mesa-dev] [RFC PATCH 07/26] glsl: process bindless/bound layout qualifiers

Timothy Arceri tarceri at itsqueeze.com
Tue Apr 11 23:54:43 UTC 2017



On 12/04/17 02:48, Samuel Pitoiset wrote:
> This adds bindless_sampler and bound_sampler (and respectively
> bindless_image and bound_image) to the parser.
>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/compiler/glsl/ast.h                  |  8 +++++
>  src/compiler/glsl/ast_to_hir.cpp         | 55 ++++++++++++++++++++++++++++++++
>  src/compiler/glsl/ast_type.cpp           | 55 ++++++++++++++++++++++++++++++++
>  src/compiler/glsl/glsl_parser.yy         | 21 ++++++++++++
>  src/compiler/glsl/glsl_parser_extras.cpp | 11 +++++++
>  src/compiler/glsl/glsl_parser_extras.h   | 10 ++++++
>  src/compiler/glsl/ir.cpp                 |  1 +
>  src/compiler/glsl/ir.h                   |  6 ++++
>  src/compiler/glsl/ir_print_visitor.cpp   |  5 +--
>  src/mesa/main/mtypes.h                   | 10 ++++++
>  10 files changed, 180 insertions(+), 2 deletions(-)
>
> diff --git a/src/compiler/glsl/ast.h b/src/compiler/glsl/ast.h
> index 455cb8113c..9327e03979 100644
> --- a/src/compiler/glsl/ast.h
> +++ b/src/compiler/glsl/ast.h
> @@ -622,6 +622,14 @@ struct ast_type_qualifier {
>            * is used.
>            */
>           unsigned inner_coverage:1;
> +
> +         /** \name Layout qualifiers for GL_ARB_bindless_texture */
> +         /** \{ */
> +         unsigned bindless_sampler:1;
> +         unsigned bindless_image:1;
> +         unsigned bound_sampler:1;
> +         unsigned bound_image:1;
> +         /** \} */
>        }
>        /** \brief Set of flags, accessed by name. */
>        q;
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
> index 38c0f5c8d4..5043d6a574 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -3415,6 +3415,55 @@ validate_array_dimensions(const glsl_type *t,
>  }
>
>  static void
> +apply_bindless_qualifier_to_variable(const struct ast_type_qualifier *qual,
> +                                     ir_variable *var,
> +                                     struct _mesa_glsl_parse_state *state,
> +                                     YYLTYPE *loc)
> +{
> +   /* The ARB_bindless_texture spec says:
> +    *
> +    *  "Modify Section 4.4.6 Opaque-Uniform Layout Qualifiers of the GLSL 4.30
> +    *   spec"
> +    *
> +    *  "If these layout qualifiers are applied to other types of default block
> +    *   uniforms, or variables with non-uniform storage, a compile-time error
> +    *   will be generated."
> +    */
> +   if (!qual->flags.q.uniform) {
> +      _mesa_glsl_error(loc, state, "ARB_bindless_texture layout qualifiers "
> +                       "can only be applied to default block uniforms or "
> +                       "variables with uniform storage");
> +      return;
> +   }
> +
> +   /* The ARB_bindless_texture spec doesn't state anything in this situation,
> +    * but it makes sense to only allow bindless_sampler/bound_sampler for
> +    * sampler types, and respectively bindless_image/bound_image for image
> +    * types.
> +    */
> +   if (qual->flags.q.bindless_sampler || qual->flags.q.bound_sampler) {
> +      if (!var->type->contains_bindless_sampler() &&
> +          !var->type->contains_sampler()) {
> +         _mesa_glsl_error(loc, state, "bindless_sampler or bound_sampler can "
> +                          "only be applied to sampler types");
> +         return;
> +      }
> +   }
> +
> +   if (qual->flags.q.bindless_image || qual->flags.q.bound_image) {
> +      if (!var->type->contains_bindless_image() &&
> +          !var->type->contains_image()) {
> +         _mesa_glsl_error(loc, state, "bindless_image or bound_image can "
> +                          "only be applied to image types");
> +         return;
> +      }
> +   }
> +
> +   var->data.bindless =
> +      qual->flags.q.bindless_sampler || qual->flags.q.bindless_image;
> +}
> +
> +static void
>  apply_layout_qualifier_to_variable(const struct ast_type_qualifier *qual,
>                                     ir_variable *var,
>                                     struct _mesa_glsl_parse_state *state,
> @@ -3711,6 +3760,12 @@ 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.bindless_sampler
> +       || qual->flags.q.bindless_image
> +       || qual->flags.q.bound_sampler
> +       || qual->flags.q.bound_image)
> +      apply_bindless_qualifier_to_variable(qual, var, state, loc);
>  }
>
>  static void
> diff --git a/src/compiler/glsl/ast_type.cpp b/src/compiler/glsl/ast_type.cpp
> index d302fc45fd..cf84528775 100644
> --- a/src/compiler/glsl/ast_type.cpp
> +++ b/src/compiler/glsl/ast_type.cpp
> @@ -69,6 +69,10 @@ ast_type_qualifier::has_layout() const
>            || this->flags.q.column_major
>            || this->flags.q.row_major
>            || this->flags.q.packed
> +          || this->flags.q.bindless_sampler
> +          || this->flags.q.bindless_image
> +          || this->flags.q.bound_sampler
> +          || this->flags.q.bound_image
>            || this->flags.q.explicit_align
>            || this->flags.q.explicit_component
>            || this->flags.q.explicit_location
> @@ -181,6 +185,35 @@ validate_point_mode(MAYBE_UNUSED const ast_type_qualifier &qualifier,
>     return true;
>  }
>
> +static bool
> +validate_bindless(YYLTYPE *loc,
> +                  _mesa_glsl_parse_state *state,
> +                  const ast_type_qualifier &qualifier,
> +                  const ast_type_qualifier &new_qualifier)
> +{
> +   if (state->default_uniform_qualifier->flags.q.bindless_sampler) {
> +      state->bindless_sampler_specified = true;
> +      state->default_uniform_qualifier->flags.q.bindless_sampler = false;
> +   }
> +
> +   if (state->default_uniform_qualifier->flags.q.bindless_image) {
> +      state->bindless_image_specified = true;
> +      state->default_uniform_qualifier->flags.q.bindless_image = false;
> +   }
> +
> +   if (state->default_uniform_qualifier->flags.q.bound_sampler) {
> +      state->bound_sampler_specified = true;
> +      state->default_uniform_qualifier->flags.q.bound_sampler = false;
> +   }
> +
> +   if (state->default_uniform_qualifier->flags.q.bound_image) {
> +      state->bound_image_specified = true;
> +      state->default_uniform_qualifier->flags.q.bound_image = false;
> +   }
> +
> +   return true;
> +}
> +
>  /**
>   * This function merges duplicate layout identifiers.
>   *
> @@ -393,6 +426,18 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
>     if (q.flags.q.local_size_variable)
>        this->flags.q.local_size_variable = true;
>
> +   if (q.flags.q.bindless_sampler)
> +      this->flags.q.bindless_sampler = true;
> +
> +   if (q.flags.q.bindless_image)
> +      this->flags.q.bindless_image = true;
> +
> +   if (q.flags.q.bound_sampler)
> +      this->flags.q.bound_sampler = true;
> +
> +   if (q.flags.q.bound_image)
> +      this->flags.q.bound_image = true;
> +
>     this->flags.i |= q.flags.i;
>
>     if (this->flags.q.in &&
> @@ -427,6 +472,12 @@ ast_type_qualifier::merge_qualifier(YYLTYPE *loc,
>        this->image_base_type = q.image_base_type;
>     }
>
> +   if (q.flags.q.bindless_sampler
> +       || q.flags.q.bindless_image
> +       || q.flags.q.bound_sampler
> +       || q.flags.q.bound_image)
> +      r &= validate_bindless(loc, state, *this, q);


validate_bindless() always returns true. Is is meant to do validation?

If so it looks like it needs to be fixed, otherwise it should be renamed 
and the return value dropped.

> +
>     return r;
>  }
>
> @@ -778,6 +829,10 @@ ast_type_qualifier::validate_flags(YYLTYPE *loc,
>                      bad.flags.q.subroutine ? " subroutine" : "",
>                      bad.flags.q.blend_support ? " blend_support" : "",
>                      bad.flags.q.inner_coverage ? " inner_coverage" : "",
> +                    bad.flags.q.bindless_sampler ? " bindless_sampler" : "",
> +                    bad.flags.q.bindless_image ? " bindless_image" : "",
> +                    bad.flags.q.bound_sampler ? " bound_sampler" : "",
> +                    bad.flags.q.bound_image ? " bound_image" : "",
>                      bad.flags.q.post_depth_coverage ? " post_depth_coverage" : "");
>     return false;
>  }
> diff --git a/src/compiler/glsl/glsl_parser.yy b/src/compiler/glsl/glsl_parser.yy
> index e703073c9c..9cdc37eb9a 100644
> --- a/src/compiler/glsl/glsl_parser.yy
> +++ b/src/compiler/glsl/glsl_parser.yy
> @@ -1589,6 +1589,27 @@ layout_qualifier_id:
>           }
>        }
>
> +      /* Layout qualifiers for ARB_bindless_texture. */
> +      if (!$$.flags.i) {
> +         if (match_layout_qualifier($1, "bindless_sampler", state) == 0)
> +            $$.flags.q.bindless_sampler = 1;
> +         if (match_layout_qualifier($1, "bound_sampler", state) == 0)
> +            $$.flags.q.bound_sampler = 1;
> +
> +         if (state->has_shader_image_load_store()) {
> +            if (match_layout_qualifier($1, "bindless_image", state) == 0)
> +               $$.flags.q.bindless_image = 1;
> +            if (match_layout_qualifier($1, "bound_image", state) == 0)
> +               $$.flags.q.bound_image = 1;
> +         }
> +
> +         if ($$.flags.i && !state->has_bindless()) {
> +            _mesa_glsl_error(& @1, state,
> +                             "qualifier `%s` requires "
> +                             "ARB_bindless_texture", $1);
> +         }
> +      }
> +
>        if (!$$.flags.i) {
>           _mesa_glsl_error(& @1, state, "unrecognized layout identifier "
>                            "`%s'", $1);
> diff --git a/src/compiler/glsl/glsl_parser_extras.cpp b/src/compiler/glsl/glsl_parser_extras.cpp
> index f6da4c5054..c5ebe594e3 100644
> --- a/src/compiler/glsl/glsl_parser_extras.cpp
> +++ b/src/compiler/glsl/glsl_parser_extras.cpp
> @@ -307,6 +307,12 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
>        ctx->Const.AllowGLSLExtensionDirectiveMidShader;
>
>     this->cs_input_local_size_variable_specified = false;
> +
> +   /* ARB_bindless_texture */
> +   this->bindless_sampler_specified = false;
> +   this->bindless_image_specified = false;
> +   this->bound_sampler_specified = false;
> +   this->bound_image_specified = false;
>  }
>
>  /**
> @@ -1849,6 +1855,11 @@ set_shader_inout_layout(struct gl_shader *shader,
>        /* Nothing to do. */
>        break;
>     }
> +
> +   shader->bindless_sampler = state->bindless_sampler_specified;
> +   shader->bindless_image = state->bindless_image_specified;
> +   shader->bound_sampler = state->bound_sampler_specified;
> +   shader->bound_image = state->bound_image_specified;
>  }
>
>  extern "C" {
> diff --git a/src/compiler/glsl/glsl_parser_extras.h b/src/compiler/glsl/glsl_parser_extras.h
> index 6045a6128e..3a84d655b1 100644
> --- a/src/compiler/glsl/glsl_parser_extras.h
> +++ b/src/compiler/glsl/glsl_parser_extras.h
> @@ -428,6 +428,16 @@ struct _mesa_glsl_parse_state {
>     bool cs_input_local_size_variable_specified;
>
>     /**
> +    * True if a shader declare bindless_sampler/bindless_image, and
> +    * respectively bound_sampler/bound_image at global scope as specified by
> +    * ARB_bindless_texture.
> +    */
> +   bool bindless_sampler_specified;
> +   bool bindless_image_specified;
> +   bool bound_sampler_specified;
> +   bool bound_image_specified;
> +
> +   /**
>      * Output layout qualifiers from GLSL 1.50 (geometry shader controls),
>      * and GLSL 4.00 (tessellation control shader).
>      */
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index 3fbf0f68d3..0beae62e99 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -1748,6 +1748,7 @@ ir_variable::ir_variable(const struct glsl_type *type, const char *name,
>     this->data.image_restrict = false;
>     this->data.from_ssbo_unsized_array = false;
>     this->data.fb_fetch_output = false;
> +   this->data.bindless = false;
>
>     if (type != NULL) {
>        if (type->is_sampler())
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index d7a81c5196..2e38131829 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -850,6 +850,12 @@ public:
>        unsigned fb_fetch_output:1;
>
>        /**
> +       * Non-zero if this variable is considered bindless as defined by
> +       * ARB_bindless_texture.
> +       */
> +      unsigned bindless:1;
> +
> +      /**
>         * Emit a warning if this variable is accessed.
>         */
>     private:
> diff --git a/src/compiler/glsl/ir_print_visitor.cpp b/src/compiler/glsl/ir_print_visitor.cpp
> index 1c84c1be16..1cafd3dd02 100644
> --- a/src/compiler/glsl/ir_print_visitor.cpp
> +++ b/src/compiler/glsl/ir_print_visitor.cpp
> @@ -194,6 +194,7 @@ void ir_print_visitor::visit(ir_variable *ir)
>     const char *const patc = (ir->data.patch) ? "patch " : "";
>     const char *const inv = (ir->data.invariant) ? "invariant " : "";
>     const char *const prec = (ir->data.precise) ? "precise " : "";
> +   const char *const bindless = (ir->data.bindless) ? "bindless " : "";
>     const char *const mode[] = { "", "uniform ", "shader_storage ",
>                                  "shader_shared ", "shader_in ", "shader_out ",
>                                  "in ", "out ", "inout ",
> @@ -202,8 +203,8 @@ void ir_print_visitor::visit(ir_variable *ir)
>     const char *const interp[] = { "", "smooth", "flat", "noperspective" };
>     STATIC_ASSERT(ARRAY_SIZE(interp) == INTERP_MODE_COUNT);
>
> -   fprintf(f, "(%s%s%s%s%s%s%s%s%s%s%s) ",
> -           binding, loc, component, cent, samp, patc, inv, prec, mode[ir->data.mode],
> +   fprintf(f, "(%s%s%s%s%s%s%s%s%s%s%s%s) ",
> +           binding, loc, component, cent, bindless, samp, patc, inv, prec, mode[ir->data.mode],
>             stream,
>             interp[ir->data.interpolation]);
>
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 154a46537f..d64f1e44cb 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2565,6 +2565,16 @@ struct gl_shader
>     bool origin_upper_left;
>     bool pixel_center_integer;
>
> +   /**
> +    * Whether bindless_sampler/bindless_image, and respectively
> +    * bound_sampler/bound_image are declared at global scope as defined by
> +    * ARB_bindless_texture.
> +    */
> +   bool bindless_sampler;
> +   bool bindless_image;
> +   bool bound_sampler;
> +   bool bound_image;
> +
>     /** Global xfb_stride out qualifier if any */
>     GLuint TransformFeedbackBufferStride[MAX_FEEDBACK_BUFFERS];
>
>


More information about the mesa-dev mailing list