[Mesa-dev] [PATCH 04/10] i965/fs: Make emit_spill/unspill static functions taking builder as argument.
Jason Ekstrand
jason at jlekstrand.net
Tue May 17 20:38:53 UTC 2016
On Tue, May 17, 2016 at 12:24 PM, Francisco Jerez <currojerez at riseup.net>
wrote:
> Jason Ekstrand <jason at jlekstrand.net> writes:
>
> > On Mon, May 16, 2016 at 9:22 PM, Francisco Jerez <currojerez at riseup.net>
> > 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
> >
> 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.
>
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.
> >
> >> 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 = unspill_dst.nr;
> >> 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(ibld.at(block, inst->next), spill_src,
> >> + subset_spill_offset, inst->regs_written);
> >> }
> >> }
> >>
> >> --
> >> 2.7.3
> >>
> >> _______________________________________________
> >> mesa-dev mailing list
> >> mesa-dev at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160517/9e2d7f96/attachment-0001.html>
More information about the mesa-dev
mailing list