<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Oct 26, 2017 at 1:39 AM, Iago Toral <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, 2017-10-25 at 16:25 -0700, Jason Ekstrand wrote:<br>
> Before, we were careful to place the zip after the last of the split<br>
> instructions but did unzip on-demand.  This changes things so that<br>
> the<br>
> unzips go before all of the split instructions and the unzip comes<br>
> explicitly after all the split instructions.  As a side-effect of<br>
> this<br>
> change, we now emit the split instruction from highest SIMD group to<br>
> lowest instead of low to high.  We could have kept the old behavior,<br>
> but<br>
> it shouldn't matter and this made the code easier.<br>
<br>
</span>Took me a while to review that all this checked out, but it seems to do<br>
what you explain here. I have a couple of minor comments below.<br>
<span class=""><br>
<br>
> Cc: <a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.<wbr>org</a><br>
> ---<br>
>  src/intel/compiler/brw_fs.cpp | 7 ++++---<br>
>  1 file changed, 4 insertions(+), 3 deletions(-)<br>
><br>
> diff --git a/src/intel/compiler/brw_fs.<wbr>cpp<br>
> b/src/intel/compiler/brw_fs.<wbr>cpp<br>
> index 49ca58d..ef36af9 100644<br>
> --- a/src/intel/compiler/brw_fs.<wbr>cpp<br>
> +++ b/src/intel/compiler/brw_fs.<wbr>cpp<br>
> @@ -5186,6 +5186,7 @@ fs_visitor::lower_simd_width()<br>
>  <br>
>           assert(!inst-><wbr>writes_accumulator && !inst->mlen);<br>
>  <br>
> +         exec_node * const after_inst = inst->next;<br>
<br>
</span>I think code style should be:<br>
<br>
exec_node *const after_inst = inst->next<br></blockquote><div><br></div><div>Sure.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
not a big deal anyway...<br>
<div><div class="h5"><br>
>           for (unsigned i = 0; i < n; i++) {<br>
>              /* Emit a copy of the original instruction with the<br>
> lowered width.<br>
>               * If the EOT flag was set throw it away except for the<br>
> last<br>
> @@ -5193,7 +5194,7 @@ fs_visitor::lower_simd_width()<br>
>               */<br>
>              fs_inst split_inst = *inst;<br>
>              split_inst.exec_<wbr>size = lower_width;<br>
> -            split_inst.eot = inst->eot && i == n - 1;<br>
> +            split_inst.eot = inst->eot && i == 0;<br>
>  <br>
>              /* Select the correct channel enables for the i-th<br>
> group, then<br>
>               * transform the sources and destination and emit the<br>
> lowered<br>
> @@ -5205,11 +5206,11 @@ fs_visitor::lower_simd_width()<br>
>                 split_inst.<wbr>src[j] = emit_unzip(<a href="http://lbld.at" rel="noreferrer" target="_blank">lbld.at</a>(block, inst),<br>
> inst, j);<br>
>  <br>
>              split_inst.dst = emit_zip(<a href="http://lbld.at" rel="noreferrer" target="_blank">lbld.at</a>(block, inst),<br>
> -                             <wbr>         <a href="http://lbld.at" rel="noreferrer" target="_blank">lbld.at</a>(block, inst->next),<br>
> inst);<br>
> +                             <wbr>         <a href="http://lbld.at" rel="noreferrer" target="_blank">lbld.at</a>(block, after_inst),<br>
> inst);<br>
>              split_inst.size_<wbr>written =<br>
>                 split_inst.<wbr>dst.component_size(lower_<wbr>width) *<br>
> dst_size;<br>
>  <br>
> -            lbld.emit(split_<wbr>inst);<br>
> +            <a href="http://lbld.at" rel="noreferrer" target="_blank">lbld.at</a>(block, inst->next).emit(split_inst);<br>
<br>
</div></div>It was not immediately obvious to me that inst->next here is not the<br>
same as 'after_inst' due to the emit_zip() above, I am not sure if that<br>
deserves a comment though, I'll let you decide :)<br></blockquote><div><br></div><div>I've inserted a ~10 line comment. :)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
>           }<br>
>  <br>
>           inst->remove(block);<br>
</blockquote></div><br></div></div>