<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 | 4 + src/mesa/drivers/dri/i965/brw_eu.h | 25 ++++ src/mesa/drivers/dri/i965/brw_eu_emit.c | 166 +++++++++++++++++++++ src/mesa/drivers/dri/i965/brw_fs.cpp | 3 + src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 18 +++ .../drivers/dri/i965/brw_schedule_instructions.cpp | 3 + src/mesa/drivers/dri/i965/brw_shader.cpp | 2 + src/mesa/drivers/dri/i965/brw_vec4.cpp | 3 + src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 18 +++ 9 files changed, 242 insertions(+) diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h index 988b07e..631473a 100644 --- a/src/mesa/drivers/dri/i965/brw_defines.h +++ b/src/mesa/drivers/dri/i965/brw_defines.h @@ -780,6 +780,10 @@ enum opcode { SHADER_OPCODE_UNTYPED_SURFACE_READ, SHADER_OPCODE_UNTYPED_SURFACE_WRITE, + SHADER_OPCODE_TYPED_ATOMIC, + SHADER_OPCODE_TYPED_SURFACE_READ, + SHADER_OPCODE_TYPED_SURFACE_WRITE, + SHADER_OPCODE_GEN4_SCRATCH_READ, SHADER_OPCODE_GEN4_SCRATCH_WRITE, SHADER_OPCODE_GEN7_SCRATCH_READ, diff --git a/src/mesa/drivers/dri/i965/brw_eu.h b/src/mesa/drivers/dri/i965/brw_eu.h index e17dc49..17822ce 100644 --- a/src/mesa/drivers/dri/i965/brw_eu.h +++ b/src/mesa/drivers/dri/i965/brw_eu.h @@ -383,6 +383,31 @@ brw_untyped_surface_write(struct brw_compile *p, unsigned msg_length, unsigned num_channels); +void +brw_typed_atomic(struct brw_compile *p, + struct brw_reg dst, + struct brw_reg mrf, + struct brw_reg surface, + unsigned atomic_op, + unsigned msg_length, + bool response_expected); + +void +brw_typed_surface_read(struct brw_compile *p, + struct brw_reg dst, + struct brw_reg mrf, + struct brw_reg surface, + unsigned msg_length, + unsigned num_channels); + +void +brw_typed_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 13dd59a..772be7a 100644 --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c @@ -2753,6 +2753,172 @@ brw_untyped_surface_write(struct brw_compile *p, brw_send_indirect_message(p, sfid, dst, mrf, desc); } +static void +brw_set_dp_typed_atomic_message(struct brw_compile *p, + struct brw_instruction *insn, + unsigned atomic_op, + bool response_expected) +{ + const unsigned access_mode = p->current->header.access_mode; + const unsigned compression_control = p->current->header.compression_control; + + if (p->brw->is_haswell) { + if (access_mode == BRW_ALIGN_1) { + if (compression_control == GEN6_COMPRESSION_2Q) + insn->bits3.ud |= 1 << 12; /* Use high 8 slots of the sample mask */ + + insn->bits3.gen7_dp.msg_type = + HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP; + } else { + insn->bits3.gen7_dp.msg_type = + HSW_DATAPORT_DC_PORT1_TYPED_ATOMIC_OP_SIMD4X2; + } + + } else { + insn->bits3.gen7_dp.msg_type = GEN7_DATAPORT_RC_TYPED_ATOMIC_OP; + + if (compression_control == GEN6_COMPRESSION_2Q) + insn->bits3.ud |= 1 << 12; /* Use high 8 slots of the sample mask */ </blockquote><div> </div><div>As in the previous patch, it would be nice to have a brief reminder of why it is safe to use a SIMD8 operation in a SIMD4x2 shader. A similar comment applies to brw_set_dp_typed_surface_{read,write}_message(). </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + } + + if (response_expected) + insn->bits3.ud |= 1 << 13; /* Return data expected */ + + insn->bits3.ud |= atomic_op << 8; +} + +void +brw_typed_atomic(struct brw_compile *p, + struct brw_reg dst, + struct brw_reg mrf, + struct brw_reg surface, + unsigned atomic_op, + unsigned msg_length, + bool response_expected) { + const unsigned sfid = (p->brw->is_haswell ? HSW_SFID_DATAPORT_DATA_CACHE_1 : + GEN6_SFID_DATAPORT_RENDER_CACHE); + 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, + brw_surface_payload_size(p, response_expected, p->brw->is_haswell, false), + true); </blockquote><div> </div><div>My concern from patch 9 about passing surface directly to brw_load_indirect_message_descriptor() applies here as well. A similar comment applies to brw_typed_surface_{read,write}(). </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + + brw_set_dp_typed_atomic_message( + p, insn, atomic_op, response_expected); + + brw_send_indirect_message(p, sfid, dst, mrf, desc); +} + +static void +brw_set_dp_typed_surface_read_message(struct brw_compile *p, + struct brw_instruction *insn, + unsigned num_channels) +{ + const unsigned access_mode = p->current->header.access_mode; + const unsigned compression_control = p->current->header.compression_control; + + if (p->brw->is_haswell) { + insn->bits3.gen7_dp.msg_type = HSW_DATAPORT_DC_PORT1_TYPED_SURFACE_READ; + + if (access_mode == BRW_ALIGN_1) { + if (compression_control == GEN6_COMPRESSION_2Q) + insn->bits3.ud |= 2 << 12; /* Use high 8 slots of the sample mask */ + else + insn->bits3.ud |= 1 << 12; /* Use low 8 slots of the sample mask */ + } + + } else { + insn->bits3.gen7_dp.msg_type = GEN7_DATAPORT_RC_TYPED_SURFACE_READ; + + if (access_mode == BRW_ALIGN_1) { + if (compression_control == GEN6_COMPRESSION_2Q) + insn->bits3.ud |= 1 << 13; /* Use high 8 slots of the sample mask */ + } + } + + /* Set mask of unused channels. */ + insn->bits3.ud |= (0xf & (0xf << num_channels)) << 8; +} + +void +brw_typed_surface_read(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 : + GEN6_SFID_DATAPORT_RENDER_CACHE); + 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, + brw_surface_payload_size(p, num_channels, p->brw->is_haswell, false), + true); + + brw_set_dp_typed_surface_read_message( + p, insn, num_channels); + + brw_send_indirect_message(p, sfid, dst, mrf, desc); +} + +static void +brw_set_dp_typed_surface_write_message(struct brw_compile *p, + struct brw_instruction *insn, + unsigned num_channels) +{ + const unsigned access_mode = p->current->header.access_mode; + const unsigned compression_control = p->current->header.compression_control; + + if (p->brw->is_haswell) { + insn->bits3.gen7_dp.msg_type = HSW_DATAPORT_DC_PORT1_TYPED_SURFACE_WRITE; + + if (access_mode == BRW_ALIGN_1) { + if (compression_control == GEN6_COMPRESSION_2Q) + insn->bits3.ud |= 2 << 12; /* Use high 8 slots of the sample mask */ + else + insn->bits3.ud |= 1 << 12; /* Use low 8 slots of the sample mask */ + } + + } else { + insn->bits3.gen7_dp.msg_type = GEN7_DATAPORT_RC_TYPED_SURFACE_WRITE; + + if (access_mode == BRW_ALIGN_1) { + if (compression_control == GEN6_COMPRESSION_2Q) + insn->bits3.ud |= 1 << 13; /* Use high 8 slots of the sample mask */ + } + } + + /* Set mask of unused channels. */ + insn->bits3.ud |= (0xf & (0xf << num_channels)) << 8; +} + +void +brw_typed_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 : + GEN6_SFID_DATAPORT_RENDER_CACHE); + 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, true); + + brw_set_dp_typed_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 721162f..20cb4b9 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -782,6 +782,9 @@ fs_visitor::implied_mrf_writes(fs_inst *inst) case SHADER_OPCODE_UNTYPED_ATOMIC: case SHADER_OPCODE_UNTYPED_SURFACE_READ: case SHADER_OPCODE_UNTYPED_SURFACE_WRITE: + case SHADER_OPCODE_TYPED_ATOMIC: + case SHADER_OPCODE_TYPED_SURFACE_READ: + case SHADER_OPCODE_TYPED_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 2ebb90a..9601183 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp @@ -1696,6 +1696,24 @@ fs_generator::generate_code(exec_list *instructions) src[0], inst->mlen, src[1].dw1.ud); break; + case SHADER_OPCODE_TYPED_ATOMIC: + assert(src[1].file == BRW_IMMEDIATE_VALUE); + brw_typed_atomic(p, dst, brw_message_reg(inst->base_mrf), + src[0], src[1].dw1.ud, inst->mlen, true); + break; + + case SHADER_OPCODE_TYPED_SURFACE_READ: + assert(src[1].file == BRW_IMMEDIATE_VALUE); + brw_typed_surface_read(p, dst, brw_message_reg(inst->base_mrf), + src[0], inst->mlen, src[1].dw1.ud); + break; + + case SHADER_OPCODE_TYPED_SURFACE_WRITE: + assert(src[1].file == BRW_IMMEDIATE_VALUE); + brw_typed_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. Same comment applies to brw_vec4_generator.cpp. </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 39b63bc..0e4d8f1 100644 --- a/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp +++ b/src/mesa/drivers/dri/i965/brw_schedule_instructions.cpp @@ -338,6 +338,7 @@ schedule_node::set_latency_gen7(bool is_haswell) break; case SHADER_OPCODE_UNTYPED_ATOMIC: + case SHADER_OPCODE_TYPED_ATOMIC: /* 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 }; @@ -357,6 +358,8 @@ schedule_node::set_latency_gen7(bool is_haswell) case SHADER_OPCODE_UNTYPED_SURFACE_READ: case SHADER_OPCODE_UNTYPED_SURFACE_WRITE: + case SHADER_OPCODE_TYPED_SURFACE_READ: + case SHADER_OPCODE_TYPED_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>As in the previous patch, I don't think writes have the same latency as reads. </div> <div>With those issues addressed, the patch is: Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> </div></div></div></div>