[Mesa-dev] [PATCH 21/24] i965/gen7: Handle atomic instructions from the FS back-end.
Paul Berry
stereotype441 at gmail.com
Thu Sep 26 13:56:56 PDT 2013
On 15 September 2013 00:19, Francisco Jerez <currojerez at riseup.net> wrote:
> This can deal with all the 15 32-bit untyped atomic operations the
> hardware supports, but only INC and PREDEC are going to be exposed
> through the API for now.
> ---
> src/mesa/drivers/dri/i965/brw_fs.h | 7 +++
> src/mesa/drivers/dri/i965/brw_fs_visitor.cpp | 83
> ++++++++++++++++++++++++++++
> 2 files changed, 90 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h
> b/src/mesa/drivers/dri/i965/brw_fs.h
> index dcd489c..44930f7 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.h
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h
> @@ -392,6 +392,13 @@ public:
> void emit_shader_time_write(enum shader_time_shader_type type,
> fs_reg value);
>
> + void emit_untyped_atomic(unsigned atomic_op, unsigned surf_index,
> + unsigned offset, fs_reg dst, fs_reg src0,
> + fs_reg src1);
> +
> + void emit_untyped_surface_read(unsigned surf_index, unsigned offset,
> + fs_reg dst);
> +
> bool try_rewrite_rhs_to_dst(ir_assignment *ir,
> fs_reg dst,
> fs_reg src,
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> index 762832a..412d27a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp
> @@ -2112,8 +2112,91 @@ fs_visitor::visit(ir_end_primitive *)
> }
>
> void
> +fs_visitor::emit_untyped_atomic(unsigned atomic_op, unsigned surf_index,
> + unsigned offset, fs_reg dst, fs_reg src0,
> + fs_reg src1)
> +{
> + const unsigned operand_len = dispatch_width / 8;
> + unsigned mlen = 0;
> +
> + /* Initialize the sample mask in the message header. */
> + emit(MOV(brw_uvec_mrf(8, mlen, 0), brw_imm_ud(0)))
> + ->force_writemask_all = true;
> + emit(MOV(brw_uvec_mrf(1, mlen, 7),
> + retype(brw_vec1_grf(1, 7), BRW_REGISTER_TYPE_UD)))
> + ->force_writemask_all = true;
>
For fragment shaders that don't use discard (fp->UsesKill == false) this
will do the right thing.
For fragment shaders that do use discard, we store the current pixel mask
in the f0.1 register, so you'll need to do something like this:
emit(MOV(brw_uvec_mrf(1, mlen, 7), brw_flag_reg(0, 1))->force_writemask_all
= true;
Otherwise pixels that have been discarded will erroneously get the atomic
operation applied to them.
Note: if all the pixels within a 2x2 subspan get discarded, we disable that
subspan in the execution mask. So in order to test this effectively,
you'll probably need to write a piglit test that discards only some of the
pixels within a subspan, and not others.
> + mlen++;
> +
> + /* Set the atomic operation offset. */
> + emit(MOV(brw_uvec_mrf(dispatch_width, mlen, 0), brw_imm_ud(offset)));
> + mlen += operand_len;
> +
> + /* Set the atomic operation arguments. */
> + if (src0.file != BAD_FILE) {
> + emit(MOV(brw_uvec_mrf(dispatch_width, mlen, 0), src0));
> + mlen += operand_len;
> + }
> +
> + if (src1.file != BAD_FILE) {
> + emit(MOV(brw_uvec_mrf(dispatch_width, mlen, 0), src1));
> + mlen += operand_len;
> + }
>
src0 is address and src1 is write data, right? It would be nice to have
that in a comment so that readers don't have to cross reference to the
bspec.
> +
> + /* Emit the instruction. */
> + fs_inst inst(SHADER_OPCODE_UNTYPED_ATOMIC, dst,
> + fs_reg(atomic_op), fs_reg(surf_index));
> + inst.base_mrf = 0;
> + inst.mlen = mlen;
> + emit(inst);
> +}
> +
> +void
> +fs_visitor::emit_untyped_surface_read(unsigned surf_index, unsigned
> offset,
> + fs_reg dst)
> +{
> + const unsigned operand_len = dispatch_width / 8;
> + unsigned mlen = 0;
> +
> + /* Initialize the sample mask in the message header. */
> + emit(MOV(brw_uvec_mrf(8, mlen, 0), brw_imm_ud(0)))
> + ->force_writemask_all = true;
> + emit(MOV(brw_uvec_mrf(1, mlen, 7),
> + retype(brw_vec1_grf(1, 7), BRW_REGISTER_TYPE_UD)))
> + ->force_writemask_all = true;
>
Same comment about discard applies here, although it's less critical
because this is a read operation (I suspect the only effect will be a tiny
performance penalty).
> + mlen++;
> +
> + /* Set the surface read offset. */
> + emit(MOV(brw_uvec_mrf(dispatch_width, mlen, 0), brw_imm_ud(offset)));
> + mlen += operand_len;
> +
> + /* Emit the instruction. */
> + fs_inst inst(SHADER_OPCODE_UNTYPED_SURFACE_READ, dst,
> fs_reg(surf_index));
> + inst.base_mrf = 0;
> + inst.mlen = mlen;
> + emit(inst);
> +}
> +
> +void
> fs_visitor::visit(ir_atomic *ir)
> {
> + ir_variable *loc = ir->location->variable_referenced();
> + unsigned surf_index = SURF_INDEX_WM_ABO(loc->atomic.buffer_index);
> +
> + result = fs_reg(this, ir->type);
> +
> + switch (ir->op) {
> + case ir_atomic_read:
> + emit_untyped_surface_read(surf_index, loc->atomic.offset, result);
> + break;
> + case ir_atomic_inc:
> + emit_untyped_atomic(BRW_AOP_INC, surf_index, loc->atomic.offset,
> + result, fs_reg(), fs_reg());
> + break;
> + case ir_atomic_dec:
> + emit_untyped_atomic(BRW_AOP_PREDEC, surf_index, loc->atomic.offset,
> + result, fs_reg(), fs_reg());
>
These calls to fs_reg() don't look right to me. Don't we need to pass the
address and the increment/decrement amount to emit_untyped_atomic()?
> + break;
> + }
> }
>
> fs_inst *
> --
> 1.8.3.4
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130926/c53488f6/attachment-0001.html>
More information about the mesa-dev
mailing list