Vulkan: SPIR-V Gen: Code blocks

src/compiler/translator/BuildSPIRV.cpp

@@ -783,6 +783,68 @@ spirv::IdRef SPIRVBuilder::getCompositeConstant(spirv::IdRef typeId, const spirv
     return iter->second;
 }
 
+void SPIRVBuilder::startNewFunction()
+{
+    ASSERT(mSpirvCurrentFunctionBlocks.empty());
+
+    // Add the first block of the function.
+    mSpirvCurrentFunctionBlocks.emplace_back();
+    mSpirvCurrentFunctionBlocks.back().labelId = getNewId();
+}
+
+void SPIRVBuilder::assembleSpirvFunctionBlocks()
+{
+    // Take all the blocks and place them in the functions section of SPIR-V in sequence.
+    for (const SpirvBlock &block : mSpirvCurrentFunctionBlocks)
+    {
+        // Every block must be properly terminated.
+        ASSERT(block.isTerminated);
+
+        // Generate the OpLabel instruction for the block.
+        spirv::WriteLabel(&mSpirvFunctions, block.labelId);
+
+        // Add the variable declarations if any.
+        mSpirvFunctions.insert(mSpirvFunctions.end(), block.localVariables.begin(),
+                               block.localVariables.end());
+
+        // Add the body of the block.
+        mSpirvFunctions.insert(mSpirvFunctions.end(), block.body.begin(), block.body.end());
+    }
+
+    // Clean up.
+    mSpirvCurrentFunctionBlocks.clear();
+}
+
+spirv::IdRef SPIRVBuilder::declareVariable(spirv::IdRef typeId,
+                                           spv::StorageClass storageClass,
+                                           spirv::IdRef *initializerId,
+                                           const char *name)
+{
+    const bool isFunctionLocal = storageClass == spv::StorageClassFunction;
+
+    // Make sure storage class is consistent with where the variable is declared.
+    ASSERT(!isFunctionLocal || !mSpirvCurrentFunctionBlocks.empty());
+
+    // Function-local variables go in the first block of the function, while the rest are in the
+    // global variables section.
+    spirv::Blob *spirvSection = isFunctionLocal
+                                    ? &mSpirvCurrentFunctionBlocks.front().localVariables
+                                    : &mSpirvVariableDecls;
+
+    spirv::IdRef variableId          = getNewId();
+    const spirv::IdRef typePointerId = getTypePointerId(typeId, storageClass);
+
+    spirv::WriteVariable(spirvSection, typePointerId, variableId, storageClass, initializerId);
+
+    // Output debug information.
+    if (name)
+    {
+        spirv::WriteName(&mSpirvDebug, variableId, name);
+    }
+
+    return variableId;
+}
+
 uint32_t SPIRVBuilder::nextUnusedBinding()
 {
     return mNextUnusedBinding++;

src/compiler/translator/BuildSPIRV.h

@@ -154,6 +154,31 @@ struct SpirvTypeData
     uint32_t sizeInStorageBlock;
 };
 
