This reverts to using the oword block read messages for uniform pull constant loads, as used to be the case until 4c1fdae0a01b3f92ec03b61aac1d3df5. There are two important differences though: Now the L3 cacheability bits are set up correctly, and we target the constant cache instead of the data cache. The latter turns out to get no L3 way allocation on boot on most platforms, so data cache messages are currently *not* cached on L3 regardless of the MOCS bits, what probably explains the apparent slowness of oword fetches back then.
Constant cache loads seem to perform better than SIMD4x2 sampler loads in a number of cases, they alleviate some of the cache thrashing caused by the competition with textures for the L1/L2 sampler caches, and they allow fetching up to 8 consecutive owords (128B) with just one message. FPS deltas relative to master are shown below for all generations since Gen6 and all oword block sizes from 1 to 8. 1oword 2oword 4oword 8oword OglShMapPcf SNB 3% 3% 5% 6% IVB 9% 11% 28% 41% BYT 5% 7% 25% 42% HSW 9% 9% 20% 30% BDW 3% 5% 19% 33% BSW 3% 5% 25% 44% ubo-worst SNB 2% 2% 2% 3% IVB -85% -71% -71% -71% BYT 14% 14% 14% 14% HSW 0% -1% -1% -1% BDW -40% 0% 0% 0% BSW 0% 0% 0% 0% ubo-best SNB 191% 190% 205% 205% IVB 152% 350% 474% 563% BYT 83% 208% 292% 353% HSW 292% 464% 615% 726% BDW 38% 267% 546% 581% BSW 124% 721% 1135% 580% shader-db HSW gained - lost 0 -3 -1 -1 instruction delta in 3.44% 1.90% -1.77% -3.48% affected programs OglShMapPcf is a PCF shadow mapping benchmark from SynMark that exercises pull constants and texture sampling, other tests from the Finnish benchmarking system show either a smaller improvement or no significant change. ubo-worst and ubo-best are the worst- and best-case scenarios of a simple microbenchmark that reads n constants (with n between 1 and 128) from a UBO, accessing up to 2kB of memory per invocation with alignment taken into account. Typically the gap between master and my constant cache branch increases with the amount of bandwidth used by the shader, with n=128 showing the greatest improvement. IVB's apparent worst-case regression deserves an explanation. After some investigation it seems like it's caused by a hardware bug leading to serialization of read requests to the L3 for the same cacheline as result of a (on IVB buggy) mechanism of the L3 to preserve coherency. As read requests for matching cachelines from any L3 client are not pipelined throughput will decrease in cases where there are no non-overlapping requests left in the queue that can be processed in between. I suspect that this situation is relatively uncommon in real-world applications, as the regression disappears completely from my microbenchmark as soon as each individual shader invocation accesses more than two non-overlapping cachelines from L3. To make this situation less likely we should make sure that we don't use the 1/2 oword messages at all if the shader intends to read from any other location in the same cacheline at some other point. This is generally a good idea anyway on all generations because using the 1 and 2 oword messages is expected to waste bandwidth since the minimum L3 request size for the DC is exactly 4 owords (i.e. one cacheline. This probably explains the negative result in the first column for BDW). A future commit will have this effect. I haven't been able to find any real-world example where this would still result in a regression, but if someone happens to find one it shouldn't be too difficult to add an IVB-specific heuristic that falls back to using the sampler for pull constant loads when a shader uses less than certain amount of L3 bandwidth. --- src/mesa/drivers/dri/i965/brw_eu_emit.c | 5 +- src/mesa/drivers/dri/i965/brw_fs.cpp | 34 ++++------- src/mesa/drivers/dri/i965/brw_fs.h | 2 +- src/mesa/drivers/dri/i965/brw_fs_generator.cpp | 84 ++++++++------------------ 4 files changed, 40 insertions(+), 85 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c b/src/mesa/drivers/dri/i965/brw_eu_emit.c index c2e490d..7829878 100644 --- a/src/mesa/drivers/dri/i965/brw_eu_emit.c +++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c @@ -2194,7 +2194,7 @@ gen7_block_read_scratch(struct brw_compile *p, } /** - * Read a float[4] vector from the data port Data Cache (const buffer). + * Read a float[4] vector from the data port constant cache. * Location (in buffer) should be a multiple of 16. * Used for fetching shader constants. */ @@ -2206,8 +2206,7 @@ void brw_oword_block_read(struct brw_compile *p, { struct brw_context *brw = p->brw; const unsigned target_cache = - (brw->gen >= 7 ? GEN7_SFID_DATAPORT_DATA_CACHE : - brw->gen >= 6 ? GEN6_SFID_DATAPORT_SAMPLER_CACHE : + (brw->gen >= 6 ? GEN6_SFID_DATAPORT_CONSTANT_CACHE : BRW_DATAPORT_READ_TARGET_DATA_CACHE); /* On newer hardware, offset is in units of owords. */ diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp index 35639de..4a61943 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp @@ -2990,32 +2990,22 @@ fs_visitor::lower_uniform_pull_constant_loads() continue; if (brw->gen >= 7) { - /* The offset arg before was a vec4-aligned byte offset. We need to - * turn it into a dword offset. - */ - fs_reg const_offset_reg = inst->src[1]; - assert(const_offset_reg.file == IMM && - const_offset_reg.type == BRW_REGISTER_TYPE_UD); - const_offset_reg.fixed_hw_reg.dw1.ud /= 4; + assert(inst->src[1].file == IMM && + inst->src[1].type == BRW_REGISTER_TYPE_UD); fs_reg payload = fs_reg(this, glsl_type::uint_type); + fs_inst *setup = MOV(payload, retype(brw_vec8_grf(0, 0), + BRW_REGISTER_TYPE_UD)); + setup->force_writemask_all = true; + setup->ir = inst->ir; + setup->annotation = inst->annotation; + inst->insert_before(block, setup); - /* We have to use a message header on Skylake to get SIMD4x2 mode. - * Reserve space for the register. - */ - if (brw->gen >= 9) { - payload.reg_offset++; - virtual_grf_sizes[payload.reg] = 2; - } - - /* This is actually going to be a MOV, but since only the first dword - * is accessed, we have a special opcode to do just that one. Note - * that this needs to be an operation that will be considered a def - * by live variable analysis, or register allocation will explode. + /* The offset arg before was a vec4-aligned byte offset. We need to + * turn it into an oword offset. */ - fs_inst *setup = new(mem_ctx) fs_inst(FS_OPCODE_SET_SIMD4X2_OFFSET, - 8, payload, const_offset_reg); + setup = MOV(component(payload, 2), + fs_reg(inst->src[1].fixed_hw_reg.dw1.ud / 16)); setup->force_writemask_all = true; - setup->ir = inst->ir; setup->annotation = inst->annotation; inst->insert_before(block, setup); diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h index 2aa58eb..8349ad2 100644 --- a/src/mesa/drivers/dri/i965/brw_fs.h +++ b/src/mesa/drivers/dri/i965/brw_fs.h @@ -805,7 +805,7 @@ private: void generate_uniform_pull_constant_load_gen7(fs_inst *inst, struct brw_reg dst, struct brw_reg surf_index, - struct brw_reg offset); + struct brw_reg payload); void generate_varying_pull_constant_load(fs_inst *inst, struct brw_reg dst, struct brw_reg index, struct brw_reg offset); diff --git a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp index ab848f1..3916c1a 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_generator.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_generator.cpp @@ -1003,44 +1003,14 @@ void fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst, struct brw_reg dst, struct brw_reg index, - struct brw_reg offset) + struct brw_reg payload) { assert(inst->mlen == 0); assert(index.type == BRW_REGISTER_TYPE_UD); - - assert(offset.file == BRW_GENERAL_REGISTER_FILE); - /* Reference just the dword we need, to avoid angering validate_reg(). */ - offset = brw_vec1_grf(offset.nr, 0); - - /* We use the SIMD4x2 mode because we want to end up with 4 components in - * the destination loaded consecutively from the same offset (which appears - * in the first component, and the rest are ignored). - */ - dst.width = BRW_WIDTH_4; - - struct brw_reg src = offset; - bool header_present = false; - int mlen = 1; - - if (brw->gen >= 9) { - /* Skylake requires a message header in order to use SIMD4x2 mode. */ - src = retype(brw_vec8_grf(offset.nr - 1, 0), BRW_REGISTER_TYPE_UD); - mlen = 2; - header_present = true; - - brw_push_insn_state(p); - brw_set_default_mask_control(p, BRW_MASK_DISABLE); - brw_MOV(p, src, retype(brw_vec8_grf(0, 0), BRW_REGISTER_TYPE_UD)); - brw_set_default_access_mode(p, BRW_ALIGN_1); - - brw_MOV(p, get_element_ud(src, 2), - brw_imm_ud(GEN9_SAMPLER_SIMD_MODE_EXTENSION_SIMD4X2)); - brw_pop_insn_state(p); - } + assert(payload.file == BRW_GENERAL_REGISTER_FILE); if (index.file == BRW_IMMEDIATE_VALUE) { - - uint32_t surf_index = index.dw1.ud; + const uint32_t surf_index = index.dw1.ud; brw_push_insn_state(p); brw_set_default_compression_control(p, BRW_COMPRESSION_NONE); @@ -1048,22 +1018,20 @@ fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst, brw_inst *send = brw_next_insn(p, BRW_OPCODE_SEND); brw_pop_insn_state(p); - brw_set_dest(p, send, dst); - brw_set_src0(p, send, src); - brw_set_sampler_message(p, send, + brw_set_dest(p, send, vec8(dst)); + brw_set_src0(p, send, payload); + brw_set_dp_read_message(p, send, surf_index, - 0, /* LD message ignores sampler unit */ - GEN5_SAMPLER_MESSAGE_SAMPLE_LD, - 1, /* rlen */ - mlen, - header_present, - BRW_SAMPLER_SIMD_MODE_SIMD4X2, - 0); + BRW_DATAPORT_OWORD_BLOCK_1_OWORDLOW, + GEN7_DATAPORT_DC_OWORD_BLOCK_READ, + GEN6_SFID_DATAPORT_CONSTANT_CACHE, + 1, /* mlen */ + true, /* header */ + 1); /* rlen */ brw_mark_surface_used(prog_data, surf_index); } else { - struct brw_reg addr = vec1(retype(brw_address_reg(0), BRW_REGISTER_TYPE_UD)); brw_push_insn_state(p); @@ -1077,29 +1045,27 @@ fs_generator::generate_uniform_pull_constant_load_gen7(fs_inst *inst, brw_set_src0(p, insn_and, vec1(retype(index, BRW_REGISTER_TYPE_UD))); brw_set_src1(p, insn_and, brw_imm_ud(0x0ff)); - /* a0.0 |= <descriptor> */ brw_inst *insn_or = brw_next_insn(p, BRW_OPCODE_OR); - brw_set_sampler_message(p, insn_or, - 0 /* surface */, - 0 /* sampler */, - GEN5_SAMPLER_MESSAGE_SAMPLE_LD, - 1 /* rlen */, - mlen, - header_present, - BRW_SAMPLER_SIMD_MODE_SIMD4X2, - 0); + brw_set_dp_read_message(p, insn_or, + 0, /* surface */ + BRW_DATAPORT_OWORD_BLOCK_1_OWORDLOW, + GEN7_DATAPORT_DC_OWORD_BLOCK_READ, + GEN6_SFID_DATAPORT_CONSTANT_CACHE, + 1, /* mlen */ + true, /* header */ + 1); /* rlen */ brw_inst_set_exec_size(p->brw, insn_or, BRW_EXECUTE_1); brw_inst_set_src1_reg_type(p->brw, insn_or, BRW_REGISTER_TYPE_UD); brw_set_src0(p, insn_or, addr); brw_set_dest(p, insn_or, addr); - - /* dst = send(offset, a0.0) */ + /* dst = send(payload, a0.0) */ brw_inst *insn_send = brw_next_insn(p, BRW_OPCODE_SEND); - brw_set_dest(p, insn_send, dst); - brw_set_src0(p, insn_send, src); - brw_set_indirect_send_descriptor(p, insn_send, BRW_SFID_SAMPLER, addr); + brw_set_dest(p, insn_send, vec8(dst)); + brw_set_src0(p, insn_send, payload); + brw_set_indirect_send_descriptor(p, insn_send, + GEN6_SFID_DATAPORT_CONSTANT_CACHE, addr); brw_pop_insn_state(p); -- 2.1.3 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev