<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Sep 29, 2014 at 9:50 AM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Sep 26, 2014 at 12:24 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> This commit reworks copy propagation a bit to support propagating the<br>
> copying of partial registers. This comes up every time we have pull<br>
> constants because we do a pull constant read immediately followed by a move<br>
> to splat the one component of the out to 8 or 16-wide. This allows us to<br>
> eliminate the copy and simply use the one component of the register.<br>
><br>
> Shader DB results:<br>
><br>
> total instructions in shared programs: 5044937 -> 5044428 (-0.01%)<br>
> instructions in affected programs: 66112 -> 65603 (-0.77%)<br>
> GAINED: 0<br>
> LOST: 0<br>
><br>
> Signed-off-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>><br>
> ---<br>
> src/mesa/drivers/dri/i965/brw_fs.h | 1 +<br>
> .../drivers/dri/i965/brw_fs_copy_propagation.cpp | 83 ++++++++++++++++------<br>
> 2 files changed, 64 insertions(+), 20 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h<br>
> index 50b5fc1..9b63114 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs.h<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h<br>
> @@ -337,6 +337,7 @@ public:<br>
> bool opt_cse_local(bblock_t *block);<br>
> bool opt_copy_propagate();<br>
> bool try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry);<br>
> + bool try_constant_propagate(fs_inst *inst, acp_entry *entry);<br>
> bool opt_copy_propagate_local(void *mem_ctx, bblock_t *block,<br>
> exec_list *acp);<br>
> void opt_drop_redundant_mov_to_flags();<br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp<br>
> index e5816df..a97dc04 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp<br>
> @@ -277,24 +277,30 @@ is_logic_op(enum opcode opcode)<br>
> bool<br>
> fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)<br>
> {<br>
> + if (inst->src[arg].file != GRF)<br>
> + return false;<br>
> +<br>
> if (entry->src.file == IMM)<br>
> return false;<br>
> + assert(entry->src.file == GRF || entry->src.file == UNIFORM);<br>
><br>
> if (entry->opcode == SHADER_OPCODE_LOAD_PAYLOAD &&<br>
> inst->opcode == SHADER_OPCODE_LOAD_PAYLOAD)<br>
> return false;<br>
><br>
> - /* Bail if inst is reading more than entry is writing. */<br>
> - if ((inst->regs_read(this, arg) * inst->src[arg].stride *<br>
> - type_sz(inst->src[arg].type)) > type_sz(entry->dst.type))<br>
> + assert(entry->dst.file == GRF);<br>
> + if (inst->src[arg].reg != entry->dst.reg)<br>
> return false;<br>
><br>
> - if (inst->src[arg].file != entry->dst.file ||<br>
> - inst->src[arg].reg != entry->dst.reg ||<br>
> - inst->src[arg].reg_offset != entry->dst.reg_offset ||<br>
> - inst->src[arg].subreg_offset != entry->dst.subreg_offset) {<br>
> + /* Bail if inst is reading a range that isn't contained in the range<br>
> + * that entry is writing.<br>
> + */<br>
> + int reg_size = dispatch_width * sizeof(float);<br>
> + if (inst->src[arg].reg_offset < entry->dst.reg_offset ||<br>
> + (inst->src[arg].reg_offset * reg_size + inst->src[arg].subreg_offset +<br>
> + inst->regs_read(this, arg) * inst->src[arg].stride * reg_size) ><br>
> + (entry->dst.reg_offset + 1) * reg_size)<br>
> return false;<br>
> - }<br>
><br>
> /* See resolve_ud_negate() and comment in brw_fs_emit.cpp. */<br>
> if (inst->conditional_mod &&<br>
> @@ -361,11 +367,39 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)<br>
><br>
> inst->src[arg].file = entry->src.file;<br>
> inst->src[arg].reg = entry->src.reg;<br>
> - inst->src[arg].reg_offset = entry->src.reg_offset;<br>
> - inst->src[arg].subreg_offset = entry->src.subreg_offset;<br>
> inst->src[arg].stride *= entry->src.stride;<br>
> inst->saturate = inst->saturate || entry->saturate;<br>
><br>
> + switch (entry->src.file) {<br>
> + case BAD_FILE:<br>
> + case HW_REG:<br>
> + case UNIFORM:<br>
> + inst->src[arg].reg_offset = entry->src.reg_offset;<br>
> + inst->src[arg].subreg_offset = entry->src.subreg_offset;<br>
> + break;<br>
> + case GRF:<br>
> + {<br>
> + /* In this case, we have to deal with mapping parts of vgrfs to<br>
> + * other parts of vgrfs so we have to do some reg_offset magic.<br>
> + */<br>
> +<br>
> + /* Compute the offset of inst->src[arg] relative to inst->dst */<br>
> + assert(entry->dst.subreg_offset == 0);<br>
> + int rel_offset = inst->src[arg].reg_offset - entry->dst.reg_offset;<br>
> + int rel_suboffset = inst->src[arg].subreg_offset;<br>
> +<br>
> + /* Compute the final register offset (in bytes) */<br>
> + int offset = entry->src.reg_offset * reg_size + entry->src.subreg_offset;<br>
> + offset += rel_offset * reg_size + rel_suboffset;<br>
> + inst->src[arg].reg_offset = offset / reg_size;<br>
> + inst->src[arg].subreg_offset = offset % reg_size;<br>
> + }<br>
> + break;<br>
> + default:<br>
> + unreachable("Invalid register file");<br>
> + break;<br>
> + }<br>
> +<br>
> if (!inst->src[arg].abs) {<br>
> inst->src[arg].abs = entry->src.abs;<br>
> inst->src[arg].negate ^= entry->src.negate;<br>
> @@ -375,9 +409,8 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, acp_entry *entry)<br>
> }<br>
><br>
><br>
> -static bool<br>
> -try_constant_propagate(struct brw_context *brw, fs_inst *inst,<br>
> - acp_entry *entry)<br>
> +bool<br>
> +fs_visitor::try_constant_propagate(fs_inst *inst, acp_entry *entry)<br>
> {<br>
> bool progress = false;<br>
><br>
> @@ -385,12 +418,22 @@ try_constant_propagate(struct brw_context *brw, fs_inst *inst,<br>
> return false;<br>
><br>
> for (int i = inst->sources - 1; i >= 0; i--) {<br>
> - if (inst->src[i].file != entry->dst.file ||<br>
> - inst->src[i].reg != entry->dst.reg ||<br>
> - inst->src[i].reg_offset != entry->dst.reg_offset ||<br>
> - inst->src[i].subreg_offset != entry->dst.subreg_offset ||<br>
> - inst->src[i].type != entry->dst.type ||<br>
> - inst->src[i].stride > 1)<br>
> + if (inst->src[i].file != GRF)<br>
> + continue;<br>
> +<br>
> + assert(entry->dst.file == GRF);<br>
> + if (inst->src[i].reg != entry->dst.reg ||<br>
> + inst->src[i].type != entry->dst.type)<br>
> + continue;<br>
> +<br>
> + /* Bail if inst is reading a range that isn't contained in the range<br>
> + * that entry is writing.<br>
> + */<br>
> + int reg_size = dispatch_width * sizeof(float);<br>
> + if (inst->src[i].reg_offset < entry->dst.reg_offset ||<br>
> + (inst->src[i].reg_offset * reg_size + inst->src[i].subreg_offset +<br>
> + inst->regs_read(this, i) * inst->src[i].stride * reg_size) ><br>
> + (entry->dst.reg_offset + 1) * reg_size)<br>
> continue;<br>
<br>
</div></div>Does this bit, the constant propagation code, actually benefit from<br>
this? I guess it'd let an immediate loaded with a mov(16) be constant<br>
propagated into two SIMD8 instructions? That seems plausible, I was<br>
just a little surprised at first.<br>
</blockquote></div><br></div><div class="gmail_extra">Yes, that's exactly what it's for<br></div></div>