+// A block of code.  SPIR-V produces forward references to blocks, such as OpBranchConditional
+// specifying the id of the if and else blocks, each of those referencing the id of the block after
+// the else.  Additionally, local variable declarations are accumulated at the top of the first
+// block in a function.  For these reasons, each block of SPIR-V is generated separately and
+// assembled at the end of the function, allowing prior blocks to be modified when necessary.
+struct SpirvBlock
+{
+    // Id of the block
+    spirv::IdRef labelId;
+
+    // Local variable declarations.  Only the first block of a function is allowed to contain any
+    // instructions here.
+    spirv::Blob localVariables;
+
+    // Everything *after* OpLabel (which itself is not generated until blocks are assembled) and
+    // local variables.
+    spirv::Blob body;
+
+    // Whether the block is terminated.  Useful for functions without return, asserting that code is
+    // not added after return/break/continue etc (i.e. dead code, which should really be removed
+    // earlier by a transformation, but could also be hacked by returning a bogus block to contain
+    // all the "garbage" to throw away), last switch case without a break, etc.
+    bool isTerminated = false;
+};
+
 // Helper class to construct SPIR-V
 class SPIRVBuilder : angle::NonCopyable
 {
@@ -182,6 +207,22 @@ class SPIRVBuilder : angle::NonCopyable
     spirv::Blob *getSpirvTypePointerDecls() { return &mSpirvTypePointerDecls; }
     spirv::Blob *getSpirvVariableDecls() { return &mSpirvVariableDecls; }
     spirv::Blob *getSpirvFunctions() { return &mSpirvFunctions; }
+    spirv::Blob *getSpirvCurrentFunctionBlock()
+    {
+        ASSERT(!mSpirvCurrentFunctionBlocks.empty() &&
+               !mSpirvCurrentFunctionBlocks.back().isTerminated);
+        return &mSpirvCurrentFunctionBlocks.back().body;
+    }
+    bool isCurrentFunctionBlockTerminated() const
+    {
+        ASSERT(!mSpirvCurrentFunctionBlocks.empty());
+        return mSpirvCurrentFunctionBlocks.back().isTerminated;
+    }
+    void terminateCurrentFunctionBlock()
+    {
+        ASSERT(!mSpirvCurrentFunctionBlocks.empty());
+        mSpirvCurrentFunctionBlocks.back().isTerminated = true;
+    }
 
     void addCapability(spv::Capability capability);
     void addExecutionMode(spv::ExecutionMode executionMode);
@@ -199,6 +240,17 @@ class SPIRVBuilder : angle::NonCopyable
     spirv::IdRef getFloatConstant(float value);
     spirv::IdRef getCompositeConstant(spirv::IdRef typeId, const spirv::IdRefList &values);
 
+    // Helpers to start and end a function.
+    void startNewFunction();
+    void assembleSpirvFunctionBlocks();
+
+    // Helper to declare a variable.  Function-local variables must be placed in the first block of
+    // the current function.
+    spirv::IdRef declareVariable(spirv::IdRef typeId,
+                                 spv::StorageClass storageClass,
+                                 spirv::IdRef *initializerId,
+                                 const char *name);
+
     // TODO: remove name hashing once translation through glslang is removed.  That is necessary to
     // avoid name collision between ANGLE's internal symbols and user-defined ones when compiling
     // the generated GLSL, but is irrelevant when generating SPIR-V directly.  Currently, the SPIR-V
@@ -257,7 +309,7 @@ class SPIRVBuilder : angle::NonCopyable
     angle::HashMap<SpirvType, SpirvTypeData, SpirvTypeHash> mTypeMap;
 
     // Various sections of SPIR-V.  Each section grows as SPIR-V is generated, and the final result
-    // is obtained by stiching the sections together.  This puts the instructions in the order
+    // is obtained by stitching the sections together.  This puts the instructions in the order
     // required by the spec.
     spirv::Blob mSpirvExecutionModes;
     spirv::Blob mSpirvDebug;
@@ -266,6 +318,13 @@ class SPIRVBuilder : angle::NonCopyable
     spirv::Blob mSpirvTypePointerDecls;
     spirv::Blob mSpirvVariableDecls;
     spirv::Blob mSpirvFunctions;
+    // A list of blocks created for the current function.  These are assembled by
+    // assembleSpirvFunctionBlocks() when the function is entirely visited.  Local variables need to
+    // be inserted at the beginning of the first function block, so the entire SPIR-V of the
+    // function cannot be obtained until it's fully visited.
+    //
+    // The last block in this list is the one currently being written to.
+    std::vector<SpirvBlock> mSpirvCurrentFunctionBlocks;
 
     // List of constants that are already defined (for reuse).
     spirv::IdRef mBoolConstants[2];

src/compiler/translator/OutputSPIRV.cpp

@@ -399,8 +399,8 @@ void OutputSPIRVTraverser::accessChainPushDynamicComponent(NodeData *data,
         // Index that vector constant with the dynamic index.  For example, vec.ywxz[i] becomes the
         // constant {1, 3, 0, 2} indexed with i, and that index used on vec.
         const spirv::IdRef newIndex = mBuilder.getNewId();
-        spirv::WriteVectorExtractDynamic(mBuilder.getSpirvFunctions(), uintTypeId, newIndex,
-                                         swizzlesId, index);
+        spirv::WriteVectorExtractDynamic(mBuilder.getSpirvCurrentFunctionBlock(), uintTypeId,
+                                         newIndex, swizzlesId, index);
 
         index = newIndex;
         accessChain.swizzles.clear();
@@ -437,8 +437,8 @@ spirv::IdRef OutputSPIRVTraverser::accessChainCollapse(NodeData *data)
         mBuilder.getTypePointerId(accessChain.preSwizzleTypeId, accessChain.storageClass);
 
     accessChain.accessChainId = mBuilder.getNewId();
-    spirv::WriteAccessChain(mBuilder.getSpirvFunctions(), typePointerId, accessChain.accessChainId,
-                            data->baseId, indexIds);
+    spirv::WriteAccessChain(mBuilder.getSpirvCurrentFunctionBlock(), typePointerId,
+                            accessChain.accessChainId, data->baseId, indexIds);
 
     return accessChain.accessChainId;
 }
@@ -475,7 +475,7 @@ spirv::IdRef OutputSPIRVTraverser::accessChainLoad(NodeData *data)
                 makeAccessChainLiteralList(data, &indexList);
 
                 const spirv::IdRef result = mBuilder.getNewId();
-                spirv::WriteCompositeExtract(mBuilder.getSpirvFunctions(),
+                spirv::WriteCompositeExtract(mBuilder.getSpirvCurrentFunctionBlock(),
                                              accessChain.preSwizzleTypeId, result, loadResult,
                                              indexList);
                 loadResult = result;
@@ -483,14 +483,12 @@ spirv::IdRef OutputSPIRVTraverser::accessChainLoad(NodeData *data)
             else
             {
                 // Create a temp variable to hold the rvalue so an access chain can be made on it.
-                // TODO: variables need to be placed at the top of the SPIR-V block.  This will be
-                // fixed when blocks are properly supported.  http://anglebug.com/4889
-                const spirv::IdRef tempVar = mBuilder.getNewId();
-                spirv::WriteVariable(mBuilder.getSpirvFunctions(), accessChain.preSwizzleTypeId,
-                                     tempVar, spv::StorageClassFunction, nullptr);
+                const spirv::IdRef tempVar = mBuilder.declareVariable(
+                    accessChain.preSwizzleTypeId, spv::StorageClassFunction, nullptr, "indexable");
 
                 // Write the rvalue into the temp variable
-                spirv::WriteStore(mBuilder.getSpirvFunctions(), tempVar, loadResult, nullptr);
+                spirv::WriteStore(mBuilder.getSpirvCurrentFunctionBlock(), tempVar, loadResult,
+                                  nullptr);
 
                 // Make the temp variable the source of the access chain.
                 data->baseId                   = tempVar;
@@ -499,8 +497,8 @@ spirv::IdRef OutputSPIRVTraverser::accessChainLoad(NodeData *data)
                 // Load from the temp variable.
                 const spirv::IdRef accessChainId = accessChainCollapse(data);
                 loadResult                       = mBuilder.getNewId();
-                spirv::WriteLoad(mBuilder.getSpirvFunctions(), accessChain.preSwizzleTypeId,
-                                 loadResult, accessChainId, nullptr);
+                spirv::WriteLoad(mBuilder.getSpirvCurrentFunctionBlock(),
+                                 accessChain.preSwizzleTypeId, loadResult, accessChainId, nullptr);
             }
         }
     }
