<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jan 18, 2019 at 6:09 PM Francisco Jerez <<a href="mailto:currojerez@riseup.net">currojerez@riseup.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Because the "low" temporary needs to be accessed with word type and<br>
twice the original stride, attempting to preserve the alignment of the<br>
original destination can potentially lead to instructions with illegal<br>
destination stride greater than four. Because the CHV/BXT alignment<br>
restrictions are now being enforced by the regioning lowering pass run<br>
after lower_integer_multiplication(), there is no real need to<br>
preserve the original strides anymore.<br>
<br>
Note that this bug can be reproduced on stable branches, but<br>
back-porting would be non-trivial, because the fix relies on the<br>
regioning lowering pass recently introduced.<br>
---<br>
src/intel/compiler/brw_fs.cpp | 6 ++----<br>
1 file changed, 2 insertions(+), 4 deletions(-)<br>
<br>
diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp<br>
index f475b617df2..5768e0d6542 100644<br>
--- a/src/intel/compiler/brw_fs.cpp<br>
+++ b/src/intel/compiler/brw_fs.cpp<br>
@@ -3962,13 +3962,11 @@ fs_visitor::lower_integer_multiplication()<br>
regions_overlap(inst->dst, inst->size_written,<br>
inst->src[0], inst->size_read(0)) ||<br>
regions_overlap(inst->dst, inst->size_written,<br>
- inst->src[1], inst->size_read(1))) {<br>
+ inst->src[1], inst->size_read(1)) ||<br>
+ inst->dst.stride >= 4) {<br></blockquote><div><br></div><div>It would be nice to throw in a quick comment as to why we're adding a temporary when stride >= 4.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
needs_mov = true;<br>
- /* Get a new VGRF but keep the same stride as inst->dst */<br>
low = fs_reg(VGRF, alloc.allocate(regs_written(inst)),<br>
inst->dst.type);<br>
- low.stride = inst->dst.stride;<br>
- low.offset = inst->dst.offset % REG_SIZE;<br>
}<br>
<br>
/* Get a new VGRF but keep the same stride as inst->dst */<br>
-- <br>
2.19.2<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org" target="_blank">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</blockquote></div></div>