<div dir="ltr">On 2 December 2013 11:39, Francisco Jerez <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">--- 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 */ </blockquote><div> </div><div>This looks like a mistake. Did you mean "|= 0 << 12;"? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + else + insn->bits3.ud |= 2 << 12; /* SIMD8 mode */ </blockquote><div> </div><div>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? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + } + + /* 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; + </blockquote><div> </div><div>As in the previous patch, I'm concerned that brw_mark_surface_used() isn't being called. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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 }; </blockquote><div> </div><div>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). </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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; + </blockquote><div> </div><div>Same comment about brw_mark_surface_used() applies here. </div><div>With those issues fixed, this patch is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div></div></div></div>