[Mesa-dev] [RFC PATCH 07/26] glsl: process bindless/bound layout qualifiers
Samuel Pitoiset
samuel.pitoiset at gmail.com
Wed Apr 12 09:41:14 UTC 2017
On 04/12/2017 01:54 AM, Timothy Arceri wrote:
>
>
> 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.
Yeah, I have forgot to clean up that part.
How about merge_bindless_qualifier() with 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