[Mesa-dev] [RFC PATCH 03/26] glsl: introduce new base types for bindless samplers/images

Nicolai Hähnle nhaehnle at gmail.com
Tue Apr 18 18:14:47 UTC 2017


On 11.04.2017 18:48, Samuel Pitoiset wrote:
> Bindless sampler/image types are really different from the existing
> sampler/image types. They are considered 64-bit unsigned integers,
> they can be declared as temporary, shader inputs/outputs and are
> non-opaque types.
>
> For these reasons, it looks more convenient to introduce new
> internal base types to the GLSL compiler, called
> GLSL_TYPE_BINDLESS_SAMPLER and respectively GLSL_TYPE_BINDLESS_IMAGE.

Sorry for taking so long to get to this series, but could you explain 
the rationale here a bit more?

It seems that a lot of combinatorial explosion is generated by this 
change, and it's not clear to me where it actually helps. Conversely, it 
makes the changes here a lot bigger, and it makes the compiler's 
internal representation of types deviate from the terminology used in 
the GLSL spec. That's a good recipe for making the code less robust in 
the future. I really don't like it.

I get that towards the backend, it may be useful to distinguish between 
uniforms that are bound_* vs. uniforms that are bindless_*, but (without 
yet having looked at how you decided to tackle this at the Gallium 
level) it seems to me that recovering the bound_* performance could be 
relatively straightforward and doesn't necessarily need a type-level 
distinction, as long as copies are properly eliminated.

Can we just stick with a single set of sampler and image types, please?

Thanks,
Nicolai



