On Tue, Apr 3, 2018 at 6:21 AM, Karol Herbst <kher...@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 <kher...@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).
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
