<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, May 17, 2016 at 12:24 PM, Francisco Jerez <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes: > On Mon, May 16, 2016 at 9:22 PM, Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> > wrote: > >> This seems cleaner than exposing an implementation detail of >> brw_fs_reg_allocate.cpp to the world, and will give the caller control >> over the instruction execution flags (e.g. force_writemask_all) that >> are applied to the scratch read and write instructions. >> --- >> src/mesa/drivers/dri/i965/brw_fs.h | 4 --- >> src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp | 41 >> ++++++++++++----------- >> 2 files changed, 21 insertions(+), 24 deletions(-) >> >> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h >> b/src/mesa/drivers/dri/i965/brw_fs.h >> index bf30d65..53a3557 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs.h >> +++ b/src/mesa/drivers/dri/i965/brw_fs.h >> @@ -211,10 +211,6 @@ public: >> bool opt_saturate_propagation(); >> bool opt_cmod_propagation(); >> bool opt_zero_samples(); >> - void emit_unspill(bblock_t *block, fs_inst *inst, fs_reg reg, >> - uint32_t spill_offset, unsigned count); >> - void emit_spill(bblock_t *block, fs_inst *inst, fs_reg reg, >> - uint32_t spill_offset, unsigned count); >> >> void emit_nir_code(); >> void nir_setup_inputs(); >> diff --git a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp >> b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp >> index 2260147..5fc586f 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs_reg_allocate.cpp >> @@ -764,16 +764,17 @@ namespace { >> } >> } >> >> -void >> -fs_visitor::emit_unspill(bblock_t *block, fs_inst *inst, fs_reg dst, >> - uint32_t spill_offset, unsigned count) >> +static void >> +emit_unspill(const fs_builder &bld, fs_reg dst, >> + uint32_t spill_offset, unsigned count) >> { >> + const brw_device_info *devinfo = bld.shader->devinfo; >> + const fs_visitor *v = static_cast<const fs_visitor *>(bld.shader); >> unsigned reg_size = 1; >> - if (dispatch_width == 16 && count % 2 == 0) >> + if (v->dispatch_width == 16 && count % 2 == 0) >> reg_size = 2; >> >> - const fs_builder ibld = fs_builder(this, block, inst) >> - .group(reg_size * 8, 0); >> + const fs_builder ibld = bld.group(reg_size * 8, 0); >> >> for (unsigned i = 0; i < count / reg_size; i++) { >> /* The Gen7 descriptor-based offset is 12 bits of HWORD units. >> Because >> @@ -793,7 +794,7 @@ fs_visitor::emit_unspill(bblock_t *block, fs_inst >> *inst, fs_reg dst, >> unspill_inst->regs_written = reg_size; >> >> if (!gen7_read) { >> - unspill_inst->base_mrf = spill_base_mrf(this); >> + unspill_inst->base_mrf = spill_base_mrf(bld.shader); >> > > This makes the static_cast in spill_base_mrf make a bit more sense > </div></div>Right, we either do it here and emit_spill() or in spill_max_size() alone. Because the number of reserved registers for spilling is a property of the shader rather than the visitor it kind of made more sense to me to give spill_max_size() a backend_shader object, but I'm okay either way, it's up to you. </blockquote><div> </div><div>I don't care that much. Putting the cast in spill_max_size() means we can pass bld.shader directly. Feel free to leave it as-is if you'd like. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"> > >> unspill_inst->mlen = 1; /* header contains offset */ >> } >> >> @@ -802,17 +803,16 @@ fs_visitor::emit_unspill(bblock_t *block, fs_inst >> *inst, fs_reg dst, >> } >> } >> >> -void >> -fs_visitor::emit_spill(bblock_t *block, fs_inst *inst, fs_reg src, >> - uint32_t spill_offset, unsigned count) >> +static void >> +emit_spill(const fs_builder &bld, fs_reg src, >> + uint32_t spill_offset, unsigned count) >> { >> + const fs_visitor *v = static_cast<const fs_visitor *>(bld.shader); >> unsigned reg_size = 1; >> - if (dispatch_width == 16 && count % 2 == 0) >> + if (v->dispatch_width == 16 && count % 2 == 0) >> reg_size = 2; >> >> - const fs_builder ibld = fs_builder(this, block, inst) >> - .at(block, inst->next) >> - .group(reg_size * 8, 0); >> + const fs_builder ibld = bld.group(reg_size * 8, 0); >> >> for (unsigned i = 0; i < count / reg_size; i++) { >> fs_inst *spill_inst = >> @@ -820,7 +820,7 @@ fs_visitor::emit_spill(bblock_t *block, fs_inst *inst, >> fs_reg src, >> src.reg_offset += reg_size; >> spill_inst->offset = spill_offset + i * reg_size * REG_SIZE; >> spill_inst->mlen = 1 + reg_size; /* header, value */ >> - spill_inst->base_mrf = spill_base_mrf(this); >> + spill_inst->base_mrf = spill_base_mrf(bld.shader); >> } >> } >> >> @@ -936,6 +936,8 @@ fs_visitor::spill_reg(int spill_reg) >> * could just spill/unspill the GRF being accessed. >> */ >> foreach_block_and_inst (block, fs_inst, inst, cfg) { >> + const fs_builder ibld = fs_builder(this, block, inst); >> + >> for (unsigned int i = 0; i < inst->sources; i++) { >> if (inst->src[i].file == VGRF && >> inst->src[i].nr == spill_reg) { >> @@ -947,8 +949,7 @@ fs_visitor::spill_reg(int spill_reg) >> inst->src[i].nr = <a href="http://unspill_dst.nr" rel="noreferrer" target="_blank">unspill_dst.nr</a>; >> inst->src[i].reg_offset = 0; >> >> - emit_unspill(block, inst, unspill_dst, subset_spill_offset, >> - regs_read); >> + emit_unspill(ibld, unspill_dst, subset_spill_offset, >> regs_read); >> } >> } >> >> @@ -974,11 +975,11 @@ fs_visitor::spill_reg(int spill_reg) >> * since we write back out all of the regs_written(). >> */ >> if (inst->is_partial_write()) >> - emit_unspill(block, inst, spill_src, subset_spill_offset, >> + emit_unspill(ibld, spill_src, subset_spill_offset, >> inst->regs_written); >> >> - emit_spill(block, inst, spill_src, subset_spill_offset, >> - inst->regs_written); >> + emit_spill(<a href="http://ibld.at" rel="noreferrer" target="_blank">ibld.at</a>(block, inst->next), spill_src, >> + subset_spill_offset, inst->regs_written); >> } >> } >> >> -- >> 2.7.3 >> >> _______________________________________________ >> mesa-dev mailing list >> <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> >> <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> >> </div></div></blockquote></div> </div></div>