@@ -509,8 +507,8 @@ spirv::IdRef OutputSPIRVTraverser::accessChainLoad(NodeData *data)
         // Load from the access chain.
         const spirv::IdRef accessChainId = accessChainCollapse(data);
         loadResult                       = mBuilder.getNewId();
-        spirv::WriteLoad(mBuilder.getSpirvFunctions(), accessChain.preSwizzleTypeId, loadResult,
-                         accessChainId, nullptr);
+        spirv::WriteLoad(mBuilder.getSpirvCurrentFunctionBlock(), accessChain.preSwizzleTypeId,
+                         loadResult, accessChainId, nullptr);
     }
 
     if (!accessChain.swizzles.empty())
@@ -526,8 +524,9 @@ spirv::IdRef OutputSPIRVTraverser::accessChainLoad(NodeData *data)
         }
 
         const spirv::IdRef result = mBuilder.getNewId();
-        spirv::WriteVectorShuffle(mBuilder.getSpirvFunctions(), accessChain.postSwizzleTypeId,
-                                  result, loadResult, loadResult, swizzleList);
+        spirv::WriteVectorShuffle(mBuilder.getSpirvCurrentFunctionBlock(),
+                                  accessChain.postSwizzleTypeId, result, loadResult, loadResult,
+                                  swizzleList);
         loadResult = result;
     }
 
@@ -538,7 +537,7 @@ spirv::IdRef OutputSPIRVTraverser::accessChainLoad(NodeData *data)
 
         // Use OpVectorExtractDynamic to select the component.
         const spirv::IdRef result = mBuilder.getNewId();
