On 03/05/16 20:30, Jordan Justen wrote: > On 2016-05-03 05:21:51, Samuel Iglesias Gonsálvez wrote: >> From: Iago Toral Quiroga <ito...@igalia.com> >> >> We should not offset into them based on the relative offset of >> our source and the destination of the instruction we are copy >> propagating from, so we don't turn this: >> >> mov(16) vgrf6:F, vgrf7+0.0<0>:F >> (...) >> load_payload(8) vgrf28:F, vgrf6+1.0:F 2ndhalf >> mov(8) vgrf29:DF, vgrf28:F 2ndhalf >> >> into: >> >> mov(16) vgrf6:F, vgrf7+0.0<0>:F >> (...) >> load_payload(8) vgrf28:F, vgrf7+1.0<0>:F 2ndhalf >> mov(8) vgrf29:DF, vgrf7+1.0<0>:F 2ndhalf >> >> and instead we do this: >> >> mov(16) vgrf6:F, vgrf7+0.0<0>:F >> (...) >> load_payload(8) vgrf28:F, vgrf7+0.0<0>:F 2ndhalf >> mov(8) vgrf29:DF, vgrf7+0.0<0>:F 2ndhalf >> --- >> src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp | 16 +++++++++++++--- >> 1 file changed, 13 insertions(+), 3 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 becc8bc..9147e60 100644 >> --- a/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp >> +++ b/src/mesa/drivers/dri/i965/brw_fs_copy_propagation.cpp >> @@ -460,10 +460,20 @@ fs_visitor::try_copy_propagate(fs_inst *inst, int arg, >> acp_entry *entry) >> * parts of vgrfs so we have to do some reg_offset magic. >> */ >> >> - /* Compute the offset of inst->src[arg] relative to inst->dst */ >> + /* Compute the offset of inst->src[arg] relative to inst->dst >> + * >> + * If the source we are copy propagating from has a stride of 0, >> then >> + * we must not offset into it based on the offset of our source >> + * relative to entry->dst >> + */ >> assert(entry->dst.subreg_offset == 0); >> - int rel_offset = inst->src[arg].reg_offset - entry->dst.reg_offset; >> - int rel_suboffset = inst->src[arg].subreg_offset; >> + int rel_offset, rel_suboffset; >> + if (entry->src.stride != 0) { >> + rel_offset = inst->src[arg].reg_offset - entry->dst.reg_offset; >> + rel_suboffset = inst->src[arg].subreg_offset; >> + } else { > > Should the comment added above go here instead? > > Reviewed-by: Jordan Justen <jordan.l.jus...@intel.com>
Right, I will move it to here. Thanks, Sam > >> + rel_offset = rel_suboffset = 0; >> + } >> >> /* Compute the final register offset (in bytes) */ >> int offset = entry->src.reg_offset * 32 + entry->src.subreg_offset; >> -- >> 2.5.0 >> >> _______________________________________________ >> mesa-dev mailing list >> mesa-dev@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev