[Mesa-dev] [PATCH 3/4] i965/fs: Simplify texture destination fixups

[Francisco Jerez]

Jason Ekstrand <jason at jlekstrand.net> writes:

> On Tue, May 3, 2016 at 4:24 PM, Jason Ekstrand <jason at jlekstrand.net> wrote:
>
>>
>>
>> On Tue, May 3, 2016 at 4:18 PM, Kenneth Graunke <kenneth at whitecape.org>
>> wrote:
>>
>>> On Tuesday, May 3, 2016 3:00:26 PM PDT Jason Ekstrand wrote:
>>> > There are a few different fixups that we have to do for texture
>>> > destinations that re-arrange channels, fix hardware vs. API mismatches,
>>> or
>>> > just shrink the result to fit in the NIR destination.  These were all
>>> being
>>> > done in a somewhat haphazard manner.  This commit replaces all of the
>>> > shuffling with a single LOAD_PAYLOAD operation at the end and makes it
>>> much
>>> > easier to insert fixups between the texture instruction itself and the
>>> > LOAD_PAYLOAD.
>>> >
>>> > Shader-db results on Haswell:
>>> >
>>> >    total instructions in shared programs: 6227035 -> 6226669 (-0.01%)
>>> >    instructions in affected programs: 19119 -> 18753 (-1.91%)
>>> >    helped: 85
>>> >    HURT: 0
>>> >
>>> >    total cycles in shared programs: 56491626 -> 56476126 (-0.03%)
>>> >    cycles in affected programs: 672420 -> 656920 (-2.31%)
>>> >    helped: 92
>>> >    HURT: 42
>>> > ---
>>> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp | 32 ++++++++++
>>> +---------------------
>>> >  1 file changed, 11 insertions(+), 21 deletions(-)
>>> >
>>> > diff --git a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> b/src/mesa/drivers/
>>> dri/i965/brw_fs_nir.cpp
>>> > index ebc54ad..e0a76b4 100644
>>> > --- a/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> > +++ b/src/mesa/drivers/dri/i965/brw_fs_nir.cpp
>>> > @@ -3311,43 +3311,33 @@ fs_visitor::nir_emit_texture(const fs_builder
>>> &bld,
>>> nir_tex_instr *instr)
>>> >           emit_gen6_gather_wa(key_tex->gen6_gather_wa[texture], dst);
>>> >     }
>>> >
>>> > -   if (instr->op == nir_texop_query_levels) {
>>> > -      /* # levels is in .w */
>>> > -      dst = offset(dst, bld, 3);
>>> > -   }
>>> > +   fs_reg *nir_dest = ralloc_array(mem_ctx, fs_reg, dest_size);
>>>
>>> Why not just stack allocate this using the pessimal size?
>>>
>>>     fs_reg nir_dest[4];
>>>
>>
>> Good point!  Fixed locally.
>>
>
> Never mind.  It's because bld.LOAD_PAYLOAD doesn't make a copy of the
> array, it just copies the pointer into the fs_inst.
>
fs_builder::LOAD_PAYLOAD just passes the pointer to the fs_inst
constructor which does the copy itself when you use the variable-source
variant -- I fixed that a while ago precisely in order to be able to
allocate source arrays in the stack. ;)

>
>> > +   for (unsigned i = 0; i < dest_size; i++)
>>> > +      nir_dest[i] = offset(dst, bld, i);
>>> >
>>> >     bool is_cube_array = instr->sampler_dim == GLSL_SAMPLER_DIM_CUBE &&
>>> >                          instr->is_array;
>>> >
>>> > -   /* fixup #layers for cube map arrays */
>>> > -   if (instr->op == nir_texop_txs && (devinfo->gen < 7 ||
>>> is_cube_array)) {
>>> > +   if (instr->op == nir_texop_query_levels) {
>>> > +      /* # levels is in .w */
>>> > +      nir_dest[0] = offset(dst, bld, 3);
>>> > +   } else if (instr->op == nir_texop_txs && dest_size >= 3 &&
>>> > +              (devinfo->gen < 7 || is_cube_array)) {
>>> >        fs_reg depth = offset(dst, bld, 2);
>>> >        fs_reg fixed_depth = vgrf(glsl_type::int_type);
>>> >
>>> >        if (is_cube_array) {
>>> > +         /* fixup #layers for cube map arrays */
>>> >           bld.emit(SHADER_OPCODE_INT_QUOTIENT, fixed_depth, depth,
>>> brw_imm_d(6));
>>> >        } else if (devinfo->gen < 7) {
>>> >           /* Gen4-6 return 0 instead of 1 for single layer surfaces. */
>>> >           bld.emit_minmax(fixed_depth, depth, brw_imm_d(1),
>>> BRW_CONDITIONAL_GE);
>>> >        }
>>> >
>>> > -      fs_reg *fixed_payload = ralloc_array(mem_ctx, fs_reg, inst-
>>> >regs_written);
>>> > -      int components = inst->regs_written / (inst->exec_size / 8);
>>> > -      for (int i = 0; i < components; i++) {
>>> > -         if (i == 2) {
>>> > -            fixed_payload[i] = fixed_depth;
>>> > -         } else {
>>> > -            fixed_payload[i] = offset(dst, bld, i);
>>> > -         }
>>> > -      }
>>> > -      bld.LOAD_PAYLOAD(dst, fixed_payload, components, 0);
>>> > +      nir_dest[2] = fixed_depth;
>>> >     }
>>> >
>>> > -   fs_reg nir_dest = get_nir_dest(instr->dest);
>>> > -   nir_dest.type = dst.type;
>>> > -   emit_percomp(bld, fs_inst(BRW_OPCODE_MOV, bld.dispatch_width(),
>>> > -                             nir_dest, dst),
>>> > -                (1 << dest_size) - 1);
>>> > +   bld.LOAD_PAYLOAD(get_nir_dest(instr->dest), nir_dest, dest_size, 0);
>>> >  }
>>> >
>>> >  void
>>> >
>>>
>>>
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160503/08755d73/attachment-0001.sig>