-        spirv::WriteVectorExtractDynamic(mBuilder.getSpirvFunctions(),
+        spirv::WriteVectorExtractDynamic(mBuilder.getSpirvCurrentFunctionBlock(),
                                          accessChain.postDynamicComponentTypeId, result, loadResult,
                                          accessChain.dynamicComponent);
         loadResult = result;
@@ -570,8 +569,8 @@ void OutputSPIRVTraverser::accessChainStore(NodeData *data, spirv::IdRef value)
     {
         // Load the vector before the swizzle.
         const spirv::IdRef loadResult = mBuilder.getNewId();
-        spirv::WriteLoad(mBuilder.getSpirvFunctions(), accessChain.preSwizzleTypeId, loadResult,
-                         accessChainId, nullptr);
+        spirv::WriteLoad(mBuilder.getSpirvCurrentFunctionBlock(), accessChain.preSwizzleTypeId,
+                         loadResult, accessChainId, nullptr);
 
         // Overwrite the components being written.  This is done by first creating an identity
         // swizzle, then replacing the components being written with a swizzle from the value.  For
@@ -603,13 +602,14 @@ void OutputSPIRVTraverser::accessChainStore(NodeData *data, spirv::IdRef value)
         // Use the generated swizzle to select components from the loaded vector and the value to be
         // written.  Use the final result as the value to be written to the vector.
         const spirv::IdRef result = mBuilder.getNewId();
-        spirv::WriteVectorShuffle(mBuilder.getSpirvFunctions(), accessChain.postSwizzleTypeId,
-                                  result, loadResult, value, swizzleList);
+        spirv::WriteVectorShuffle(mBuilder.getSpirvCurrentFunctionBlock(),
+                                  accessChain.postSwizzleTypeId, result, loadResult, value,
+                                  swizzleList);
         value = result;
     }
 
     // Store through the access chain.
-    spirv::WriteStore(mBuilder.getSpirvFunctions(), accessChainId, value, nullptr);
+    spirv::WriteStore(mBuilder.getSpirvCurrentFunctionBlock(), accessChainId, value, nullptr);
 }
 
 void OutputSPIRVTraverser::makeAccessChainIdList(NodeData *data, spirv::IdRefList *idsOut)
@@ -808,7 +808,8 @@ spirv::IdRef OutputSPIRVTraverser::createArrayOrStructConstructor(
     const spirv::IdRefList &parameters)
 {
     const spirv::IdRef result = mBuilder.getNewId();
-    spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), typeId, result, parameters);
+    spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), typeId, result,
+                                   parameters);
     return result;
 }
 
@@ -822,7 +823,7 @@ spirv::IdRef OutputSPIRVTraverser::createConstructorVectorFromScalar(
     spirv::IdRefList replicatedParameter(type.getNominalSize(), parameters[0]);
 
     const spirv::IdRef result = mBuilder.getNewId();
-    spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), typeId, result,
+    spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), typeId, result,
                                    replicatedParameter);
     return result;
 }
@@ -838,7 +839,7 @@ spirv::IdRef OutputSPIRVTraverser::createConstructorVectorFromNonScalar(
     extractComponents(node, node->getType().getNominalSize(), parameters, &extractedComponents);
 
     const spirv::IdRef result = mBuilder.getNewId();
-    spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), typeId, result,
+    spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), typeId, result,
                                    extractedComponents);
     return result;
 }
@@ -898,13 +899,14 @@ spirv::IdRef OutputSPIRVTraverser::createConstructorMatrixFromScalar(
         }
 
         // Create the column.
-        spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), columnTypeId, columnIds.back(),
-                                       componentIds);
+        spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), columnTypeId,
+                                       columnIds.back(), componentIds);
     }
 
     // Create the matrix out of the columns.
     const spirv::IdRef result = mBuilder.getNewId();
-    spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), typeId, result, columnIds);
+    spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), typeId, result,
+                                   columnIds);
     return result;
 }
 
@@ -939,12 +941,13 @@ spirv::IdRef OutputSPIRVTraverser::createConstructorMatrixFromVectors(
         const spirv::IdRefList componentIds(componentsStart, componentsStart + type.getRows());
 
         // Create the column.
-        spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), columnTypeId, columnIds.back(),
-                                       componentIds);
+        spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), columnTypeId,
+                                       columnIds.back(), componentIds);
     }
 
     const spirv::IdRef result = mBuilder.getNewId();
