[Mesa-dev] [PATCH 11/25] i965: Add the untyped surface write opcode.
Paul Berry
stereotype441 at gmail.com
Tue Dec 31 09:02:25 PST 2013
On 2 December 2013 11:39, Francisco Jerez <currojerez at riseup.net> wrote:
> ---
> src/mesa/drivers/dri/i965/brw_defines.h | 1 +
> src/mesa/drivers/dri/i965/brw_eu.h | 8 ++++
> src/mesa/drivers/dri/i965/brw_eu_emit.c | 49
> ++++++++++++++++++++++
> src/mesa/drivers/dri/i965/brw_fs.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 6 +++
> .../drivers/dri/i965/brw_schedule_instructions.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_shader.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_vec4.cpp | 1 +
> src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 6 +++
> 9 files changed, 74 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h
> b/src/mesa/drivers/dri/i965/brw_defines.h
> index 67a2aaa..988b07e 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -778,6 +778,7 @@ enum opcode {
>
> SHADER_OPCODE_UNTYPED_ATOMIC,
> SHADER_OPCODE_UNTYPED_SURFACE_READ,
> + SHADER_OPCODE_UNTYPED_SURFACE_WRITE,
>
> SHADER_OPCODE_GEN4_SCRATCH_READ,
> SHADER_OPCODE_GEN4_SCRATCH_WRITE,
> diff --git a/src/mesa/drivers/dri/i965/brw_eu.h
> b/src/mesa/drivers/dri/i965/brw_eu.h
> index 45b421b..e17dc49 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu.h
> +++ b/src/mesa/drivers/dri/i965/brw_eu.h
> @@ -375,6 +375,14 @@ brw_untyped_surface_read(struct brw_compile *p,
> unsigned msg_length,
> unsigned num_channels);
>
> +void
> +brw_untyped_surface_write(struct brw_compile *p,
> + struct brw_reg dst,
> + struct brw_reg mrf,
> + struct brw_reg surface,
> + unsigned msg_length,
> + unsigned num_channels);
> +
> /***********************************************************************
> * brw_eu_util.c:
> */
> diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> index b94a6d1..13dd59a 100644
> --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
> +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
> @@ -2704,6 +2704,55 @@ brw_untyped_surface_read(struct brw_compile *p,
> brw_send_indirect_message(p, sfid, dst, mrf, desc);
> }
>
> +static void
> +brw_set_dp_untyped_surface_write_message(struct brw_compile *p,
> + struct brw_instruction *insn,
> + unsigned num_channels)
> +{
> + insn->bits3.gen7_dp.msg_type = (p->brw->is_haswell ?
> +
> HSW_DATAPORT_DC_PORT1_UNTYPED_SURFACE_WRITE :
> +
> GEN7_DATAPORT_DC_UNTYPED_SURFACE_WRITE);
> +
> + if (p->current->header.access_mode == BRW_ALIGN_1) {
> + if (p->compressed)
> + insn->bits3.ud |= 1 << 12; /* SIMD16 mode */
> + else
> + insn->bits3.ud |= 2 << 12; /* SIMD8 mode */
> + } else {
> + if (p->brw->is_haswell)
> + insn->bits3.ud |= 2 << 12; /* SIMD4x2 mode */
>
This looks like a mistake. Did you mean "|= 0 << 12;"?
> + else
> + insn->bits3.ud |= 2 << 12; /* SIMD8 mode */
>
I remember when we were discussing the atomic operations, you had an
argument for why it was safe to use SIMD8 mode on IVB when compiling a
SIMD4x2 shader. Does that argument still apply here? Can you recap it in
a comment, please?
> + }
> +
> + /* Set mask of 32-bit channels to drop. */
> + insn->bits3.ud |= (0xf & (0xf << num_channels)) << 8;
> +}
> +
> +void
> +brw_untyped_surface_write(struct brw_compile *p,
> + struct brw_reg dst,
> + struct brw_reg mrf,
> + struct brw_reg surface,
> + unsigned msg_length,
> + unsigned num_channels)
> +{
> + const unsigned sfid = (p->brw->is_haswell ?
> HSW_SFID_DATAPORT_DATA_CACHE_1 :
> + GEN7_SFID_DATAPORT_DATA_CACHE);
> + const bool header_present = p->current->header.access_mode ==
> BRW_ALIGN_1;
> + struct brw_reg desc = retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD);
> + struct brw_instruction *insn;
> +
> + insn = brw_load_indirect_message_descriptor(
> + p, desc, surface, msg_length, 0,
> + header_present);
> +
> + brw_set_dp_untyped_surface_write_message(
> + p, insn, num_channels);
> +
> + brw_send_indirect_message(p, sfid, dst, mrf, desc);
> +}
> +
> /**
> * This instruction is generated as a single-channel align1 instruction by
> * both the VS and FS stages when using INTEL_DEBUG=shader_time.
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp
> b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 4408cbe..721162f 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -781,6 +781,7 @@ fs_visitor::implied_mrf_writes(fs_inst *inst)
> return 2;
> case SHADER_OPCODE_UNTYPED_ATOMIC:
> case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> + case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> return 0;
> default:
> assert(!"not reached");
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> index 0d50051..2ebb90a 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp
> @@ -1690,6 +1690,12 @@ fs_generator::generate_code(exec_list *instructions)
> src[0], inst->mlen, src[1].dw1.ud);
> break;
>
> + case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> + assert(src[1].file == BRW_IMMEDIATE_VALUE);
> + brw_untyped_surface_write(p, dst,
> brw_message_reg(inst->base_mrf),
> + src[0], inst->mlen, src[1].dw1.ud);
> + break;
> +
>
As in the previous patch, I'm concerned that brw_mark_surface_used() isn't
being called.
> case FS_OPCODE_SET_SIMD4X2_OFFSET:
> generate_set_simd4x2_offset(inst, dst, src[0]);
> break;
> diff --git a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> index baf67fb..39b63bc 100644
> --- a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp
> @@ -356,6 +356,7 @@ schedule_node::set_latency_gen7(bool is_haswell)
> break;
>
> case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> + case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> /* Test code:
> * mov(8) g112<1>UD 0x00000000UD { align1 WE_all
> 1Q };
> * mov(1) g112.7<1>UD g1.7<0,1,0>UD { align1 WE_all };
>
Surface write has the same latency as surface read? From a scheduling
perspective I would expect the latency of surface write to be very small,
since there is no response message (and therefore there is nothing to stop
the EU from dispatching additional instructions after the SEND).
> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp
> b/src/mesa/drivers/dri/i965/brw_shader.cpp
> index 128354a..2824515 100644
> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp
> @@ -659,6 +659,7 @@ bool
> backend_instruction::has_side_effects() const
> {
> switch (opcode) {
> + case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> case SHADER_OPCODE_UNTYPED_ATOMIC:
> return true;
> default:
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> index 9d5d26f..2bf2c88 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
> @@ -293,6 +293,7 @@ vec4_visitor::implied_mrf_writes(vec4_instruction
> *inst)
> return inst->header_present ? 1 : 0;
> case SHADER_OPCODE_UNTYPED_ATOMIC:
> case SHADER_OPCODE_UNTYPED_SURFACE_READ:
> + case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> return 0;
> default:
> assert(!"not reached");
> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> index d29c3dd..f06282c 100644
> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
> @@ -1172,6 +1172,12 @@
> vec4_generator::generate_vec4_instruction(vec4_instruction *instruction,
> src[0], inst->mlen, src[1].dw1.ud);
> break;
>
> + case SHADER_OPCODE_UNTYPED_SURFACE_WRITE:
> + assert(src[1].file == BRW_IMMEDIATE_VALUE);
> + brw_untyped_surface_write(p, dst, brw_message_reg(inst->base_mrf),
> + src[0], inst->mlen, src[1].dw1.ud);
> + break;
> +
>
Same comment about brw_mark_surface_used() applies here.
With those issues fixed, this patch is:
Reviewed-by: Paul Berry <stereotype441 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20131231/5fb97fc8/attachment-0001.html>
More information about the mesa-dev
mailing list