Vulkan: Save linked ProgramExecutable data

src/libANGLE/Program.cpp

@@ -737,6 +737,7 @@ void LoadInterfaceBlock(BinaryInputStream *stream, InterfaceBlock *block)
 // Saves the linking context for later use in resolveLink().
 struct Program::LinkingState
 {
+    std::shared_ptr<ProgramExecutable> linkedExecutable;
     std::unique_ptr<ProgramLinkedResources> resources;
     egl::BlobCache::Key programHash;
     std::unique_ptr<rx::LinkEvent> linkEvent;
@@ -1104,7 +1105,6 @@ ProgramState::ProgramState()
 ProgramState::~ProgramState()
 {
     ASSERT(!hasAttachedShader());
-    SafeDelete(mExecutable);
 }
 
 const std::string &ProgramState::getLabel()
@@ -1408,26 +1408,40 @@ angle::Result Program::linkMergedVaryings(const Context *context,
     return angle::Result::Continue;
 }
 
+angle::Result Program::link(const Context *context)
+{
+    angle::Result result = linkImpl(context);
+
+    // Avoid having two ProgramExecutables if the link failed and the Program had successfully
+    // linked previously.
+    if (mLinkingState && mLinkingState->linkedExecutable)
+    {
+        mState.mExecutable = mLinkingState->linkedExecutable;
+    }
+
+    return result;
+}
+
 // The attached shaders are checked for linking errors by matching up their variables.
 // Uniform, input and output variables get collected.
 // The code gets compiled into binaries.
-angle::Result Program::link(const Context *context)
+angle::Result Program::linkImpl(const Context *context)
 {
     ASSERT(mLinkResolved);
+    // Don't make any local variables pointing to anything within the ProgramExecutable, since
+    // unlink() could make a new ProgramExecutable making any references/pointers invalid.
     const auto &data = context->getState();
-    InfoLog &infoLog = mState.mExecutable->getInfoLog();
-
     auto *platform   = ANGLEPlatformCurrent();
     double startTime = platform->currentTime(platform);
 
     // Unlink the program, but do not clear the validation-related caching yet, since we can still
-    // use the previously linked program if linking the shaders fails
+    // use the previously linked program if linking the shaders fails.
     mLinked = false;
 
-    infoLog.reset();
+    mState.mExecutable->getInfoLog().reset();
 
     // Validate we have properly attached shaders before checking the cache.
-    if (!linkValidateShaders(infoLog))
+    if (!linkValidateShaders(mState.mExecutable->getInfoLog()))
     {
         return angle::Result::Continue;
     }
@@ -1455,6 +1469,7 @@ angle::Result Program::link(const Context *context)
 
     // Cache load failed, fall through to normal linking.
     unlink();
+    InfoLog &infoLog = mState.mExecutable->getInfoLog();
 
     // Re-link shaders after the unlink call.
     bool result = linkValidateShaders(infoLog);
@@ -1605,6 +1620,14 @@ angle::Result Program::link(const Context *context)
     mState.updateProgramInterfaceInputs();
     mState.updateProgramInterfaceOutputs();
 
+    // Linking has succeeded, so we need to save some information that may get overwritten by a
+    // later linkProgram() that could fail.
+    if (mState.mSeparable)
+    {
+        mState.mExecutable->saveLinkedStateInfo();
+        mLinkingState->linkedExecutable = mState.mExecutable;
+    }
+
     return angle::Result::Continue;
 }
 
@@ -1805,6 +1828,14 @@ void ProgramState::updateProgramInterfaceOutputs()
 // Returns the program object to an unlinked state, before re-linking, or at destruction
 void Program::unlink()
 {
+    if (mLinkingState && mLinkingState->linkedExecutable)
+    {
+        // The new ProgramExecutable that we'll attempt to link with needs to start from a copy of
+        // the last successfully linked ProgramExecutable, so we don't lose any state information.
+        mState.mExecutable.reset(new ProgramExecutable(*mLinkingState->linkedExecutable));
+    }
+    mState.mExecutable->reset();
+
     mState.mUniformLocations.clear();
     mState.mBufferVariables.clear();
     mState.mActiveUniformBlockBindings.reset();
@@ -1830,8 +1861,6 @@ void Program::unlink()
     mValidated = false;
 
     mLinked = false;
-
-    mState.mExecutable->reset();
 }
 
 angle::Result Program::loadBinary(const Context *context,
@@ -1839,9 +1868,9 @@ angle::Result Program::loadBinary(const Context *context,
                                   const void *binary,
                                   GLsizei length)
 {
-    InfoLog &infoLog = mState.mExecutable->getInfoLog();
     ASSERT(mLinkResolved);
     unlink();
+    InfoLog &infoLog = mState.mExecutable->getInfoLog();
 
 #if ANGLE_PROGRAM_BINARY_LOAD != ANGLE_ENABLED
     return angle::Result::Continue;
@@ -3017,7 +3046,7 @@ void Program::validate(const Caps &caps)
 
 bool Program::validateSamplersImpl(InfoLog *infoLog, const Caps &caps)
 {
-    const ProgramExecutable *executable = mState.mExecutable;
+    const ProgramExecutable *executable = mState.mExecutable.get();
     ASSERT(mLinkResolved);
 
     // if any two active samplers in a program are of different types, but refer to the same

src/libANGLE/Program.h

@@ -414,7 +414,7 @@ class ProgramState final : angle::NonCopyable
     // uniforms in GLES3.1+. It is used to pre-set the location of uniforms.
     ProgramAliasedBindings mUniformLocationBindings;
 
-    ProgramExecutable *mExecutable;
+    std::shared_ptr<ProgramExecutable> mExecutable;
 };
 
 struct ProgramVaryingRef
@@ -849,6 +849,8 @@ class Program final : angle::NonCopyable, public LabeledObject
     void unlink();
     void deleteSelf(const Context *context);
 
+    angle::Result linkImpl(const Context *context);
+
     bool linkValidateShaders(InfoLog &infoLog);
     bool linkAttributes(const Context *context, InfoLog &infoLog);
     bool linkInterfaceBlocks(const Caps &caps,

src/libANGLE/ProgramExecutable.cpp

@@ -30,8 +30,40 @@ ProgramExecutable::ProgramExecutable()
       mSamplerUniformRange(0, 0),
       mImageUniformRange(0, 0)
 {
-    mActiveSamplerTypes.fill(TextureType::InvalidEnum);
-    mActiveSamplerFormats.fill(SamplerFormat::InvalidEnum);
+    reset();
+}
+
+ProgramExecutable::ProgramExecutable(const ProgramExecutable &other)
+    : mProgramState(other.mProgramState),
+      mProgramPipelineState(other.mProgramPipelineState),
+      mLinkedGraphicsShaderStages(other.mLinkedGraphicsShaderStages),
+      mLinkedComputeShaderStages(other.mLinkedComputeShaderStages),
+      mActiveAttribLocationsMask(other.mActiveAttribLocationsMask),
+      mMaxActiveAttribLocation(other.mMaxActiveAttribLocation),
+      mAttributesTypeMask(other.mAttributesTypeMask),
+      mAttributesMask(other.mAttributesMask),
+      mActiveSamplersMask(other.mActiveSamplersMask),
+      mActiveSamplerRefCounts(other.mActiveSamplerRefCounts),
+      mActiveSamplerTypes(other.mActiveSamplerTypes),
+      mActiveSamplerFormats(other.mActiveSamplerFormats),
+      mActiveSamplerShaderBits(other.mActiveSamplerShaderBits),
+      mActiveImagesMask(other.mActiveImagesMask),
+      mActiveImageShaderBits(other.mActiveImageShaderBits),
+      mCanDrawWith(other.mCanDrawWith),
+      mOutputVariables(other.mOutputVariables),
+      mOutputLocations(other.mOutputLocations),
+      mProgramInputs(other.mProgramInputs),
+      mLinkedTransformFeedbackVaryings(other.mLinkedTransformFeedbackVaryings),
+      mTransformFeedbackStrides(other.mTransformFeedbackStrides),
+      mTransformFeedbackBufferMode(other.mTransformFeedbackBufferMode),
+      mUniforms(other.mUniforms),
+      mSamplerUniformRange(other.mSamplerUniformRange),
+      mUniformBlocks(other.mUniformBlocks),
+      mAtomicCounterBuffers(other.mAtomicCounterBuffers),
+      mImageUniformRange(other.mImageUniformRange),
+      mShaderStorageBlocks(other.mShaderStorageBlocks)
+{
+    reset();
 }
 
 ProgramExecutable::~ProgramExecutable() = default;
@@ -477,4 +509,19 @@ void ProgramExecutable::updateCanDrawWith()
         (hasLinkedShaderStage(ShaderType::Vertex) && hasLinkedShaderStage(ShaderType::Fragment));
 }
 
+void ProgramExecutable::saveLinkedStateInfo()
+{
+    // Only a Program's linked data needs to be saved, not a ProgramPipeline's
+    ASSERT(mProgramState);
+
+    for (ShaderType shaderType : getLinkedShaderStages())
+    {
+        Shader *shader = mProgramState->getAttachedShader(shaderType);
+        ASSERT(shader);
+        mLinkedOutputVaryings[shaderType] = shader->getOutputVaryings();
+        mLinkedInputVaryings[shaderType]  = shader->getInputVaryings();
+        mLinkedShaderVersions[shaderType] = shader->getShaderVersion();
+    }
+}
+
 }  // namespace gl

src/libANGLE/ProgramExecutable.h

@@ -102,6 +102,7 @@ class ProgramExecutable
 {
   public:
     ProgramExecutable();
+    ProgramExecutable(const ProgramExecutable &other);
     virtual ~ProgramExecutable();
 
     void reset();
@@ -273,6 +274,17 @@ class ProgramExecutable
         return *mResources;
     }
 
+    void saveLinkedStateInfo();
+    std::vector<sh::ShaderVariable> getLinkedOutputVaryings(ShaderType shaderType)
+    {
+        return mLinkedOutputVaryings[shaderType];
+    }
+    std::vector<sh::ShaderVariable> getLinkedInputVaryings(ShaderType shaderType)
+    {
+        return mLinkedInputVaryings[shaderType];
+    }
+    int getLinkedShaderVersion(ShaderType shaderType) { return mLinkedShaderVersions[shaderType]; }
+
   private:
     // TODO(timvp): http://anglebug.com/3570: Investigate removing these friend
     // class declarations and accessing the necessary members with getters/setters.
@@ -343,6 +355,9 @@ class ProgramExecutable
     RangeUI mImageUniformRange;
     std::vector<InterfaceBlock> mShaderStorageBlocks;
 
+    ShaderMap<std::vector<sh::ShaderVariable>> mLinkedOutputVaryings;
+    ShaderMap<std::vector<sh::ShaderVariable>> mLinkedInputVaryings;
+    ShaderMap<int> mLinkedShaderVersions;
     // TODO: http://anglebug.com/4514: Remove
     std::unique_ptr<gl::ProgramLinkedResources> mResources;
 };

src/libANGLE/ProgramPipeline.cpp

@@ -514,31 +514,26 @@ bool ProgramPipeline::linkVaryings(InfoLog &infoLog) const
     ShaderType previousShaderType = ShaderType::InvalidEnum;
     for (ShaderType shaderType : getExecutable().getLinkedShaderStages())
     {
-        Program *currentProgram = mState.mPrograms[shaderType];
-        ASSERT(currentProgram);
-        Shader *currentShader =
-            const_cast<Shader *>(currentProgram->getState().getAttachedShader(shaderType));
-        ASSERT(currentShader);
+        Program *program = getShaderProgram(shaderType);
+        ASSERT(program);
+        ProgramExecutable &executable = program->getExecutable();
 
         if (previousShaderType != ShaderType::InvalidEnum)
         {
-            const Program *previousProgram = mState.mPrograms[previousShaderType];
+            Program *previousProgram = getShaderProgram(previousShaderType);
             ASSERT(previousProgram);
-            Shader *previousShader = const_cast<Shader *>(
-                previousProgram->getState().getAttachedShader(previousShaderType));
-            ASSERT(previousShader);
-            const std::vector<sh::ShaderVariable> &outputVaryings =
-                previousShader->getOutputVaryings();
+            ProgramExecutable &previousExecutable = previousProgram->getExecutable();
 
             if (!Program::linkValidateShaderInterfaceMatching(
-                    outputVaryings, currentShader->getInputVaryings(), previousShaderType,
-                    currentShader->getType(), previousShader->getShaderVersion(),
-                    currentShader->getShaderVersion(), true, infoLog))
+                    previousExecutable.getLinkedOutputVaryings(previousShaderType),
+                    executable.getLinkedInputVaryings(shaderType), previousShaderType, shaderType,
+                    previousExecutable.getLinkedShaderVersion(previousShaderType),
+                    executable.getLinkedShaderVersion(shaderType), true, infoLog))
             {
                 return false;
             }
         }
