<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Oct 28, 2016 at 4:07 PM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Oct 28, 2016 at 2:53 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> The address immediate field is only 9 bits and, since the value is in<br>
> bytes, the highest GRF we can point to with it is g15. This makes it<br>
> pretty close to useless for MOV_INDIRECT. There were already piles of<br>
> restrictions preventing us from using it prior to Broadwell, so let's get<br>
> rid of the gen8+ code path entirely.<br>
><br>
> Signed-off-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br>
> Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=97779" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/<wbr>show_bug.cgi?id=97779</a><br>
> Cc: "12.0 13.0" <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.<wbr>freedesktop.org</a>><br>
> ---<br>
> src/mesa/drivers/dri/i965/brw_<wbr>fs_generator.cpp | 55 +++++++++++++-------------<br>
> 1 file changed, 28 insertions(+), 27 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/<wbr>brw_fs_generator.cpp b/src/mesa/drivers/dri/i965/<wbr>brw_fs_generator.cpp<br>
> index d25d26a..7130bf5 100644<br>
> --- a/src/mesa/drivers/dri/i965/<wbr>brw_fs_generator.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/<wbr>brw_fs_generator.cpp<br>
> @@ -386,33 +386,34 @@ fs_generator::generate_mov_<wbr>indirect(fs_inst *inst,<br>
> retype(spread(indirect_byte_<wbr>offset, 2), BRW_REGISTER_TYPE_UW);<br>
><br>
> struct brw_reg ind_src;<br>
> - if (devinfo->gen < 8) {<br>
> - /* From the Haswell PRM section "Register Region Restrictions":<br>
> - *<br>
> - * "The lower bits of the AddressImmediate must not overflow to<br>
> - * change the register address. The lower 5 bits of Address<br>
> - * Immediate when added to lower 5 bits of address register gives<br>
> - * the sub-register offset. The upper bits of Address Immediate<br>
> - * when added to upper bits of address register gives the register<br>
> - * address. Any overflow from sub-register offset is dropped."<br>
> - *<br>
> - * This restriction is only listed in the Haswell PRM but emperical<br>
> - * testing indicates that it applies on all older generations and is<br>
> - * lifted on Broadwell.<br>
> - *<br>
> - * Since the indirect may cause us to cross a register boundary, this<br>
> - * makes the base offset almost useless. We could try and do<br>
> - * something clever where we use a actual base offset if<br>
> - * base_offset % 32 == 0 but that would mean we were generating<br>
> - * different code depending on the base offset. Instead, for the<br>
> - * sake of consistency, we'll just do the add ourselves.<br>
> - */<br>
> - brw_ADD(p, addr, indirect_byte_offset, brw_imm_uw(imm_byte_offset));<br>
> - ind_src = brw_VxH_indirect(0, 0);<br>
> - } else {<br>
> - brw_MOV(p, addr, indirect_byte_offset);<br>
> - ind_src = brw_VxH_indirect(0, imm_byte_offset);<br>
> - }<br>
> +<br>
> + /* There are a number of reasons why we don't use the base offset here.<br>
> + * One reason is that the field is only 9 bits which means we can only<br>
> + * use it on the first 16 GRFs. Also, from the Haswell PRM section<br>
<br>
</div></div>s/on/to access/<span class=""><br></span></blockquote><div><br></div><div>Yeah, that's better<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> + * "Register Region Restrictions":<br>
> + *<br>
> + * "The lower bits of the AddressImmediate must not overflow to<br>
> + * change the register address. The lower 5 bits of Address<br>
> + * Immediate when added to lower 5 bits of address register gives<br>
> + * the sub-register offset. The upper bits of Address Immediate<br>
> + * when added to upper bits of address register gives the register<br>
> + * address. Any overflow from sub-register offset is dropped."<br>
> + *<br>
> + * Since the indirect may cause us to cross a register boundary, this<br>
> + * makes the base offset almost useless. We could try and do something<br>
> + * clever where we use a actual base offset if base_offset % 32 == 0 but<br>
> + * that would mean we were generating different code depending on the<br>
> + * base offset. Instead, for the sake of consistency, we'll just do the<br>
> + * add ourselves. This restriction is only listed in the Haswell PRM<br>
> + * but emperical testing indicates that it applies on all older<br>
<br>
</span>empirical<br></blockquote><div><br></div><div>Thanks<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="">
> + * generations and is lifted on Broadwell.<br>
> + *<br>
> + * In the end, while base_offset is nice to look at in the generated<br>
> + * code, using it saves us 0 instructions and would require quite a bit<br>
> + * of case-by-case work. It's just not worth it.<br>
> + */<br>
> + brw_ADD(p, addr, indirect_byte_offset, brw_imm_uw(imm_byte_offset));<br>
> + ind_src = brw_VxH_indirect(0, 0);<br>
<br>
</span>I think you can move the declaration of ind_src here as well.<br></blockquote><div><br></div><div>Done.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Reviewed-by: Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>><br>
</blockquote></div><br></div><div class="gmail_extra">Thanks!<br></div></div>