[Mesa-dev] [PATCH v5 40/70] nir: Implement __intrinsic_load_ssbo

[Kristian Høgsberg]

On Thu, Sep 10, 2015 at 03:35:56PM +0200, Iago Toral Quiroga wrote:
> v2:
> - Fix ssbo loads with boolean variables.
> 
> Reviewed-by: Connor Abbott <connor.w.abbott at intel.com>
> ---
>  src/glsl/nir/glsl_to_nir.cpp            | 80 ++++++++++++++++++++++++++++++++-
>  src/glsl/nir/nir_intrinsics.h           |  2 +-
>  src/glsl/nir/nir_lower_phis_to_scalar.c |  2 +
>  3 files changed, 81 insertions(+), 3 deletions(-)
> 
> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
> index 6f1e20a..cb7b196 100644
> --- a/src/glsl/nir/glsl_to_nir.cpp
> +++ b/src/glsl/nir/glsl_to_nir.cpp
> @@ -646,11 +646,14 @@ nir_visitor::visit(ir_call *ir)
>           op = nir_intrinsic_image_size;
>        } else if (strcmp(ir->callee_name(), "__intrinsic_store_ssbo") == 0) {
>           op = nir_intrinsic_store_ssbo;
> +      } else if (strcmp(ir->callee_name(), "__intrinsic_load_ssbo") == 0) {
> +         op = nir_intrinsic_load_ssbo;
>        } else {
>           unreachable("not reached");
>        }
>  
>        nir_intrinsic_instr *instr = nir_intrinsic_instr_create(shader, op);
> +      nir_alu_instr *load_ssbo_compare;
>  
>        switch (op) {
>        case nir_intrinsic_atomic_counter_read_var:
> @@ -776,11 +779,75 @@ nir_visitor::visit(ir_call *ir)
>           instr->src[1] = evaluate_rvalue(block);
>           break;
>        }
> +      case nir_intrinsic_load_ssbo: {
> +         exec_node *param = ir->actual_parameters.get_head();
> +         ir_rvalue *block = ((ir_instruction *)param)->as_rvalue();
> +
> +         param = param->get_next();
> +         ir_rvalue *offset = ((ir_instruction *)param)->as_rvalue();
> +
> +         /* Check if we need the indirect version */
> +         ir_constant *const_offset = offset->as_constant();
> +         if (!const_offset) {
> +            op = nir_intrinsic_load_ssbo_indirect;
> +            ralloc_free(instr);
> +            instr = nir_intrinsic_instr_create(shader, op);
> +            instr->src[1] = evaluate_rvalue(offset);
> +            instr->const_index[0] = 0;
> +         } else {
> +            instr->const_index[0] = const_offset->value.u[0];
> +         }
> +
> +         instr->src[0] = evaluate_rvalue(block);
> +
> +         const glsl_type *type = ir->return_deref->var->type;
> +         instr->num_components = type->vector_elements;
> +
> +         /* Setup destination register */
> +         nir_ssa_dest_init(&instr->instr, &instr->dest,
> +                           type->vector_elements, NULL);
> +
> +         /* Insert the created nir instruction now since in the case of boolean
> +          * result we will need to emit another instruction after it
> +          */
> +         nir_instr_insert_after_cf_list(this->cf_node_list, &instr->instr);

I'd prefer moving this insert into the GLSL_TYPE_BOOL condition below...

> +         /*
> +          * In SSBO/UBO's, a true boolean value is any non-zero value, but we
> +          * consider a true boolean to be ~0. Fix this up with a != 0
> +          * comparison.
> +          */
> +         if (type->base_type == GLSL_TYPE_BOOL) {

... that is, here...

> +            nir_load_const_instr *const_zero =
> +               nir_load_const_instr_create(shader, 1);
> +            const_zero->value.u[0] = 0;
> +            nir_instr_insert_after_cf_list(this->cf_node_list,
> +                                           &const_zero->instr);
> +
> +            load_ssbo_compare = nir_alu_instr_create(shader, nir_op_ine);

and then, insteed of introducing and using 'load_ssbo_compare' here,
re-use 'instr' for the compare instruction...

> +            load_ssbo_compare->src[0].src.is_ssa = true;
> +            load_ssbo_compare->src[0].src.ssa = &instr->dest.ssa;
> +            load_ssbo_compare->src[1].src.is_ssa = true;
> +            load_ssbo_compare->src[1].src.ssa = &const_zero->def;
> +            for (unsigned i = 0; i < type->vector_elements; i++)
> +               load_ssbo_compare->src[1].swizzle[i] = 0;
> +            nir_ssa_dest_init(&load_ssbo_compare->instr,
> +                              &load_ssbo_compare->dest.dest,
> +                              type->vector_elements, NULL);
> +            load_ssbo_compare->dest.write_mask = (1 << type->vector_elements) - 1;
> +            nir_instr_insert_after_cf_list(this->cf_node_list,
> +                                           &load_ssbo_compare->instr);
> +         }
> +
> +         break;
> +      }
>        default:
>           unreachable("not reached");
>        }
>  
> -      nir_instr_insert_after_cf_list(this->cf_node_list, &instr->instr);
> +      /* nir_intrinsic_load_ssbo{_indirect} were already emitted */
> +      if (op != nir_intrinsic_load_ssbo && op != nir_intrinsic_load_ssbo_indirect)
> +         nir_instr_insert_after_cf_list(this->cf_node_list, &instr->instr);
>  

