<p dir="ltr"><br>
On Apr 23, 2016 4:39 PM, "Kenneth Graunke" <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> wrote:<br>
><br>
> Often, we don't need a full 4 channels worth of data from the sampler.<br>
> For example, depth comparisons and red textures only return one value.<br>
> To handle this, the sampler message header contains a mask which can<br>
> be used to disable channels, and reduce the message length (in SIMD16<br>
> mode on all hardware, and SIMD8 mode on Broadwell and later).<br>
><br>
> We've never used it before, since it required setting up a message<br>
> header.  This meant trading a smaller response length for a larger<br>
> message length and additional MOVs to set it up.<br>
><br>
> However, Skylake introduces a terrific new feature: for headerless<br>
> messages, you can simply reduce the response length, and it makes<br>
> the implicit header contain an appropriate mask.  So to read only<br>
> RG, you would simply set the message length to 2 or 4 (SIMD8/16).<br>
><br>
> This means we can finally take advantage of this at no cost.<br>
><br>
> total instructions in shared programs: 9091831 -> 9073067 (-0.21%)<br>
> instructions in affected programs: 191370 -> 172606 (-9.81%)<br>
> helped: 2609<br>
> HURT: 0<br>
><br>
> total cycles in shared programs: 70868114 -> 68454752 (-3.41%)<br>
> cycles in affected programs: 35841154 -> 33427792 (-6.73%)<br>
> helped: 16357<br>
> HURT: 8188<br>
><br>
> total spills in shared programs: 3492 -> 1707 (-51.12%)<br>
> spills in affected programs: 2749 -> 964 (-64.93%)<br>
> helped: 74<br>
> HURT: 0<br>
><br>
> total fills in shared programs: 4266 -> 2647 (-37.95%)<br>
> fills in affected programs: 3029 -> 1410 (-53.45%)<br>
> helped: 74<br>
> HURT: 0<br>
><br>
> LOST:   1<br>
> GAINED: 143<br>
><br>
> Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>><br>
> ---<br>
>  src/mesa/drivers/dri/i965/brw_fs.cpp         | 10 ++++++++++<br>
>  src/mesa/drivers/dri/i965/brw_fs.h           |  3 ++-<br>
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp     | 15 +++++++++++++--<br>
>  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp |  5 +++--<br>
>  4 files changed, 28 insertions(+), 5 deletions(-)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
> index 81ec18d..78f7d40 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp<br>
> @@ -4048,6 +4048,16 @@ lower_sampler_logical_send_gen7(const fs_builder &bld, fs_inst *inst, opcode op,<br>
>        header_size = 1;<br>
>        sources[0] = fs_reg();<br>
>        length++;<br>
> +<br>
> +      /* If we're requesting fewer than four channels worth of response,<br>
> +       * and we have an explicit header, we need to set up the sampler<br>
> +       * writemask.  It's reversed from normal: 1 means "don't write".<br>
> +       */<br>
> +      if (inst->regs_written != 4 * reg_width) {<br>
> +         assert((inst->regs_written % reg_width) == 0);<br>
> +         unsigned mask = ~((1 << (inst->regs_written / reg_width)) - 1) & 0xf;<br>
> +         inst->offset |= mask << 12;<br>
> +      }<br>
>     }<br>
><br>
>     if (shadow_c.file != BAD_FILE) {<br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.h b/src/mesa/drivers/dri/i965/brw_fs.h<br>
> index bcd2e3e..a5c3297 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs.h<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs.h<br>
> @@ -213,7 +213,8 @@ public:<br>
>                       uint32_t surface,<br>
>                       fs_reg surface_reg,<br>
>                       uint32_t sampler,<br>
> -                     fs_reg sampler_reg);<br>
> +                     fs_reg sampler_reg,<br>
> +                     unsigned return_channels);<br>
>     fs_reg emit_mcs_fetch(const fs_reg &coordinate, unsigned components,<br>
>                           const fs_reg &sampler);<br>
>     void emit_gen6_gather_wa(uint8_t wa, fs_reg dst);<br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> index 43d3745..3d7013f 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp<br>
> @@ -3224,14 +3224,25 @@ fs_visitor::nir_emit_texture(const fs_builder &bld, nir_tex_instr *instr)<br>
>        unreachable("unknown texture opcode");<br>
>     }<br>
><br>
> +   unsigned num_components = nir_tex_instr_dest_size(instr);<br>
> +<br>
> +   if (instr->dest.is_ssa) {<br>
> +      uint8_t write_mask = nir_ssa_def_components_read(&instr->dest.ssa);<br>
> +      assert(write_mask != 0); /* dead code should have been eliminated */<br>
> +      num_components = _mesa_fls(write_mask);<br>
> +   }<br>
> +<br>
> +   const bool can_reduce_return_length = devinfo->gen >= 9 &&<br>
> +      instr->op != nir_texop_tg4 && instr->op != nir_texop_query_levels;</p>
<p dir="ltr">I think I'd prefer this go in fs_visitor::emit_texture;  that's where we do most of the gen check type things.  Then again, what I'd really like is to roll emit_texture into nir_emit_texture and be done with the distinction once and for all.  If you don't want to go to all that work, I won't blame you; feel free to leave it as-is if you'd like.  I can come along and merge them later.</p>
<p dir="ltr">Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>></p>
<p dir="ltr">>     emit_texture(op, dest_type, coordinate, instr->coord_components,<br>
>                  shadow_comparitor, lod, lod2, lod_components, sample_index,<br>
>                  tex_offset, mcs, gather_component, is_cube_array,<br>
> -                texture, texture_reg, sampler, sampler_reg);<br>
> +                texture, texture_reg, sampler, sampler_reg,<br>
> +                can_reduce_return_length ? num_components : 4);<br>
><br>
>     fs_reg dest = get_nir_dest(instr->dest);<br>
>     dest.type = this->result.type;<br>
> -   unsigned num_components = nir_tex_instr_dest_size(instr);<br>
>     emit_percomp(bld, fs_inst(BRW_OPCODE_MOV, bld.dispatch_width(),<br>
>                               dest, this->result),<br>
>                  (1 << num_components) - 1);<br>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
> index daabf70..da29f0b 100644<br>
> --- a/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
> +++ b/src/mesa/drivers/dri/i965/brw_fs_visitor.cpp<br>
> @@ -115,7 +115,8 @@ fs_visitor::emit_texture(ir_texture_opcode op,<br>
>                           uint32_t surface,<br>
>                           fs_reg surface_reg,<br>
>                           uint32_t sampler,<br>
> -                         fs_reg sampler_reg)<br>
> +                         fs_reg sampler_reg,<br>
> +                         unsigned return_channels)<br>
>  {<br>
>     fs_inst *inst = NULL;<br>
><br>
> @@ -204,7 +205,7 @@ fs_visitor::emit_texture(ir_texture_opcode op,<br>
>     }<br>
><br>
>     inst = bld.emit(opcode, dst, srcs, ARRAY_SIZE(srcs));<br>
> -   inst->regs_written = 4 * dispatch_width / 8;<br>
> +   inst->regs_written = return_channels * dispatch_width / 8;<br>
><br>
>     if (shadow_c.file != BAD_FILE)<br>
>        inst->shadow_compare = true;<br>
> --<br>
> 2.8.0<br>
><br>
> _______________________________________________<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">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</p>