<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">This is required in combination with the following commit, because<br>
otherwise if a source region with an extended 8+ stride is present in<br>
the instruction (which we're about to declare legal) we'll end up<br>
emitting code that attempts to write to such a region, even though<br>
strides greater than four are still illegal for the destination.<br>
---<br>
 src/intel/compiler/brw_fs_lower_regioning.cpp | 20 ++++++++++++++-----<br>
 1 file changed, 15 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/src/intel/compiler/brw_fs_lower_regioning.cpp b/src/intel/compiler/brw_fs_lower_regioning.cpp<br>
index 6a3c23892b4..b86e95ed9eb 100644<br>
--- a/src/intel/compiler/brw_fs_lower_regioning.cpp<br>
+++ b/src/intel/compiler/brw_fs_lower_regioning.cpp<br>
@@ -71,15 +71,25 @@ namespace {<br>
           !is_byte_raw_mov(inst)) {<br>
          return get_exec_type_size(inst);<br>
       } else {<br>
-         unsigned stride = inst->dst.stride * type_sz(inst->dst.type);<br>
+         /* Calculate the maximum byte stride and the minimum type size across<br>
+          * all source and destination operands.<br>
+          */<br>
+         unsigned max_stride = inst->dst.stride * type_sz(inst->dst.type);<br>
+         unsigned min_size = type_sz(inst->dst.type);<br>
<br>
          for (unsigned i = 0; i < inst->sources; i++) {<br>
-            if (!is_uniform(inst->src[i]) && !inst->is_control_source(i))<br>
-               stride = MAX2(stride, inst->src[i].stride *<br>
-                             type_sz(inst->src[i].type));<br>
+            if (!is_uniform(inst->src[i]) && !inst->is_control_source(i)) {<br>
+               max_stride = MAX2(max_stride, inst->src[i].stride *<br>
+                                 type_sz(inst->src[i].type));<br>
+               min_size = MIN2(min_size, type_sz(inst->src[i].type));<br>
+            }<br>
          }<br>
<br>
-         return stride;<br>
+         /* Attempt to use the largest byte stride among all present operands,<br>
+          * but never exceed a stride of 4 since that would lead to illegal<br>
+          * destination regions during lowering.<br>
+          */<br>
+         return MIN2(max_stride, 4 * min_size);<br></blockquote><div><br></div><div>Why not just fall back to tightly packed in this case?  I think I can answer my own question:  Because using something that's equal to one of the strides reduces the liklihood that we'll need a temporary.  If that's the correct answer, then maybe what we want is the maximum of all strides with stride_in_bytes <= 4 * type_sz?</div><div><br></div><div>--Jason<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">
       }<br>
    }<br>
<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>