[Mesa-dev] [RFC PATCH 07/26] glsl: process bindless/bound layout qualifiers
Timothy Arceri
tarceri at itsqueeze.com
Wed Apr 12 10:04:12 UTC 2017
On 12/04/17 19:41, Samuel Pitoiset wrote:
>
>
> 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?
>
Looks good to me :)
More information about the mesa-dev
mailing list