>
> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
> ---
>  src/compiler/glsl/ast_to_hir.cpp                |  2 ++
>  src/compiler/glsl/ir.cpp                        |  2 +-
>  src/compiler/glsl/ir_clone.cpp                  |  2 ++
>  src/compiler/glsl/link_uniform_initializers.cpp |  2 ++
>  src/compiler/glsl_types.cpp                     |  4 ++++
>  src/compiler/glsl_types.h                       | 12 ++++++++----
>  src/intel/compiler/brw_fs.cpp                   |  2 ++
>  src/intel/compiler/brw_shader.cpp               |  2 ++
>  src/intel/compiler/brw_vec4_visitor.cpp         |  2 ++
>  src/mesa/program/ir_to_mesa.cpp                 |  4 ++++
>  src/mesa/state_tracker/st_glsl_types.cpp        |  2 ++
>  11 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
> index 27dc21fffe..38c0f5c8d4 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -1162,7 +1162,9 @@ do_comparison(void *mem_ctx, int operation, ir_rvalue *op0, ir_rvalue *op1)
>
>     case GLSL_TYPE_ERROR:
>     case GLSL_TYPE_VOID:
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
>     case GLSL_TYPE_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>     case GLSL_TYPE_IMAGE:
>     case GLSL_TYPE_INTERFACE:
>     case GLSL_TYPE_ATOMIC_UINT:
> diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp
> index 2bbc7a1cf7..3fbf0f68d3 100644
> --- a/src/compiler/glsl/ir.cpp
> +++ b/src/compiler/glsl/ir.cpp
> @@ -622,7 +622,7 @@ ir_constant::ir_constant(const struct glsl_type *type,
>     this->array_elements = NULL;
>
>     assert((type->base_type >= GLSL_TYPE_UINT)
> -	  && (type->base_type <= GLSL_TYPE_BOOL));
> +	  && (type->base_type <= GLSL_TYPE_BINDLESS_IMAGE));
>
>     this->type = type;
>     memcpy(& this->value, data, sizeof(this->value));
> diff --git a/src/compiler/glsl/ir_clone.cpp b/src/compiler/glsl/ir_clone.cpp
> index bfe2573c07..bc21964be7 100644
> --- a/src/compiler/glsl/ir_clone.cpp
> +++ b/src/compiler/glsl/ir_clone.cpp
> @@ -339,6 +339,8 @@ ir_constant::clone(void *mem_ctx, struct hash_table *ht) const
>     case GLSL_TYPE_BOOL:
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_INT64:
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>        return new(mem_ctx) ir_constant(this->type, &this->value);
>
>     case GLSL_TYPE_STRUCT: {
> diff --git a/src/compiler/glsl/link_uniform_initializers.cpp b/src/compiler/glsl/link_uniform_initializers.cpp
> index 8911c3de62..f2520393cf 100644
> --- a/src/compiler/glsl/link_uniform_initializers.cpp
> +++ b/src/compiler/glsl/link_uniform_initializers.cpp
> @@ -80,6 +80,8 @@ copy_constant_to_storage(union gl_constant_value *storage,
>        case GLSL_TYPE_VOID:
>        case GLSL_TYPE_SUBROUTINE:
>        case GLSL_TYPE_FUNCTION:
> +      case GLSL_TYPE_BINDLESS_SAMPLER:
> +      case GLSL_TYPE_BINDLESS_IMAGE:
>        case GLSL_TYPE_ERROR:
>           /* All other types should have already been filtered by other
>            * paths in the caller.
> diff --git a/src/compiler/glsl_types.cpp b/src/compiler/glsl_types.cpp
> index db65bb0e00..1b06eb68f4 100644
> --- a/src/compiler/glsl_types.cpp
> +++ b/src/compiler/glsl_types.cpp
> @@ -1281,6 +1281,8 @@ glsl_type::component_slots() const
>     case GLSL_TYPE_DOUBLE:
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_INT64:
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>        return 2 * this->components();
>
>     case GLSL_TYPE_STRUCT:
> @@ -1967,6 +1969,8 @@ glsl_type::count_attribute_slots(bool is_vertex_input) const
>     case GLSL_TYPE_INT:
>     case GLSL_TYPE_FLOAT:
>     case GLSL_TYPE_BOOL:
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>        return this->matrix_columns;
>     case GLSL_TYPE_DOUBLE:
>     case GLSL_TYPE_UINT64:
> diff --git a/src/compiler/glsl_types.h b/src/compiler/glsl_types.h
> index 7709556fe0..4cb330e4a3 100644
> --- a/src/compiler/glsl_types.h
> +++ b/src/compiler/glsl_types.h
> @@ -56,6 +56,8 @@ enum glsl_base_type {
>     GLSL_TYPE_UINT64,
>     GLSL_TYPE_INT64,
>     GLSL_TYPE_BOOL,
> +   GLSL_TYPE_BINDLESS_SAMPLER,
> +   GLSL_TYPE_BINDLESS_IMAGE,
>     GLSL_TYPE_SAMPLER,
>     GLSL_TYPE_IMAGE,
>     GLSL_TYPE_ATOMIC_UINT,
> @@ -70,9 +72,11 @@ enum glsl_base_type {
>
>  static inline bool glsl_base_type_is_64bit(enum glsl_base_type type)
>  {
> -   return type == GLSL_TYPE_DOUBLE ||
> -          type == GLSL_TYPE_UINT64 ||
> -          type == GLSL_TYPE_INT64;
> +   return type == GLSL_TYPE_DOUBLE           ||
> +          type == GLSL_TYPE_UINT64           ||
> +          type == GLSL_TYPE_INT64            ||
> +          type == GLSL_TYPE_BINDLESS_SAMPLER ||
> +          type == GLSL_TYPE_BINDLESS_IMAGE;
>  }
>
>  enum glsl_sampler_dim {
> @@ -439,7 +443,7 @@ struct glsl_type {
>     {
>        return (vector_elements == 1)
>  	 && (base_type >= GLSL_TYPE_UINT)
> -	 && (base_type <= GLSL_TYPE_BOOL);
> +	 && (base_type <= GLSL_TYPE_BINDLESS_IMAGE);
>     }
>
>     /**
> diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp
> index 9dc21ac5e3..f4f247ec25 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -486,6 +486,8 @@ type_size_scalar(const struct glsl_type *type)
>     case GLSL_TYPE_ERROR:
>     case GLSL_TYPE_INTERFACE:
>     case GLSL_TYPE_FUNCTION:
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>        unreachable("not reached");
>     }
>
> diff --git a/src/intel/compiler/brw_shader.cpp b/src/intel/compiler/brw_shader.cpp
> index 73bbc93135..45511be3c4 100644
> --- a/src/intel/compiler/brw_shader.cpp
> +++ b/src/intel/compiler/brw_shader.cpp
> @@ -64,6 +64,8 @@ brw_type_for_base_type(const struct glsl_type *type)
>     case GLSL_TYPE_ERROR:
>     case GLSL_TYPE_INTERFACE:
>     case GLSL_TYPE_FUNCTION:
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>        unreachable("not reached");
>     }
>
> diff --git a/src/intel/compiler/brw_vec4_visitor.cpp b/src/intel/compiler/brw_vec4_visitor.cpp
> index 262a084ca8..71376c3ae2 100644
> --- a/src/intel/compiler/brw_vec4_visitor.cpp
> +++ b/src/intel/compiler/brw_vec4_visitor.cpp
> @@ -625,6 +625,8 @@ type_size_xvec4(const struct glsl_type *type, bool as_vec4)
>     case GLSL_TYPE_ERROR:
>     case GLSL_TYPE_INTERFACE:
>     case GLSL_TYPE_FUNCTION:
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>        unreachable("not reached");
>     }
>
> diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp
> index 6b33266f1b..79c82faac7 100644
> --- a/src/mesa/program/ir_to_mesa.cpp
> +++ b/src/mesa/program/ir_to_mesa.cpp
> @@ -548,7 +548,9 @@ type_size(const struct glsl_type *type)
>  	 size += type_size(type->fields.structure[i].type);
>        }
>        return size;
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
>     case GLSL_TYPE_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>     case GLSL_TYPE_IMAGE:
>     case GLSL_TYPE_SUBROUTINE:
>        /* Samplers take up one slot in UNIFORMS[], but they're baked in
> @@ -2601,7 +2603,9 @@ _mesa_associate_uniform_storage(struct gl_context *ctx,
>  	    format = uniform_native;
>  	    columns = 1;
>  	    break;
> +	 case GLSL_TYPE_BINDLESS_SAMPLER:
>  	 case GLSL_TYPE_SAMPLER:
> +	 case GLSL_TYPE_BINDLESS_IMAGE:
>  	 case GLSL_TYPE_IMAGE:
>           case GLSL_TYPE_SUBROUTINE:
>  	    format = uniform_native;
> diff --git a/src/mesa/state_tracker/st_glsl_types.cpp b/src/mesa/state_tracker/st_glsl_types.cpp
> index 37c3164254..e1365d5ada 100644
> --- a/src/mesa/state_tracker/st_glsl_types.cpp
> +++ b/src/mesa/state_tracker/st_glsl_types.cpp
> @@ -69,6 +69,8 @@ st_glsl_attrib_type_size(const struct glsl_type *type, bool is_vs_input)
>        break;
>     case GLSL_TYPE_UINT64:
>     case GLSL_TYPE_INT64:
> +   case GLSL_TYPE_BINDLESS_SAMPLER:
> +   case GLSL_TYPE_BINDLESS_IMAGE:
>        if (type->vector_elements <= 2 || is_vs_input)
>           return 1;
>        else
>


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


More information about the mesa-dev mailing list