Yes, that's fine - we modify instr so we need to update dest.
Kristian
> On Sep 24, 2015, at 1:59 AM, Samuel Iglesias Gonsálvez <sigles...@igalia.com>
> wrote:
>
>
>
>> On 23/09/15 12:07, Samuel Iglesias Gonsalvez wrote:
>> From: Iago Toral Quiroga <ito...@igalia.com>
>>
>> v2:
>> - Fix ssbo loads with boolean variables.
>>
>> v3:
>> - Simplify the changes (Kristian)
>>
>> Reviewed-by: Connor Abbott <connor.w.abb...@intel.com>
>> ---
>> src/glsl/nir/glsl_to_nir.cpp | 66
>> +++++++++++++++++++++++++++++++++
>> src/glsl/nir/nir_intrinsics.h | 2 +-
>> src/glsl/nir/nir_lower_phis_to_scalar.c | 2 +
>> 3 files changed, 69 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp
>> index 93d5d23..906797e 100644
>> --- a/src/glsl/nir/glsl_to_nir.cpp
>> +++ b/src/glsl/nir/glsl_to_nir.cpp
>> @@ -651,6 +651,8 @@ nir_visitor::visit(ir_call *ir)
>> op = nir_intrinsic_image_samples;
>> } 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");
>> }
>> @@ -790,6 +792,70 @@ nir_visitor::visit(ir_call *ir)
>> nir_instr_insert_after_cf_list(this->cf_node_list, &instr->instr);
>> 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;
>
> I forgot to update 'dest' with the new instruction here. The fix is just
> an one-liner:
>
> + dest = &instr->dest;
>
> If you don't mind, I will include it as part of this patch before
> pushing it to master.
>
> The same happens to a similar code in nir_intrinsic_store_ssbo case. My
> plan is to add the same one-line change in the previous patch (nir:
> modify the instruction insertion in nir_visitor::visit(ir_call *ir))
>
> Kristian, Do you agree with these changes?
>
> Sam
>
>> + } 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);
>> +
>> + /*
>> + * 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) {
>> + 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);
>> +
>> + nir_alu_instr *load_ssbo_compare =
>> + nir_alu_instr_create(shader, nir_op_ine);
>> + 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);
>> + dest = &load_ssbo_compare->dest.dest;
>> + }
>> +
>> + break;
>> + }
>> default:
>> unreachable("not reached");
>> }
>> diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h
>> index 53f541b..4a2dd79 100644
>> --- a/src/glsl/nir/nir_intrinsics.h
>> +++ b/src/glsl/nir/nir_intrinsics.h
>> @@ -179,7 +179,7 @@ SYSTEM_VALUE(user_clip_plane, 4, 1) /* const_index[0] is
>> user_clip_plane[idx] */
>> 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 d72a71d..aa124d9 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;
>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
