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

Tue Nov 3 05:28:58 PST 2015

Iago Toral <itoral at igalia.com> writes:

> 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
>
> Mmmm... not sure what could be going on with this, at first glance, it
> does not look like this test should be affected by this series. The test
> does not use any UBOs, does not really check what is written to the FBO,
> does not emit texture accesses... Also, there are other tests in
> piglit.spec.arb_fragment_layer_viewport that do very similar stuff
> (specially the our-of-range test) and those are passing fine.
>
Odd, I guess it may have been an intermittent failing test that just
happens to have failed during your run.  Mark, have you seen any of
these fail intermittently by any chance?

>>     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. ;)
>
> Unfortunately it seems that these don't break for me with the DEVID
> override, that's weird I guess, since they are compiler tests that I can
> fully run without INTEL_NO_HW set:
>
> $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest
> generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag pass 1.10 GL_ARB_shader_texture_lod
> Successfully compiled fragment shader
> generated_tests/spec/arb_shader_texture_lod/compiler/tex_grad-texture2DProj-2D-vec4.frag: 
> PIGLIT: {"result": "pass" }
>
> $ INTEL_DEVID_OVERRIDE=0x29A2 bin/glslparsertest_gles2
> tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag pass 1.00
> Successfully compiled fragment shader
> tests/spec/glsl-es-1.00/compiler/structure-and-array-operations/sampler-array-index.frag: 0:21(21): warning: sampler arrays indexed with non-constant expressions will be forbidden in GLSL 3.00 and later
> PIGLIT: {"result": "pass" }
>
Looking at the logs it actually seems to have been an assertion failure:

glslparsertest_gles2:
/mnt/space/jenkins/jobs/Leeroy/workspace/repos/mesa/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp:112:
void brw::fs_live_variables::setup_one_write(brw::block_data*, fs_inst*,
int, const fs_reg&): Assertion `var < num_vars' failed.

Did you test it on a debug build?

> I tried with most chipsets in between PCI_CHIP_I965_G (0x29A2) and
> PCI_CHIP_G41_G (0x2E32).
>
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20151103/6a666825/attachment.sig>