<div dir="ltr"> <div class="gmail_extra"> <div class="gmail_quote">On Tue, May 3, 2016 at 4:54 PM, Francisco Jerez <<a href="mailto:currojerez@riseup.net" target="_blank">currojerez@riseup.net</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> writes: > On Tue, May 3, 2016 at 4:24 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote: > >> >> >> On Tue, May 3, 2016 at 4:18 PM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> >> 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. > </div></div>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. ;) </blockquote><div> </div><div>Yup. I made the change locally (after double and tripple-checking what curro just said) and verified with piglit. </div><div>--Jason </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div class="h5"> > >> > + 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 >>> > >>> >>> >> </div></div>> _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div></div>