-    spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), typeId, result, columnIds);
+    spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), typeId, result,
+                                   columnIds);
     return result;
 }
 
@@ -1002,7 +1005,7 @@ spirv::IdRef OutputSPIRVTraverser::createConstructorMatrixFromMatrix(
         {
             // Extract the column.
             const spirv::IdRef parameterColumnId = mBuilder.getNewId();
-            spirv::WriteCompositeExtract(mBuilder.getSpirvFunctions(), paramColumnTypeId,
+            spirv::WriteCompositeExtract(mBuilder.getSpirvCurrentFunctionBlock(), paramColumnTypeId,
                                          parameterColumnId, parameters[0],
                                          {spirv::LiteralInteger(columnIndex)});
 
@@ -1011,7 +1014,7 @@ spirv::IdRef OutputSPIRVTraverser::createConstructorMatrixFromMatrix(
             if (needsSwizzle)
             {
                 constructorColumnId = mBuilder.getNewId();
-                spirv::WriteVectorShuffle(mBuilder.getSpirvFunctions(), columnTypeId,
+                spirv::WriteVectorShuffle(mBuilder.getSpirvCurrentFunctionBlock(), columnTypeId,
                                           constructorColumnId, parameterColumnId, parameterColumnId,
                                           swizzle);
             }
@@ -1040,8 +1043,8 @@ spirv::IdRef OutputSPIRVTraverser::createConstructorMatrixFromMatrix(
                 if (componentIndex < parameterType.getRows())
                 {
                     componentId = mBuilder.getNewId();
-                    spirv::WriteCompositeExtract(mBuilder.getSpirvFunctions(), paramComponentTypeId,
-                                                 componentId, parameters[0],
+                    spirv::WriteCompositeExtract(mBuilder.getSpirvCurrentFunctionBlock(),
+                                                 paramComponentTypeId, componentId, parameters[0],
                                                  {spirv::LiteralInteger(columnIndex),
                                                   spirv::LiteralInteger(componentIndex)});
                 }
@@ -1072,13 +1075,14 @@ spirv::IdRef OutputSPIRVTraverser::createConstructorMatrixFromMatrix(
 
             // Create the column vector.
             columnIds.push_back(mBuilder.getNewId());
-            spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), columnTypeId,
+            spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), columnTypeId,
                                            columnIds.back(), componentIds);
         }
     }
 
     const spirv::IdRef result = mBuilder.getNewId();
-    spirv::WriteCompositeConstruct(mBuilder.getSpirvFunctions(), typeId, result, columnIds);
+    spirv::WriteCompositeConstruct(mBuilder.getSpirvCurrentFunctionBlock(), typeId, result,
+                                   columnIds);
     return result;
 }
 
@@ -1120,8 +1124,8 @@ void OutputSPIRVTraverser::extractComponents(TIntermAggregate *node,
                  ++componentIndex)
             {
                 const spirv::IdRef componentId = mBuilder.getNewId();
-                spirv::WriteCompositeExtract(mBuilder.getSpirvFunctions(), componentTypeId,
-                                             componentId, parameterId,
+                spirv::WriteCompositeExtract(mBuilder.getSpirvCurrentFunctionBlock(),
+                                             componentTypeId, componentId, parameterId,
                                              {spirv::LiteralInteger(componentIndex)});
 
                 extractedComponentsOut->push_back(componentId);
@@ -1148,7 +1152,8 @@ void OutputSPIRVTraverser::extractComponents(TIntermAggregate *node,
             {
                 const spirv::IdRef componentId = mBuilder.getNewId();
                 spirv::WriteCompositeExtract(
-                    mBuilder.getSpirvFunctions(), componentTypeId, componentId, parameterId,
+                    mBuilder.getSpirvCurrentFunctionBlock(), componentTypeId, componentId,
+                    parameterId,
                     {spirv::LiteralInteger(columnIndex), spirv::LiteralInteger(componentIndex)});
 
                 extractedComponentsOut->push_back(componentId);
@@ -1422,30 +1427,17 @@ bool OutputSPIRVTraverser::visitBlock(Visit visit, TIntermBlock *node)
         return true;
     }
 
-    // When starting the block, generate an OpLabel instruction.  This is referenced by instructions
-    // that reference the block such as OpBranchConditional.
+    // Any construct that needs code blocks must have already handled creating the necessary blocks
+    // and setting the right one "current".  If there's a block opened in GLSL for scoping reasons,
+    // it's ignored here as there are no scopes within a function in SPIR-V.
     if (visit == PreVisit)
     {
-        mNodeData.emplace_back();
-
-        const spirv::IdRef blockLabelId = mBuilder.getNewId();
-        spirv::WriteLabel(mBuilder.getSpirvFunctions(), blockLabelId);
-
-        mNodeData.back().baseId = blockLabelId;
-    }
-    else
-    {
-        // Any node that needed to generate code has already done so, just clean up its data.  If
-        // the child node has no effect, it's automatically discarded (such as variable.field[n].x,
-        // side effects of n already having generated code).
-        mNodeData.pop_back();
-    }
-
-    if (visit != PostVisit)
-    {
-        return true;
+        return node->getChildCount() > 0;
     }
 
+    // Any node that needed to generate code has already done so, just clean up its data.  If
+    // the child node has no effect, it's automatically discarded (such as variable.field[n].x,
+    // side effects of n already having generated code).
     mNodeData.pop_back();
 
     return true;
@@ -1491,19 +1483,25 @@ bool OutputSPIRVTraverser::visitFunctionDefinition(Visit visit, TIntermFunctionD
             mBuilder.setEntryPointId(functionId);
         }
 
+        mBuilder.startNewFunction();
+
         return true;
     }
 
     if (visit == PostVisit)
     {
-        // TODO: if the function returns void, the AST may not have an explicit OpReturn node, so
-        // generate one at the end if not already.  For testing, unconditionally add it.
-        // http://anglebug.com/4889
-        if (node->getFunction()->getReturnType().getBasicType() == EbtVoid)
+        // If no explicit return was specified, add one automatically here.
+        if (!mBuilder.isCurrentFunctionBlockTerminated())
         {
-            spirv::WriteReturn(mBuilder.getSpirvFunctions());
+            // Only meaningful if the function returns void.  Otherwise it must have had a return
+            // value.
+            ASSERT(node->getFunction()->getReturnType().getBasicType() == EbtVoid);
+            spirv::WriteReturn(mBuilder.getSpirvCurrentFunctionBlock());
+            mBuilder.terminateCurrentFunctionBlock();
         }
 
+        mBuilder.assembleSpirvFunctionBlocks();
+
         // End the function
         spirv::WriteFunctionEnd(mBuilder.getSpirvFunctions());
     }
@@ -1579,6 +1577,11 @@ bool OutputSPIRVTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
 
 bool OutputSPIRVTraverser::visitDeclaration(Visit visit, TIntermDeclaration *node)
 {
+    if (!mInGlobalScope && visit == PreVisit)
+    {
+        mNodeData.emplace_back();
+    }
+
     if (visit != PreVisit)
     {
         return true;
@@ -1608,16 +1611,11 @@ bool OutputSPIRVTraverser::visitDeclaration(Visit visit, TIntermDeclaration *nod
 
     // TODO: handle constant declarations.  http://anglebug.com/4889
 
-    spv::StorageClass storageClass   = GetStorageClass(type);
-    const spirv::IdRef typePointerId = mBuilder.getTypePointerId(typeId, storageClass);
+    spv::StorageClass storageClass = GetStorageClass(type);
 
-    spirv::Blob *spirvSection = storageClass == spv::StorageClassFunction
-                                    ? mBuilder.getSpirvFunctions()
-                                    : mBuilder.getSpirvVariableDecls();
-
-    const spirv::IdRef variableId = mBuilder.getNewId();
     // TODO: handle initializers.  http://anglebug.com/4889
-    spirv::WriteVariable(spirvSection, typePointerId, variableId, storageClass, nullptr);
+    const spirv::IdRef variableId =
+        mBuilder.declareVariable(typeId, storageClass, nullptr, mBuilder.hashName(variable).data());
 
     if (IsShaderIn(type.getQualifier()) || IsShaderOut(type.getQualifier()))
     {
@@ -1647,9 +1645,6 @@ bool OutputSPIRVTraverser::visitDeclaration(Visit visit, TIntermDeclaration *nod
     // Write DescriptorSet, Binding, Location etc decorations if necessary.
     mBuilder.writeInterfaceVariableDecorations(type, variableId);
 
-    // Output debug information.
-    spirv::WriteName(mBuilder.getSpirvDebug(), variableId, mBuilder.hashName(variable).data());
-
     // Remember the id of the variable for future look up.  For interface blocks, also remember the
     // id of the interface block.
     ASSERT(mSymbolIdMap.count(variable) == 0);