Vulkan: Deduplicate SpvAssignLocations call in program pipeline

Some logic already in SpvGetShaderSpirvCode was duplicated for program
pipelines.  In particular, this necessitated exposing UniformBindingInfo
which is an internal detail of SpvAssignLocations.

In a follow up, UniformBindingInfo will be removed altogether.

Bug: angleproject:7220
Change-Id: I9fa60c716de6dfb0d3a72c4f4dd3822043bab308
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4622010
Reviewed-by: Yuxin Hu <yuxinhu@google.com>
Reviewed-by: Roman Lavrov <romanl@google.com>
Commit-Queue: Shahbaz Youssefi <syoussefi@chromium.org>

src/libANGLE/ProgramExecutable.h

@@ -125,7 +125,7 @@ class ProgramExecutable final : public angle::Subject
     void resetInfoLog() { mInfoLog.reset(); }
 
     void resetLinkedShaderStages() { mLinkedShaderStages.reset(); }
-    const ShaderBitSet &getLinkedShaderStages() const { return mLinkedShaderStages; }
+    const ShaderBitSet getLinkedShaderStages() const { return mLinkedShaderStages; }
     void setLinkedShaderStages(ShaderType shaderType)
     {
         mLinkedShaderStages.set(shaderType);

src/libANGLE/renderer/vulkan/ProgramPipelineVk.cpp

@@ -69,19 +69,11 @@ angle::Result ProgramPipelineVk::link(const gl::Context *glContext,
 
     mExecutable.mOriginalShaderInfo.clear();
 
-    gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum;
-    UniformBindingIndexMap uniformBindingIndexMap;
+    SpvAssignLocations(options, glExecutable, varyingPacking, linkedTransformFeedbackStage,
+                       &spvProgramInterfaceInfo, &mExecutable.mVariableInfoMap);
+
     for (const gl::ShaderType shaderType : glExecutable.getLinkedShaderStages())
     {
-        const bool isTransformFeedbackStage =
-            shaderType == linkedTransformFeedbackStage &&
-            !glExecutable.getLinkedTransformFeedbackVaryings().empty();
-
-        SpvAssignLocations(options, glExecutable, varyingPacking, shaderType, frontShaderType,
-                           isTransformFeedbackStage, &spvProgramInterfaceInfo,
-                           &uniformBindingIndexMap, &mExecutable.mVariableInfoMap);
-        frontShaderType = shaderType;
-
         const gl::Program *program               = mState.getShaderProgram(shaderType);
         ProgramVk *programVk                     = vk::GetImpl(program);
         ProgramExecutableVk &programExecutableVk = programVk->getExecutable();

src/libANGLE/renderer/vulkan/spv_utils.cpp

@@ -653,6 +653,21 @@ void AssignUniformBindings(const SpvSourceOptions &options,
     }
 }
 
+struct UniformBindingInfo final
+{
+    UniformBindingInfo();
+    UniformBindingInfo(uint32_t bindingIndex,
+                       uint32_t idInFrontShader,
+                       gl::ShaderBitSet shaderBitSet,
+                       gl::ShaderType frontShaderType);
+    uint32_t bindingIndex          = 0;
+    uint32_t idInFrontShader       = 0;
+    gl::ShaderBitSet shaderBitSet  = gl::ShaderBitSet();
+    gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum;
+};
+
+using UniformBindingIndexMap = angle::HashMap<std::string, UniformBindingInfo>;
+
 bool InsertIfAbsent(UniformBindingIndexMap *uniformBindingIndexMapOut,
                     const std::string &name,
                     const uint32_t varId,
@@ -4912,90 +4927,97 @@ uint32_t SpvGetXfbBufferBlockId(const uint32_t bufferIndex)
 void SpvAssignLocations(const SpvSourceOptions &options,
                         const gl::ProgramExecutable &programExecutable,
                         const gl::ProgramVaryingPacking &varyingPacking,
-                        const gl::ShaderType shaderType,
-                        const gl::ShaderType frontShaderType,
-                        bool isTransformFeedbackStage,
+                        const gl::ShaderType transformFeedbackStage,
                         SpvProgramInterfaceInfo *programInterfaceInfo,
-                        UniformBindingIndexMap *uniformBindingIndexMapOut,
                         ShaderInterfaceVariableInfoMap *variableInfoMapOut)
 {
+    const gl::ShaderBitSet shaderStages = programExecutable.getLinkedShaderStages();
+
     // Assign outputs to the fragment shader, if any.
-    if ((shaderType == gl::ShaderType::Fragment) &&
+    if (shaderStages[gl::ShaderType::Fragment] &&
         programExecutable.hasLinkedShaderStage(gl::ShaderType::Fragment))
     {
         AssignOutputLocations(programExecutable, gl::ShaderType::Fragment, variableInfoMapOut);
     }
 
     // Assign attributes to the vertex shader, if any.
-    if ((shaderType == gl::ShaderType::Vertex) &&
+    if (shaderStages[gl::ShaderType::Vertex] &&
         programExecutable.hasLinkedShaderStage(gl::ShaderType::Vertex))
     {
         AssignAttributeLocations(programExecutable, gl::ShaderType::Vertex, variableInfoMapOut);
+
+        if (options.supportsTransformFeedbackEmulation)
+        {
+            // If transform feedback emulation is not enabled, mark all transform feedback output
+            // buffers as inactive.
+            const bool isTransformFeedbackStage =
+                transformFeedbackStage == gl::ShaderType::Vertex &&
+                options.enableTransformFeedbackEmulation &&
+                !programExecutable.getLinkedTransformFeedbackVaryings().empty();
+
+            AssignTransformFeedbackEmulationBindings(gl::ShaderType::Vertex, programExecutable,
+                                                     isTransformFeedbackStage, programInterfaceInfo,
+                                                     variableInfoMapOut);
+        }
     }
 
-    if (programExecutable.hasLinkedGraphicsShader())
+    UniformBindingIndexMap uniformBindingIndexMap;
+    gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum;
+    for (const gl::ShaderType shaderType : shaderStages)
     {
-        const gl::VaryingPacking &inputPacking  = varyingPacking.getInputPacking(shaderType);
-        const gl::VaryingPacking &outputPacking = varyingPacking.getOutputPacking(shaderType);
-
-        // Assign varying locations.
-        if (shaderType != gl::ShaderType::Vertex)
+        if (programExecutable.hasLinkedGraphicsShader())
         {
-            AssignVaryingLocations(options, inputPacking, shaderType, frontShaderType,
-                                   programInterfaceInfo, variableInfoMapOut);
+            const gl::VaryingPacking &inputPacking  = varyingPacking.getInputPacking(shaderType);
+            const gl::VaryingPacking &outputPacking = varyingPacking.getOutputPacking(shaderType);
 
-            // Record active members of in gl_PerVertex.
-            if (shaderType != gl::ShaderType::Fragment &&
-                frontShaderType != gl::ShaderType::InvalidEnum)
+            // Assign varying locations.
+            if (shaderType != gl::ShaderType::Vertex)
             {
-                // If an output builtin is active in the previous stage, assume it's active in the
-                // input of the current stage as well.
+                AssignVaryingLocations(options, inputPacking, shaderType, frontShaderType,
+                                       programInterfaceInfo, variableInfoMapOut);
+
+                // Record active members of in gl_PerVertex.
+                if (shaderType != gl::ShaderType::Fragment &&
+                    frontShaderType != gl::ShaderType::InvalidEnum)
+                {
+                    // If an output builtin is active in the previous stage, assume it's active in
+                    // the input of the current stage as well.
+                    const gl::ShaderMap<gl::PerVertexMemberBitSet> &outputPerVertexActiveMembers =
+                        inputPacking.getOutputPerVertexActiveMembers();
+                    variableInfoMapOut->setInputPerVertexActiveMembers(
+                        shaderType, outputPerVertexActiveMembers[frontShaderType]);
+                }
+            }
+            if (shaderType != gl::ShaderType::Fragment)
+            {
+                AssignVaryingLocations(options, outputPacking, shaderType, frontShaderType,
+                                       programInterfaceInfo, variableInfoMapOut);
+
+                // Record active members of out gl_PerVertex.
                 const gl::ShaderMap<gl::PerVertexMemberBitSet> &outputPerVertexActiveMembers =
-                    inputPacking.getOutputPerVertexActiveMembers();
-                variableInfoMapOut->setInputPerVertexActiveMembers(
-                    shaderType, outputPerVertexActiveMembers[frontShaderType]);
+                    outputPacking.getOutputPerVertexActiveMembers();
+                variableInfoMapOut->setOutputPerVertexActiveMembers(
+                    shaderType, outputPerVertexActiveMembers[shaderType]);
+            }
+
+            // Assign qualifiers to all varyings captured by transform feedback
+            if (!programExecutable.getLinkedTransformFeedbackVaryings().empty() &&
+                shaderType == programExecutable.getLinkedTransformFeedbackStage())
+            {
+                AssignTransformFeedbackQualifiers(programExecutable, outputPacking, shaderType,
+                                                  options.supportsTransformFeedbackExtension,
+                                                  variableInfoMapOut);
             }
         }
-        if (shaderType != gl::ShaderType::Fragment)
-        {
-            AssignVaryingLocations(options, outputPacking, shaderType, frontShaderType,
-                                   programInterfaceInfo, variableInfoMapOut);
 
-            // Record active members of out gl_PerVertex.
-            const gl::ShaderMap<gl::PerVertexMemberBitSet> &outputPerVertexActiveMembers =
-                outputPacking.getOutputPerVertexActiveMembers();
-            variableInfoMapOut->setOutputPerVertexActiveMembers(
-                shaderType, outputPerVertexActiveMembers[shaderType]);
-        }
+        AssignUniformBindings(options, programExecutable, shaderType, programInterfaceInfo,
+                              variableInfoMapOut);
+        AssignTextureBindings(options, programExecutable, shaderType, programInterfaceInfo,
+                              &uniformBindingIndexMap, variableInfoMapOut);
+        AssignNonTextureBindings(options, programExecutable, shaderType, programInterfaceInfo,
+                                 &uniformBindingIndexMap, variableInfoMapOut);
 
-        // Assign qualifiers to all varyings captured by transform feedback
-        if (!programExecutable.getLinkedTransformFeedbackVaryings().empty() &&
-            shaderType == programExecutable.getLinkedTransformFeedbackStage())
-        {
-            AssignTransformFeedbackQualifiers(programExecutable, outputPacking, shaderType,
-                                              options.supportsTransformFeedbackExtension,
-                                              variableInfoMapOut);
-        }
-    }
-
-    AssignUniformBindings(options, programExecutable, shaderType, programInterfaceInfo,
-                          variableInfoMapOut);
-    AssignTextureBindings(options, programExecutable, shaderType, programInterfaceInfo,
-                          uniformBindingIndexMapOut, variableInfoMapOut);
-    AssignNonTextureBindings(options, programExecutable, shaderType, programInterfaceInfo,
-                             uniformBindingIndexMapOut, variableInfoMapOut);
-
-    if (options.supportsTransformFeedbackEmulation &&
-        gl::ShaderTypeSupportsTransformFeedback(shaderType))
-    {
-        // If transform feedback emulation is not enabled, mark all transform feedback output
-        // buffers as inactive.
-        isTransformFeedbackStage =
-            isTransformFeedbackStage && options.enableTransformFeedbackEmulation;
-
-        AssignTransformFeedbackEmulationBindings(shaderType, programExecutable,
-                                                 isTransformFeedbackStage, programInterfaceInfo,
-                                                 variableInfoMapOut);
+        frontShaderType = shaderType;
     }
 }
 
@@ -5060,8 +5082,7 @@ void SpvGetShaderSpirvCode(const gl::Context *context,
     }
 
     const gl::ProgramExecutable &programExecutable = programState.getExecutable();
-    gl::ShaderType xfbStage        = programState.getAttachedTransformFeedbackStage();
-    gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum;
+    gl::ShaderType xfbStage = programState.getAttachedTransformFeedbackStage();
 
     // This should be done before assigning varying location. Otherwise, We can encounter shader
     // interface mismatching problem in case the transformFeedback stage is not Vertex stage.
@@ -5077,17 +5098,9 @@ void SpvGetShaderSpirvCode(const gl::Context *context,
                                                 programInterfaceInfo, variableInfoMapOut);
         }
     }
-    UniformBindingIndexMap uniformBindingIndexMap;
-    for (const gl::ShaderType shaderType : programExecutable.getLinkedShaderStages())
-    {
-        const bool isXfbStage = shaderType == xfbStage &&
-                                !programExecutable.getLinkedTransformFeedbackVaryings().empty();
-        SpvAssignLocations(options, programExecutable, resources.varyingPacking, shaderType,
-                           frontShaderType, isXfbStage, programInterfaceInfo,
-                           &uniformBindingIndexMap, variableInfoMapOut);
 
-        frontShaderType = shaderType;
-    }
+    SpvAssignLocations(options, programExecutable, resources.varyingPacking, xfbStage,
+                       programInterfaceInfo, variableInfoMapOut);
 }
 
 angle::Result SpvTransformSpirvCode(const SpvTransformOptions &options,

src/libANGLE/renderer/vulkan/spv_utils.h

@@ -52,21 +52,6 @@ struct SpvTransformOptions
     bool useSpirvVaryingPrecisionFixer  = false;
 };
 
-struct UniformBindingInfo final
-{
-    UniformBindingInfo();
-    UniformBindingInfo(uint32_t bindingIndex,
-                       uint32_t idInFrontShader,
-                       gl::ShaderBitSet shaderBitSet,
-                       gl::ShaderType frontShaderType);
-    uint32_t bindingIndex          = 0;
-    uint32_t idInFrontShader       = 0;
-    gl::ShaderBitSet shaderBitSet  = gl::ShaderBitSet();
-    gl::ShaderType frontShaderType = gl::ShaderType::InvalidEnum;
-};
-
-using UniformBindingIndexMap = angle::HashMap<std::string, UniformBindingInfo>;
-
 struct ShaderInterfaceVariableXfbInfo
 {
     static constexpr uint32_t kInvalid = std::numeric_limits<uint32_t>::max();
@@ -127,18 +112,13 @@ struct ShaderInterfaceVariableInfo
     bool isDuplicate = false;
 };
 
-bool GetImageNameWithoutIndices(std::string *name);
-
 uint32_t SpvGetXfbBufferBlockId(const uint32_t bufferIndex);
 
 void SpvAssignLocations(const SpvSourceOptions &options,
                         const gl::ProgramExecutable &programExecutable,
                         const gl::ProgramVaryingPacking &varyingPacking,
-                        const gl::ShaderType shaderType,
-                        const gl::ShaderType frontShaderType,
-                        bool isTransformFeedbackStage,
+                        const gl::ShaderType transformFeedbackStage,
                         SpvProgramInterfaceInfo *programInterfaceInfo,
-                        UniformBindingIndexMap *uniformBindingIndexMapOut,
                         ShaderInterfaceVariableInfoMap *variableInfoMapOut);
 
 void SpvAssignTransformFeedbackLocations(gl::ShaderType shaderType,