That way we don't need anything special here for ssbo_load...

>        if (ir->return_deref) {
>           nir_intrinsic_instr *store_instr =
> @@ -789,7 +856,16 @@ nir_visitor::visit(ir_call *ir)
>  
>           store_instr->variables[0] =
>              evaluate_deref(&store_instr->instr, ir->return_deref);
> -         store_instr->src[0] = nir_src_for_ssa(&instr->dest.ssa);
> +
> +         /* If nir_intrinsic_load_ssbo{_indirect} is loading a boolean variable,
> +          * the value is on load_ssbo_compare's dest. Use it instead.
> +          */
> +         if ((op == nir_intrinsic_load_ssbo || op == nir_intrinsic_load_ssbo_indirect) &&
> +             ir->return_deref->var->type->base_type == GLSL_TYPE_BOOL) {
> +            store_instr->src[0] = nir_src_for_ssa(&load_ssbo_compare->dest.dest.ssa);
> +         } else {
> +            store_instr->src[0] = nir_src_for_ssa(&instr->dest.ssa);
> +         }

nor here.

>           nir_instr_insert_after_cf_list(this->cf_node_list,
>                                          &store_instr->instr);
> diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
> index 38f22c1..53066c6 100644
> --- a/src/glsl/nir/nir_intrinsics.h
> +++ b/src/glsl/nir/nir_intrinsics.h
> @@ -174,7 +174,7 @@ SYSTEM_VALUE(invocation_id, 1)
>  LOAD(uniform, 0, 2, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>  LOAD(ubo, 1, 1, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
>  LOAD(input, 0, 1, NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)
> -/* LOAD(ssbo, 1, 0) */
> +LOAD(ssbo, 1, 1, NIR_INTRINSIC_CAN_ELIMINATE)
>  
>  /*
>   * Stores work the same way as loads, except now the first register input is
> diff --git a/src/glsl/nir/nir_lower_phis_to_scalar.c b/src/glsl/nir/nir_lower_phis_to_scalar.c
> index 739170d..bdb7e6a 100644
> --- a/src/glsl/nir/nir_lower_phis_to_scalar.c
> +++ b/src/glsl/nir/nir_lower_phis_to_scalar.c
> @@ -94,6 +94,8 @@ is_phi_src_scalarizable(nir_phi_src *src,
>        case nir_intrinsic_load_uniform_indirect:
>        case nir_intrinsic_load_ubo:
>        case nir_intrinsic_load_ubo_indirect:
> +      case nir_intrinsic_load_ssbo:
> +      case nir_intrinsic_load_ssbo_indirect:
>        case nir_intrinsic_load_input:
>        case nir_intrinsic_load_input_indirect:
>           return true;
> -- 
> 1.9.1
>