[Mesa-dev] [RFC PATCH 03/26] glsl: introduce new base types for bindless samplers/images
Samuel Pitoiset
samuel.pitoiset at gmail.com
Tue Apr 18 19:30:47 UTC 2017
On 04/18/2017 08:14 PM, Nicolai Hähnle wrote:
> 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?
No worries, all feedbacks are always welcome, even late. :)
So, the bindless sampler/image types introduced in ARB_bindless_texture
are really different from the "standard" ones.
They are considered as 64-bit unsigned integers (not 32-bit) and they
are non-opaque types. The latter means they can be declared as temporary
variables, as shader inputs/outputs, inside an interface block, etc.
That said, the current sampler/image types are opaque (cf,
glsl_type::is_opaque()) and it seemed quite impossible to change the
glsl_type helpers to fit my needs.
I tried many different solutions before figuring this one which seems
better for some reasons:
- easy to make bindless sampler/image types 64-bit unsigned int
- easy to make bindless sampler/image types non-opaque
- should avoid breakage because the base type is different
- reduce the amount of changes in most places in the compiler
At the Gallium level, the changes are really small. Basically, if the
type is a bindless sampler, the ir_dereference variable is visited and
it can be considered as PROGRAM_UNIFORM or PROGRAM_TEMPORARY (for shader
inputs/outputs).
Hopefully, you are convinced now. :)
>
> 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
>>
>
>
More information about the mesa-dev
mailing list