<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, May 27, 2016 at 7:05 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">This will be useful in several places. The only externally visible difference (other than non-VGRF files being supported now) is that the region sizes are now passed in byte units instead of in GRF units because the loss of precision would have become a problem in the SIMD lowering pass. --- .../drivers/dri/i965/brw_fs_copy_propagation.cpp | 16 +++------ src/mesa/drivers/dri/i965/brw_ir_fs.h | 38 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp index 38103af..d88d62b 100644 --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp @@ -757,14 +757,6 @@ can_propagate_from(fs_inst *inst) !inst->is_partial_write()); } -inline bool -regions_overlap(const fs_reg &r, unsigned n, const fs_reg &s, unsigned m) -{ - return r.file == s.file && <a href="http://r.nr" rel="noreferrer" target="_blank">r.nr</a> == <a href="http://s.nr" rel="noreferrer" target="_blank">s.nr</a> && - !(r.reg_offset + n <= s.reg_offset || - s.reg_offset + m <= r.reg_offset); -} - /* Walks a basic block and does copy propagation on it using the acp * list. */ @@ -791,8 +783,8 @@ fs_visitor::opt_copy_propagate_local(void *copy_prop_ctx, bblock_t *block, /* kill the destination from the ACP */ if (inst->dst.file == VGRF) { foreach_in_list_safe(acp_entry, entry, &acp[inst-><a href="http://dst.nr" rel="noreferrer" target="_blank">dst.nr</a> % ACP_HASH_SIZE]) { - if (regions_overlap(entry->dst, entry->regs_written, - inst->dst, inst->regs_written)) + if (regions_overlap(entry->dst, entry->regs_written * REG_SIZE, + inst->dst, inst->regs_written * REG_SIZE)) entry->remove(); } @@ -804,8 +796,8 @@ fs_visitor::opt_copy_propagate_local(void *copy_prop_ctx, bblock_t *block, /* Make sure we kill the entry if this instruction overwrites * _any_ of the registers that it reads */ - if (regions_overlap(entry->src, entry->regs_read, - inst->dst, inst->regs_written)) + if (regions_overlap(entry->src, entry->regs_read * REG_SIZE, + inst->dst, inst->regs_written * REG_SIZE)) entry->remove(); } } diff --git a/src/mesa/drivers/dri/i965/brw_ir_fs.h b/src/mesa/drivers/dri/i965/brw_ir_fs.h index cc1561d..73c6327 100644 --- a/src/mesa/drivers/dri/i965/brw_ir_fs.h +++ b/src/mesa/drivers/dri/i965/brw_ir_fs.h @@ -172,6 +172,44 @@ component(fs_reg reg, unsigned idx) } /** + * Return an integer identifying the discrete address space a register is + * contained in. A register is by definition fully contained in the single + * reg_space it belongs to, so two registers with different reg_space ids are + * guaranteed not to overlap. Most register files are a single reg_space of + * its own, only the VGRF file is composed of multiple discrete address + * spaces, one for each VGRF allocation. + */ +static inline uint32_t +reg_space(const fs_reg &r) +{ + return r.file << 16 | (r.file == VGRF ? <a href="http://r.nr" rel="noreferrer" target="_blank">r.nr</a> : 0); +} </blockquote><div> </div><div>I think I like this concept. It would lead to a bit more compact numbering if you did "return r.file == VGRF ? <a href="http://r.nr">r.nr</a> + 8 : r.file". Obviously, you would probably want to replace "8" with a BRW_NUM_REG_FILES constant. </div><div>Given that we really only care about reg_space(r) == reg_space(s) right now, this is fine. </div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> + +/** + * Return the base offset in bytes of a register relative to the start of its + * reg_space(). + */ +static inline unsigned +reg_offset(const fs_reg &r) +{ + return ((r.file == VGRF || r.file == IMM ? 0 : <a href="http://r.nr" rel="noreferrer" target="_blank">r.nr</a>) + r.reg_offset) * + (r.file == UNIFORM ? 4 : REG_SIZE) + r.subreg_offset; +} + +/** + * Return whether the register region starting at \p r and spanning \p dr + * bytes could potentially overlap the register region starting at \p s and + * spanning \p ds bytes. + */ +static inline bool +regions_overlap(const fs_reg &r, unsigned dr, const fs_reg &s, unsigned ds) +{ + return reg_space(r) == reg_space(s) && + !(reg_offset(r) + dr <= reg_offset(s) || + reg_offset(s) + ds <= reg_offset(r)); +} + +/** * Return whether the given register region is n-periodic, i.e. whether the * original region remains invariant after shifting it by \p n scalar * channels. -- 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> </blockquote></div> </div></div>