-        previousShaderType = currentShader->getType();
+        previousShaderType = shaderType;
     }
 
     Program *vertexProgram   = mState.mPrograms[ShaderType::Vertex];
@@ -547,15 +542,12 @@ bool ProgramPipeline::linkVaryings(InfoLog &infoLog) const
     {
         return false;
     }
-    Shader *vertexShader =
-        const_cast<Shader *>(vertexProgram->getState().getAttachedShader(ShaderType::Vertex));
-    Shader *fragmentShader =
-        const_cast<Shader *>(fragmentProgram->getState().getAttachedShader(ShaderType::Fragment));
-    ASSERT(vertexShader);
-    ASSERT(fragmentShader);
-    return Program::linkValidateBuiltInVaryings(vertexShader->getOutputVaryings(),
-                                                fragmentShader->getInputVaryings(),
-                                                vertexShader->getShaderVersion(), infoLog);
+    ProgramExecutable &vertexExecutable   = vertexProgram->getExecutable();
+    ProgramExecutable &fragmentExecutable = fragmentProgram->getExecutable();
+    return Program::linkValidateBuiltInVaryings(
+        vertexExecutable.getLinkedOutputVaryings(ShaderType::Vertex),
+        fragmentExecutable.getLinkedInputVaryings(ShaderType::Fragment),
+        vertexExecutable.getLinkedShaderVersion(ShaderType::Vertex), infoLog);
 }
 
 void ProgramPipeline::validate(const gl::Context *context)

src/libANGLE/State.cpp

@@ -3236,6 +3236,13 @@ angle::Result State::onProgramExecutableChange(const Context *context, Program *
     //  generated executable code will be installed as part of the current rendering state."
     ASSERT(program->isLinked());
 
+    // If this Program is currently active, we need to update the State's pointer to the current
+    // ProgramExecutable if we just changed it.
+    if (mProgram == program)
+    {
+        mExecutable = &program->getExecutable();
+    }
+
     mDirtyBits.set(DIRTY_BIT_PROGRAM_EXECUTABLE);
 
     if (program->hasAnyDirtyBit())

src/tests/deqp_support/deqp_khr_gles31_test_expectations.txt

@@ -36,9 +36,6 @@
 //// Failures blocking an official GLES 3.1 conformance run on SwiftShader
 ////
 
-// Separate shader objects:
-3570 VULKAN : KHR-GLES31.core.sepshaderobjs.StateInteraction = SKIP
-
 // Image related failures
 
 4315 VULKAN : KHR-GLES31.core.shader_image_load_store.advanced-memory-order-vsfs = FAIL