On 19/08/15 16:47, Francisco Jerez wrote:
Martin Peres <martin.pe...@linux.intel.com> writes:v2, review from Francisco Jerez: - make the destination variable as large as what the nir instrinsic defines (4) instead of the size of the return variable of glsl. This is still safe for the already existing code because all the intrinsics affected returned the same amount of components as expected by glsl IR. In the case of image_size, it is not possible to do so because the returned number of component depends on the image type and this case is not well handled by nir. Signed-off-by: Martin Peres <martin.pe...@linux.intel.com> --- src/glsl/nir/glsl_to_nir.cpp | 21 +++++++++++++++------ src/glsl/nir/nir_intrinsics.h | 2 ++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/glsl/nir/glsl_to_nir.cpp b/src/glsl/nir/glsl_to_nir.cpp index 77327b6..3063243 100644 --- a/src/glsl/nir/glsl_to_nir.cpp +++ b/src/glsl/nir/glsl_to_nir.cpp @@ -641,6 +641,8 @@ nir_visitor::visit(ir_call *ir) op = nir_intrinsic_image_atomic_comp_swap; } else if (strcmp(ir->callee_name(), "__intrinsic_memory_barrier") == 0) { op = nir_intrinsic_memory_barrier; + } else if (strcmp(ir->callee_name(), "__intrinsic_image_size") == 0) { + op = nir_intrinsic_image_size; } else { unreachable("not reached"); } @@ -666,7 +668,8 @@ nir_visitor::visit(ir_call *ir) case nir_intrinsic_image_atomic_or: case nir_intrinsic_image_atomic_xor: case nir_intrinsic_image_atomic_exchange: - case nir_intrinsic_image_atomic_comp_swap: { + case nir_intrinsic_image_atomic_comp_swap: + case nir_intrinsic_image_size: { nir_ssa_undef_instr *instr_undef = nir_ssa_undef_instr_create(shader, 1); nir_instr_insert_after_cf_list(this->cf_node_list, @@ -681,6 +684,17 @@ nir_visitor::visit(ir_call *ir) instr->variables[0] = evaluate_deref(&instr->instr, image); param = param->get_next();+ /* Set the intrinsic destination. */+ if (ir->return_deref) { + const nir_intrinsic_info *info; + info = &nir_intrinsic_infos[instr->intrinsic];I'm a fan of initializing things at the same point where they are defined. You only use it once though, you may want to drop the declaration altogether.
It exceeded the 80-char limit. I guess I can put the assignation on the following line.
+ nir_ssa_dest_init(&instr->instr, &instr->dest, + info->dest_components, NULL); + } + + if (op == nir_intrinsic_image_size) + break; + /* Set the address argument, extending the coordinate vector to four * components. */ @@ -721,11 +735,6 @@ nir_visitor::visit(ir_call *ir) instr->src[3] = evaluate_rvalue((ir_dereference *)param); param = param->get_next(); } - - /* Set the intrinsic destination. */ - if (ir->return_deref) - nir_ssa_dest_init(&instr->instr, &instr->dest, - ir->return_deref->type->vector_elements, NULL); break; } case nir_intrinsic_memory_barrier: diff --git a/src/glsl/nir/nir_intrinsics.h b/src/glsl/nir/nir_intrinsics.h index bc6e6b8..6c7a61a 100644 --- a/src/glsl/nir/nir_intrinsics.h +++ b/src/glsl/nir/nir_intrinsics.h @@ -123,6 +123,8 @@ INTRINSIC(image_atomic_or, 3, ARR(4, 1, 1), true, 1, 1, 0, 0) INTRINSIC(image_atomic_xor, 3, ARR(4, 1, 1), true, 1, 1, 0, 0) INTRINSIC(image_atomic_exchange, 3, ARR(4, 1, 1), true, 1, 1, 0, 0) INTRINSIC(image_atomic_comp_swap, 4, ARR(4, 1, 1, 1), true, 1, 1, 0, 0) +INTRINSIC(image_size, 0, ARR(), true, 4, 1, 0, + NIR_INTRINSIC_CAN_ELIMINATE | NIR_INTRINSIC_CAN_REORDER)Looks good to me, Reviewed-by: Francisco Jerez <curroje...@riseup.net>
Thanks for the review, I will address the comments on the previous patch!
#define SYSTEM_VALUE(name, components) \ INTRINSIC(load_##name, 0, ARR(), true, components, 0, 0, \ -- 2.5.0 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
