Try harder to defeat GLSL compiler dead-code optimizations
authorIan Romanick <ian.d.romanick@intel.com>
Thu, 10 Nov 2016 01:29:01 +0000 (17:29 -0800)
committerPyry Haulos <phaulos@google.com>
Tue, 11 Apr 2017 06:47:52 +0000 (06:47 +0000)
commit337d8f30c861d07fcc50b575b332efac35a5f094
tree3636e547f6057aad85feaef253a3ba7fd4bbe176
parent6bc3c7a63456fdf267b65a5b80a003741a3aee4c
Try harder to defeat GLSL compiler dead-code optimizations

A number of CTS tests generate shaders like like:

    #version 310 es

    buffer TargetInterface
    {
            highp float target;
    };

    highp vec4 readInputs()
    {
            highp vec4 retValue = vec4(0.0);
            retValue += vec4(float(target));
            return retValue;
    }

    void writeOutputs(in highp vec4 dummyValue)
    {
            target = float(dummyValue.y);
    }

    void main()
    {
            writeOutputs(readInputs());
    }

After various common optimizations this becomes:

    buffer TargetInterface
    {
            highp float target;
    };

    void main()
    {
            target = target;
    }

In the absence of memoryBarrier() or qualifiers on the buffer, there is
no guarantee about the order of writes to the buffer.  Since this write
is not guaranteed to be visible either on the GPU or the CPU, we
eliminate it.  Since there is no access to target in the shader, we
report GL_REFERENCED_BY_FRAGMENT_SHADER = GL_FALSE.  The tests expect
GL_TRUE.

The vectored versions of this test swizzle the value read from the
buffer before writing it back.  These writes are not eliminated.

Adding a uniform instead of a literal constant also prevents the reads
and writes of the SSBO from being eliminated.

v2: Ignore the uniform named "zero" in
ResourceListTestCase::verifyResourceList.  The alternative was to add
zero to the resource list, but that required making small changes
(mostly removing const) from over a dozen places in the code.  This
slightly hacky, but localized, change seemed better.

v3: Various coding standards fixes suggested by Alexander Galazin and
Pyry.  Add getDummyZeroUniformName to query name of the zero uniform and
a lot more documentation.  Both suggested by Pyry.

The following tests are affected:

- dEQP-GLES31.functional.program_interface_query.buffer_variable.random.6
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.compute.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.separable_fragment.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.vertex_fragment_only_fragment.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.vertex_fragment.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.vertex_geo_fragment_only_fragment.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.vertex_geo_fragment.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_fragment_only_fragment.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_fragment.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_geo_fragment_only_fragment.unnamed_block.float
- dEQP-GLES31.functional.program_interface_query.buffer_variable.referenced_by.vertex_tess_geo_fragment.unnamed_block.float

Change-Id: I867ad32476269ac1272c09672be0a6d6fe37e31e
Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
Bugzilla: https://cvs.khronos.org/bugzilla/show_bug.cgi?id=16114
Google bug: 37228062
modules/gles31/functional/es31fProgramInterfaceDefinitionUtil.cpp
modules/gles31/functional/es31fProgramInterfaceDefinitionUtil.hpp
modules/gles31/functional/es31fProgramInterfaceQueryTests.cpp