[Mesa-dev] [PATCH 4/4] RFC nir: add support for bindless_texture images

Jason Ekstrand jason at jlekstrand.net
Wed Apr 4 00:21:27 UTC 2018


On Tue, Apr 3, 2018 at 6:21 AM, Karol Herbst <kherbst at redhat.com> wrote:

> I added another source for all image_var_* intrinsics. Drivers have to be
> adjusted with this change.
>
> There was some discussion to add new intrinsics to handle operations on
> bindless images. Maybe we can continue with this here?
>
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
>  src/compiler/glsl/glsl_to_nir.cpp  | 19 +++++++++++++++++--
>  src/compiler/nir/nir.h             |  2 +-
>  src/compiler/nir/nir_intrinsics.py | 24 ++++++++++++------------
>  3 files changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/src/compiler/glsl/glsl_to_nir.cpp
> b/src/compiler/glsl/glsl_to_nir.cpp
> index 1fc0cac4736..4e053c140c2 100644
> --- a/src/compiler/glsl/glsl_to_nir.cpp
> +++ b/src/compiler/glsl/glsl_to_nir.cpp
> @@ -894,10 +894,14 @@ nir_visitor::visit(ir_call *ir)
>           ir_dereference *image = (ir_dereference *)param;
>           const glsl_type *type =
>              image->type->without_array();
> +         bool bindless = image->variable_referenced()->
> contains_bindless();
>
>           instr->variables[0] = evaluate_deref(&instr->instr, image);
>           param = param->get_next();
>
> +         if (bindless)
> +            instr->variables[0]->var->data.bindless = true;
> +
>           /* Set the intrinsic destination. */
>           if (ir->return_deref) {
>              unsigned num_components = ir->return_deref->type->
> vector_elements;
> @@ -909,6 +913,11 @@ nir_visitor::visit(ir_call *ir)
>
>           if (op == nir_intrinsic_image_var_size ||
>               op == nir_intrinsic_image_var_samples) {
> +            if (bindless) {
> +               instr->src[0] = nir_src_for_ssa(evaluate_rvalue(image));
> +            } else {
> +               instr->src[0] = nir_src_for_ssa(&instr_undef->def);
> +            }
>              nir_builder_instr_insert(&b, &instr->instr);
>              break;
>           }
> @@ -941,15 +950,21 @@ nir_visitor::visit(ir_call *ir)
>              instr->src[1] = nir_src_for_ssa(&instr_undef->def);
>           }
>
> +         if (bindless) {
> +            instr->src[2] = nir_src_for_ssa(evaluate_rvalue(image));
> +         } else {
> +            instr->src[2] = nir_src_for_ssa(&instr_undef->def);
> +         }
> +
>           /* Set the intrinsic parameters. */
>           if (!param->is_tail_sentinel()) {
> -            instr->src[2] =
> +            instr->src[3] =
>                 nir_src_for_ssa(evaluate_rvalue((ir_dereference *)param));
>              param = param->get_next();
>           }
>
>           if (!param->is_tail_sentinel()) {
> -            instr->src[3] =
> +            instr->src[4] =
>                 nir_src_for_ssa(evaluate_rvalue((ir_dereference *)param));
>              param = param->get_next();
>           }
> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
> index e4d626d263e..c6081cbb61f 100644
> --- a/src/compiler/nir/nir.h
> +++ b/src/compiler/nir/nir.h
> @@ -1108,7 +1108,7 @@ typedef enum {
>
>  } nir_intrinsic_index_flag;
>
> -#define NIR_INTRINSIC_MAX_INPUTS 4
> +#define NIR_INTRINSIC_MAX_INPUTS 5
>
>  typedef struct {
>     const char *name;
> diff --git a/src/compiler/nir/nir_intrinsics.py b/src/compiler/nir/nir_
> intrinsics.py
> index 1bc99552cd7..d6da63ab769 100644
> --- a/src/compiler/nir/nir_intrinsics.py
> +++ b/src/compiler/nir/nir_intrinsics.py
> @@ -291,19 +291,19 @@ atomic3("atomic_counter_comp_swap")
>  # argument with the value to be written, and image atomic operations take
>  # either one or two additional scalar arguments with the same meaning as
> in
>  # the ARB_shader_image_load_store specification.
> -intrinsic("image_var_load", src_comp=[4, 1], dest_comp=4, num_vars=1,
> +intrinsic("image_var_load", src_comp=[4, 1, 1], dest_comp=4, num_vars=1,
>            flags=[CAN_ELIMINATE])
> -intrinsic("image_var_store", src_comp=[4, 1, 4], num_vars=1)
> -intrinsic("image_var_atomic_add",  src_comp=[4, 1, 1], dest_comp=1,
> num_vars=1)
> -intrinsic("image_var_atomic_min",  src_comp=[4, 1, 1], dest_comp=1,
> num_vars=1)
> -intrinsic("image_var_atomic_max",  src_comp=[4, 1, 1], dest_comp=1,
> num_vars=1)
> -intrinsic("image_var_atomic_and",  src_comp=[4, 1, 1], dest_comp=1,
> num_vars=1)
> -intrinsic("image_var_atomic_or",   src_comp=[4, 1, 1], dest_comp=1,
> num_vars=1)
> -intrinsic("image_var_atomic_xor",  src_comp=[4, 1, 1], dest_comp=1,
> num_vars=1)
> -intrinsic("image_var_atomic_exchange",  src_comp=[4, 1, 1], dest_comp=1,
> num_vars=1)
> -intrinsic("image_var_atomic_comp_swap", src_comp=[4, 1, 1, 1],
> dest_comp=1, num_vars=1)
> -intrinsic("image_var_size",    dest_comp=0, num_vars=1,
> flags=[CAN_ELIMINATE, CAN_REORDER])
> -intrinsic("image_var_samples", dest_comp=1, num_vars=1,
> flags=[CAN_ELIMINATE, CAN_REORDER])
> +intrinsic("image_var_store", src_comp=[4, 1, 1, 4], num_vars=1)
> +intrinsic("image_var_atomic_add",  src_comp=[4, 1, 1, 1], dest_comp=1,
> num_vars=1)
> +intrinsic("image_var_atomic_min",  src_comp=[4, 1, 1, 1], dest_comp=1,
> num_vars=1)
> +intrinsic("image_var_atomic_max",  src_comp=[4, 1, 1, 1], dest_comp=1,
> num_vars=1)
> +intrinsic("image_var_atomic_and",  src_comp=[4, 1, 1, 1], dest_comp=1,
> num_vars=1)
> +intrinsic("image_var_atomic_or",   src_comp=[4, 1, 1, 1], dest_comp=1,
> num_vars=1)
> +intrinsic("image_var_atomic_xor",  src_comp=[4, 1, 1, 1], dest_comp=1,
> num_vars=1)
> +intrinsic("image_var_atomic_exchange",  src_comp=[4, 1, 1, 1],
> dest_comp=1, num_vars=1)
> +intrinsic("image_var_atomic_comp_swap", src_comp=[4, 1, 1, 1, 1],
> dest_comp=1, num_vars=1)
> +intrinsic("image_var_size",    src_comp=[1], dest_comp=0, num_vars=1,
> flags=[CAN_ELIMINATE, CAN_REORDER])
> +intrinsic("image_var_samples", src_comp=[1], dest_comp=1, num_vars=1,
> flags=[CAN_ELIMINATE, CAN_REORDER])
>

I said this on IRC, but I really think we want separate intrinsics for
bindless.  I really don't like them taking a handle from a source but then
pulling a bunch of data from a variable.  Either use a variable or don't.
I'd rather we just add a set of bindless variants that have a handle as an
additional source and some const indices for things such as format and
qualifier flags (read/write-only).
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180403/3ad97681/attachment.html>


More information about the mesa-dev mailing list