[Mesa-dev] [PATCH v2 05/31] glsl: process bindless/bound layout qualifiers
Timothy Arceri
tarceri at itsqueeze.com
Wed Apr 26 02:01:10 UTC 2017
On 24/04/17 20:35, 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 | 65 ++++++++++++++++++++++++++++++++
> src/compiler/glsl/ast_type.cpp | 53 ++++++++++++++++++++++++++
> 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 | 2 +
> src/compiler/glsl/ir.h | 12 ++++++
> src/compiler/glsl/ir_print_visitor.cpp | 8 ++--
> src/mesa/main/mtypes.h | 10 +++++
> 10 files changed, 197 insertions(+), 3 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 aeb223db9e..c9772ff83e 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -3417,6 +3417,68 @@ 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)
> +{
> + bool has_local_qualifiers = qual->flags.q.bindless_sampler ||
> + qual->flags.q.bindless_image ||
> + qual->flags.q.bound_sampler ||
> + qual->flags.q.bound_image;
> +
> + /* 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 (has_local_qualifiers && !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) &&
> + !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) &&
> + !var->type->contains_image()) {
> + _mesa_glsl_error(loc, state, "bindless_image or bound_image can only be "
> + "applied to image types");
> + return;
> + }
> +
> + /* The bindless_sampler/bindless_image (and respectively
> + * bound_sampler/bound_image) layout qualifiers can be set at global and at
> + * local scope.
> + */
> + if (var->type->contains_sampler() || var->type->contains_image()) {
> + var->data.bindless = qual->flags.q.bindless_sampler ||
> + qual->flags.q.bindless_image ||
> + state->bindless_sampler_specified ||
> + state->bindless_image_specified;
nit. Mind adding a space here? I find is easier to read that way.
> + var->data.bound = qual->flags.q.bound_sampler ||
> + qual->flags.q.bound_image ||
> + state->bound_sampler_specified ||
> + state->bound_image_specified;
> + }
> +}
> +
> +static void
> apply_layout_qualifier_to_variable(const struct ast_type_qualifier *qual,
> ir_variable *var,
> struct _mesa_glsl_parse_state *state,
> @@ -3713,6 +3775,9 @@ 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 (state->has_bindless())
> + 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..af43eb1a35 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,33 @@ validate_point_mode(MAYBE_UNUSED const ast_type_qualifier &qualifier,
> return true;
> }
>
> +static void
> +merge_bindless_qualifier(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;
> + }
> +}
> +
> /**
> * This function merges duplicate layout identifiers.
> *
> @@ -393,6 +424,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 +470,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)
> + merge_bindless_qualifier(loc, state, *this, q);
Indentation is wrong here.
With this and the suggestion above this patch is:
Reviewed-by: Timothy Arceri <tarceri at itsqueeze.com>
> +
> return r;
> }
>
> @@ -778,6 +827,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 b7303aba42..d731e35e21 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 3b0cbee3b8..01f68a321c 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -1747,6 +1747,8 @@ 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;
> + this->data.bound = false;
>
> if (type != NULL) {
> if (type->is_sampler())
> diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h
> index d7a81c5196..584714ef82 100644
> --- a/src/compiler/glsl/ir.h
> +++ b/src/compiler/glsl/ir.h
> @@ -850,6 +850,18 @@ public:
> unsigned fb_fetch_output:1;
>
> /**
> + * Non-zero if this variable is considered bindless as defined by
> + * ARB_bindless_texture.
> + */
> + unsigned bindless:1;
> +
> + /**
> + * Non-zero if this variable is considered bound as defined by
> + * ARB_bindless_texture.
> + */
> + unsigned bound: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 6c1c86a582..f8d0dc4dec 100644
> --- a/src/compiler/glsl/ir_print_visitor.cpp
> +++ b/src/compiler/glsl/ir_print_visitor.cpp
> @@ -193,6 +193,8 @@ 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 bound = (ir->data.bound) ? "bound " : "";
> const char *const mode[] = { "", "uniform ", "shader_storage ",
> "shader_shared ", "shader_in ", "shader_out ",
> "in ", "out ", "inout ",
> @@ -201,9 +203,9 @@ 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],
> - stream,
> + fprintf(f, "(%s%s%s%s%s%s%s%s%s%s%s%s%s) ",
> + binding, loc, component, cent, bindless, bound, samp, patc, inv,
> + prec, mode[ir->data.mode], stream,
> interp[ir->data.interpolation]);
>
> print_type(f, ir->type);
> diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
> index 7d4d1bcd4c..8a4a14c656 100644
> --- a/src/mesa/main/mtypes.h
> +++ b/src/mesa/main/mtypes.h
> @@ -2564,6 +2564,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