[Mesa-dev] [PATCH 00/10] i965: always mark used surfaces in the visitors

[Iago Toral]

Hi Curro,

On Fri, 2015-10-30 at 16:19 +0200, Francisco Jerez wrote:
> Iago Toral Quiroga <itoral at igalia.com> writes:
> 
> > Right now some opcodes that only use constant surface indexing mark them as
> > used in the generator while others do it in the visitor. When the opcode can
> > handle both direct and indirect surface indexing then some opcodes handle
> > only the constant part in the generator and leave the indirect case to the
> > caller. It is all very inconsistent and leads to confusion, since one has to
> > go and look into the generator code in each case to check if it marks surfaces
> > as used or not, and in which cases.
> >
> > when I was working on SSBOs I was tempted to try and fix this but then I
> > forgot. Jordan bumped into this recently too when comparing visitor
> > code paths for similar opcodes (ubos and ssbos) that need to handle this
> > differently because they use different generator opcodes.
> >
> > Since the generator opcodes never handle marking of indirect surfaces, just
> > leave surface marking to the caller completely, since callers always have
> > all the information needed for this. It also makes things more consistent
> > and clear for everyone: marking surfaces as used is always on the side
> > of the visitor, never the generator.
> >
> > No piglit regressions observed in my IVB laptop. Would be nice to have
> > someone giving this a try with Jenkins though, to make sure I did not miss
> > anything in paths specific to other gens.
> 
> Jenkins seems to be mostly happy about the series except for three
> apparent regressions:
> 
>     piglit.spec.arb_fragment_layer_viewport.layer-gs-writes-in-range.bdwm64
>     piglit.spec.arb_shader_texture_lod.compiler.tex_grad-texture2dproj-2d-vec4.frag.g965m64
>     piglit.spec.glsl-es-1_00.compiler.structure-and-array-operations.sampler-array-index.frag.g965m64
> 
> The latter two die with a crash so you may be able to look into them
> even if you don't have the original i965 by using INTEL_DEVID_OVERRIDE. ;)

thanks for testing and all the review comments. I'll look into these!

Iago

> >
> > Iago Toral Quiroga (10):
> >   i965/fs: Do not mark direct used surfaces in
> >     VARYING_PULL_CONSTANT_LOAD
> >   i965/fs: Do not mark used direct surfaces in
> >     UNIFORM_PULL_CONSTANT_LOAD
> >   i965/fs: Do not mark used direct surfaces in the generator for texture
> >     opcodes
> >   i965/vec4: Do not mark used direct surfaces in
> >     VS_OPCODE_PULL_CONSTANT_LOAD
> >   i965/vec4: Do not mark used direct surfaces in the generator for
> >     texture opcodes
> >   i965/vec4: Do not mark used surfaces in SHADER_OPCODE_SHADER_TIME_ADD
> >   i965/fs: Do not mark used surfaces in SHADER_OPCODE_SHADER_TIME_ADD
> >   i965/vec4: Do not mark used surfaces in VS_OPCODE_GET_BUFFER_SIZE
> >   i965/fs: Do not mark used surfaces in FS_OPCODE_GET_BUFFER_SIZE
> >   i965/fs: Do not mark used surfaces in
> >     FS_OPCODE_FB_WRITE/FS_OPCODE_REP_FB_WRITE
> >
> >  src/mesa/drivers/dri/i965/brw_fs.cpp             | 12 ++++++-
> >  src/mesa/drivers/dri/i965/brw_fs.h               |  3 +-
> >  src/mesa/drivers/dri/i965/brw_fs_generator.cpp   | 31 -----------------
> >  src/mesa/drivers/dri/i965/brw_fs_nir.cpp         | 35 +++++++++++++------
> >  src/mesa/drivers/dri/i965/brw_fs_visitor.cpp     | 24 ++++++++-----
> >  src/mesa/drivers/dri/i965/brw_vec4.cpp           |  3 ++
> >  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 19 ----------
> >  src/mesa/drivers/dri/i965/brw_vec4_nir.cpp       | 44 +++++++++++++++---------
> >  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp   |  7 ++--
> >  9 files changed, 87 insertions(+), 91 deletions(-)
> >
> > -- 
> > 1.9.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev