<html><head></head><body><div>On Tue, 2017-12-12 at 08:20 +0100, Iago Toral wrote:</div><blockquote type="cite"><div>On Mon, 2017-12-11 at 08:01 -0800, Jason Ekstrand wrote:</div><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 11, 2017 at 12:55 AM, Iago Toral <span dir="ltr"><<a href="mailto:itoral@igalia.com" target="_blank">itoral@igalia.com</a>></span> wrote:<br><blockquote type="cite">This didn't get any reviews yet. Any takers?<br>
<div class="HOEnZb"><div class="h5"><br>
On Fri, 2017-12-01 at 13:46 +0100, Iago Toral Quiroga wrote:<br>
> Otherwise loop unrolling will fail to see the actual cost of<br>
> the unrolling operations when the loop body contains 64-bit integer<br>
> instructions, and very specially when the divmod64 lowering applies,<br>
> since its lowering is quite expensive.<br>
><br>
> Without this change, some in-development CTS tests for int64<br>
> get stuck forever trying to register allocate a shader with<br>
> over 50K SSA values. The large number of SSA values is the result<br>
> of NIR first unrolling multiple seemingly simple loops that involve<br>
> int64 instructions, only to then lower these instructions to produce<br>
> a massive pile of code (due to the divmod64 lowering in the unrolled<br>
> instructions).<br>
><br>
> With this change, loop unrolling will see the loops with the int64<br>
> code already lowered and will realize that it is too expensive to<br>
> unroll.<br></div></div><br></blockquote><div><br></div><div>Hrm... I'm not quite sure what I think of this. I put it after nir_optimize because I wanted opt_algebraic to be able to work it's magic and hopefully remove a bunch of int64 ops before we lower them. In particular, we have optimizations to remove integer division and replace it with shifts. However, loop unrolling does need to happen before lower_indirect_derefs so that lower_indirect_derefs will do as little work as possible.</div><div><br></div><div>This is a bit of a pickle... I don't really want to add a third brw_nir_optimize call. It probably wouldn't be the end of the world but it does add compile time.</div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br></div><div>One crazy idea which I don't think I like would be to have a quick pass that walks the IR and sees if there are any 64-bit SSA values. If it does, we run brw_nir_optimize without loop unrolling then 64-bit lowering and then we go into the normal brw_nir_optimize.</div></div></div></div></blockquote><div><br></div><div>With the constraints you mention above, I am not sure that we have many more options... what if we always run opt_algebraic first followed by int64 lowering before the first nir_optimize? That would only add an extra opt_algebraic instead of a full nir_optimize. Would that be better than adding that 64-bit SSA scan pre-pass?</div></blockquote><div><br></div><div>We still need to make a decision for this, does my proposal sound better than than the other options on the table? If not I guess we should go with the 64-bit SSA scan pre-pass.</div><div><br></div><div>Iago</div><div><br></div><blockquote type="cite"><blockquote type="cite"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote type="cite"><div class="HOEnZb"><div class="h5">
> ---<br>
> src/intel/compiler/brw_nir.c | 8 ++++----<br>
> 1 file changed, 4 insertions(+), 4 deletions(-)<br>
><br>
> diff --git a/src/intel/compiler/brw_nir.c<br>
> b/src/intel/compiler/brw_nir.c<br>
> index 8f3f77f89a..ef12cdfff8 100644<br>
> --- a/src/intel/compiler/brw_nir.c<br>
> +++ b/src/intel/compiler/brw_nir.c<br>
> @@ -636,6 +636,10 @@ brw_preprocess_nir(const struct brw_compiler<br>
> *compiler, nir_shader *nir)<br>
> <br>
> OPT(nir_split_var_copies);<br>
> <br>
> + nir_lower_int64(nir, nir_lower_imul64 |<br>
> + nir_<wbr>lower_isign64 |<br>
> + nir_<wbr>lower_divmod64);<br>
> +<br>
> nir = brw_nir_optimize(nir, compiler, is_scalar);<br>
> <br>
> if (is_scalar) {<br>
> @@ -663,10 +667,6 @@ brw_preprocess_nir(const struct brw_compiler<br>
> *compiler, nir_shader *nir)<br>
> brw_nir_no_indirect_<wbr>mask(compiler, nir->info.stage);<br>
> nir_lower_indirect_derefs(<wbr>nir, indirect_mask);<br>
> <br>
> - nir_lower_int64(nir, nir_lower_imul64 |<br>
> - nir_<wbr>lower_isign64 |<br>
> - nir_<wbr>lower_divmod64);<br>
> -<br>
> /* Get rid of split copies */<br>
> nir = brw_nir_optimize(nir, compiler, is_scalar);<br>
> <br>
</div></div><div class="HOEnZb"><div class="h5">______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">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/<wbr>mailman/listinfo/mesa-dev</a><br>
</div></div><br></blockquote></div><br></div></div>
</blockquote></blockquote></body></html>