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)
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

index e0efa0e..2137de5 100644 (file)
@@ -1475,6 +1475,21 @@ std::vector<std::string> getProgramInterfaceResourceList (const ProgramInterface
        return resources;
 }
 
+/**
+ * Name of the dummy uniform added by generateProgramInterfaceProgramSources
+ *
+ * A uniform named "dummyZero" is added by
+ * generateProgramInterfaceProgramSources.  It is used in expressions to
+ * prevent various program resources from being eliminated by the GLSL
+ * compiler's optimizer.
+ *
+ * \sa deqp::gles31::Functional::ProgramInterfaceDefinition::generateProgramInterfaceProgramSources
+ */
+const char* getDummyZeroUniformName()
+{
+       return "dummyZero";
+}
+
 glu::ProgramSources generateProgramInterfaceProgramSources (const ProgramInterfaceDefinition::Program* program)
 {
        glu::ProgramSources sources;
@@ -1513,9 +1528,10 @@ glu::ProgramSources generateProgramInterfaceProgramSources (const ProgramInterfa
 
                // Use inputs and outputs so that they won't be removed by the optimizer
 
-               usageBuf <<     "highp vec4 readInputs()\n"
+               usageBuf <<     "highp uniform vec4 " << getDummyZeroUniformName() << "; // Default value is vec4(0.0).\n"
+                                       "highp vec4 readInputs()\n"
                                        "{\n"
-                                       "       highp vec4 retValue = vec4(0.0);\n";
+                                       "       highp vec4 retValue = " << getDummyZeroUniformName() << ";\n";
 
                // User-defined inputs
 
index efd9e03..d4a22fb 100644 (file)
@@ -174,6 +174,7 @@ bool                                                                                                shaderContainsIOBlocks                                          (const ProgramInterfaceDefinition::S
 glu::ShaderType                                                                                getProgramTransformFeedbackStage                        (const ProgramInterfaceDefinition::Program* program);
 std::vector<std::string>                                                       getProgramInterfaceResourceList                         (const ProgramInterfaceDefinition::Program* program, ProgramInterface interface);
 std::vector<std::string>                                                       getProgramInterfaceBlockMemberResourceList      (const glu::InterfaceBlock& interfaceBlock);
+const char*                                                                                    getDummyZeroUniformName                                         ();
 glu::ProgramSources                                                                    generateProgramInterfaceProgramSources          (const ProgramInterfaceDefinition::Program* program);
 bool                                                                                           findProgramVariablePathByPathName                       (std::vector<ProgramInterfaceDefinition::VariablePathComponent>& typePath, const ProgramInterfaceDefinition::Program* program, const std::string& pathName, const ProgramInterfaceDefinition::VariableSearchFilter& filter);
 void                                                                                           generateVariableTypeResourceNames                       (std::vector<std::string>& resources, const std::string& name, const glu::VarType& type, deUint32 resourceNameGenerationFlags);
index ede1c0b..2b06602 100644 (file)
@@ -1026,7 +1026,13 @@ bool ResourceListTestCase::verifyResourceList (const std::vector<std::string>& r
        m_testCtx.getLog() << tcu::TestLog::Message << "GL returned resources:" << tcu::TestLog::EndMessage;
 
        for (int ndx = 0; ndx < (int)resourceList.size(); ++ndx)
-               m_testCtx.getLog() << tcu::TestLog::Message << "\t" << ndx << ": " << resourceList[ndx] << tcu::TestLog::EndMessage;
+       {
+               // dummyZero is a uniform that may be added by
+               // generateProgramInterfaceProgramSources.  Omit it here to avoid
+               // confusion about the output.
+               if (resourceList[ndx] != getDummyZeroUniformName())
+                       m_testCtx.getLog() << tcu::TestLog::Message << "\t" << ndx << ": " << resourceList[ndx] << tcu::TestLog::EndMessage;
+       }
 
        m_testCtx.getLog() << tcu::TestLog::Message << "Expected list of resources:" << tcu::TestLog::EndMessage;
 
@@ -1048,8 +1054,11 @@ bool ResourceListTestCase::verifyResourceList (const std::vector<std::string>& r
        {
                if (!de::contains(expectedResources.begin(), expectedResources.end(), resourceList[ndx]))
                {
-                       // Ignore all builtin variables, mismatch causes errors otherwise
-                       if (deStringBeginsWith(resourceList[ndx].c_str(), "gl_") == DE_FALSE)
+                       // Ignore all builtin variables or the variable dummyZero,
+                       // mismatch causes errors otherwise.  dummyZero is a uniform that
+                       // may be added by generateProgramInterfaceProgramSources.
+                       if (deStringBeginsWith(resourceList[ndx].c_str(), "gl_") == DE_FALSE &&
+                               resourceList[ndx] != getDummyZeroUniformName())
                        {
                                m_testCtx.getLog() << tcu::TestLog::Message << "Error, resource list contains unexpected resource name " << resourceList[ndx] << tcu::TestLog::EndMessage;
                                error = true;