Feature/ir serialize improvements (#655)

* * Change the layout of IROp such that 'main' IROps are 0-x.
* Removed MANUAL_RANGE instuction types, as no longer needed.

* Work in prog on optimizing.

* * Constant time lookup for IROpInfo
* Refactor and document a little more the IROp layout
* Mark ops that use 'other' bits

* Fix typo in definition of kIROpFlag_UseOther

* First pass at working out serialization structure.

* Work in progress on ir-serialize

* Storing strings in IRSerialInfo
Split out IRSerialInfo from the IRSerializer - to make more explicit what is actually saved.

* First pass at serializing out data.

* First pass at serialize reading.

* Fix riff fourcc mark order.

* First pass at reconstructing IRInst / IRDecoration from serialized data.

* Handling of TextureBaseType

* Deserializing of constants.

* Small changes around ir serialization.

* Changed StringIndex indexing to not be an offset into the m_strings array, but an index into strings in order. Doing so makes cache lookup much faster, and makes the 'indicies' themselves smaller and therefore more compressible.

* Removed the need for m_arena in IRSerialWriter. Previously it's purpose was to store the string contents that were being used to lookup UnownedStringSlice.
Now we keep the StringRepresentation in scope and reference that, and so don't need the copy.

* Don't need to construct the IRModuleInst as is created and set on createModule call.

* Remove test code for testing serialization.

* Fix problem with release build in ir-serialize causing warning.

* Use SLANG_OFFSET_OF for offsets in non pod classes to avoid gcc/clang warning.
Give storage to integral static variables to avoid linkage problems with gcc/clang.

* Fix warnings under x86 win32 debug.

* Small improvements around IR serialization.

2 files changed, 164 insertions, 176 deletions

diff --git a/source/slang/ir-serialize.cpp b/source/slang/ir-serialize.cpp
index 854b60419..2645e445d 100644
--- a/source/slang/ir-serialize.cpp
+++ b/source/slang/ir-serialize.cpp
@@ -27,6 +27,20 @@ to an IRType derived type. Its 'ok' as long as it's an instruction that can be u
 bother to check if it's correct, and just casts it.
 */
 
+/* static */const IRSerialData::PayloadInfo IRSerialData::s_payloadInfos[int(Inst::PayloadType::CountOf)] = 
+{
+    { 0, 0 },   // Empty
+    { 1, 0 },   // Operand_1
+    { 2, 0 },   // Operand_2
+    { 1, 0 },   // OperandAndUInt32,
+    { 0, 0 },   // OperandExternal - This isn't correct, Operand has to be specially handled
+    { 0, 1 },   // String_1,              
+    { 0, 2 },   // String_2,              
+    { 0, 0 },   // UInt32,               
+    { 0, 0 },   // Float64,
+    { 0, 0 }    // Int64,
+};
+
 static bool isParentDerived(IROp opIn)
 {
     const int op = (kIROpMeta_PseudoOpMask & opIn);
@@ -112,7 +126,6 @@ void IRSerialWriter::_addInstruction(IRInst* inst)
     }
 }
 
-
 IRSerialData::StringIndex IRSerialWriter::getStringIndex(Name* name) 
 { 
     return name ? getStringIndex(name->text.getStringRepresentation()) : Ser::kNullStringIndex; 
@@ -297,8 +310,9 @@ Result IRSerialWriter::write(IRModule* module, IRSerialData* serialData)
             IRTextureTypeBase* textureBase = as<IRTextureTypeBase>(srcInst);
             if (textureBase)
             {
-                dstInst.m_payloadType = Ser::Inst::PayloadType::UInt32;
-                dstInst.m_payload.m_uint32 = uint32_t(srcInst->op) >> kIROpMeta_OtherShift;
+                dstInst.m_payloadType = Ser::Inst::PayloadType::OperandAndUInt32;
+                dstInst.m_payload.m_operandAndUInt32.m_uint32 = uint32_t(srcInst->op) >> kIROpMeta_OtherShift;
+                dstInst.m_payload.m_operandAndUInt32.m_operand = getInstIndex(textureBase->getElementType());
                 continue;
             }
 
@@ -315,7 +329,7 @@ Result IRSerialWriter::write(IRModule* module, IRSerialData* serialData)
             }
             else
             {
-                dstInst.m_payloadType = Ser::Inst::PayloadType::ExternalOperand;
+                dstInst.m_payloadType = Ser::Inst::PayloadType::OperandExternal;
 
                 int operandArrayBaseIndex = int(m_serialData->m_externalOperands.Count());
                 m_serialData->m_externalOperands.SetSize(operandArrayBaseIndex + numOperands);
@@ -407,6 +421,13 @@ Result IRSerialWriter::write(IRModule* module, IRSerialData* serialData)
                     dstInst.m_payload.m_stringIndices[0] = getStringIndex(semanticDecor->semanticName);
                     break;
                 }
+                case kIRDecorationOp_InterpolationMode:
+                {
+                    auto semanticDecor = static_cast<IRInterpolationModeDecoration*>(srcDecor);
+                    dstInst.m_payloadType = Ser::Inst::PayloadType::UInt32;
+                    dstInst.m_payload.m_uint32 = uint32_t(semanticDecor->mode);
+                    break;
+                }
                 case kIRDecorationOp_NameHint:
                 {
                     auto nameDecor = static_cast<IRNameHintDecoration*>(srcDecor);
@@ -790,42 +811,98 @@ void IRSerialReader::_calcStringStarts()
     memset(m_stringRepresentationCache.begin(), 0, sizeof(StringRepresentation*) * m_stringStarts.Count());
 }
 
-/* static */Result IRSerialReader::read(const IRSerialData& data, TranslationUnitRequest* translationUnit, IRModule** moduleOut)
+IRDecoration* IRSerialReader::_createDecoration(const Ser::Inst& srcInst)
 {
     typedef Ser::Inst::PayloadType PayloadType;
 
-    *moduleOut = nullptr;
-
-    m_serialData = &data;
-    _calcStringStarts();
-
-    auto compileRequest = translationUnit->compileRequest;
-
-    //SharedIRGenContext sharedContextStorage;
-    //SharedIRGenContext* sharedContext = &sharedContextStorage;
+    IRDecorationOp decorOp = IRDecorationOp(srcInst.m_op - kIROpCount);
+    SLANG_ASSERT(decorOp < kIRDecorationOp_CountOf);
 
-    //sharedContext->compileRequest = compileRequest;
-    //sharedContext->mainModuleDecl = translationUnit->SyntaxNode;
-
-    //IRGenContext contextStorage(sharedContext);
-    //IRGenContext* context = &contextStorage;
-
-    SharedIRBuilder sharedBuilderStorage;
-    SharedIRBuilder* sharedBuilder = &sharedBuilderStorage;
-    sharedBuilder->module = nullptr;
-    sharedBuilder->session = compileRequest->mSession;
+    switch (decorOp)
+    {
+        case kIRDecorationOp_HighLevelDecl:
+        {
+            // TODO!
+            // Decl* decl;
+            return createEmptyDecoration<IRHighLevelDeclDecoration>(m_module);
+        }
+        case kIRDecorationOp_Layout:
+        {
+            // TODO!
+            // Layout* layout;
+            return createEmptyDecoration<IRLayoutDecoration>(m_module);
+        }
+        case kIRDecorationOp_LoopControl:
+        {
+            auto decor = createEmptyDecoration<IRLoopControlDecoration>(m_module);
+            SLANG_ASSERT(srcInst.m_payloadType == PayloadType::UInt32);
+            decor->mode = IRLoopControl(srcInst.m_payload.m_uint32);
+            return decor;
+        }
+        case kIRDecorationOp_Target:
+        {
+            auto decor = createEmptyDecoration<IRTargetDecoration>(m_module);
+            SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_1);
+            decor->targetName = getStringRepresentation(srcInst.m_payload.m_stringIndices[0]);
+            return decor;
+        }
+        case kIRDecorationOp_TargetIntrinsic:
+        {
+            auto decor = createEmptyDecoration<IRTargetIntrinsicDecoration>(m_module);
+            SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_2);
+            decor->targetName = getStringRepresentation(srcInst.m_payload.m_stringIndices[0]);
+            decor->definition = getStringRepresentation(srcInst.m_payload.m_stringIndices[1]);
+            return decor;
+        }
+        case kIRDecorationOp_GLSLOuterArray:
+        {
+            auto decor = createEmptyDecoration<IRGLSLOuterArrayDecoration>(m_module);
+            SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_1);
+            decor->outerArrayName = getCStr(srcInst.m_payload.m_stringIndices[0]);
+            return decor;
+        }
+        case kIRDecorationOp_Semantic:
+        {
+            auto decor = createEmptyDecoration<IRSemanticDecoration>(m_module);
+            SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_1);
+            decor->semanticName = getName(srcInst.m_payload.m_stringIndices[0]);
+            return decor;
+        }
+        case kIRDecorationOp_InterpolationMode:
+        {
+            auto decor = createEmptyDecoration<IRInterpolationModeDecoration>(m_module);
+            SLANG_ASSERT(srcInst.m_payloadType == Ser::Inst::PayloadType::UInt32);
+            decor->mode = IRInterpolationMode(srcInst.m_payload.m_uint32);
+            return decor;
+        }
+        case kIRDecorationOp_NameHint:
+        {
+            auto decor = createEmptyDecoration<IRNameHintDecoration>(m_module);
+            SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_1);
+            decor->name = getName(srcInst.m_payload.m_stringIndices[0]);
+            return decor;
+        }
+        default:
+        {
+            SLANG_ASSERT(!"Unhandled decoration type");
+            return nullptr;
+        }
+    }
+}
 
-    IRBuilder builderStorage;
-    IRBuilder* builder = &builderStorage;
-    builder->sharedBuilder = sharedBuilder;
+/* static */Result IRSerialReader::read(const IRSerialData& data, Session* session, RefPtr<IRModule>& moduleOut)
+{
+    typedef Ser::Inst::PayloadType PayloadType;
 
-    RefPtr<IRModule> module = builder->createModule();
-    sharedBuilder->module = module;
+    m_serialData = &data;
+    _calcStringStarts();
 
+    auto module = new IRModule();
+    moduleOut = module;
     m_module = module;
-    
-    //context->irBuilder = builder;
 
+    module->session = session;
+    
     // Add all the instructions
 
     List<IRInst*> insts;
@@ -842,17 +919,20 @@ void IRSerialReader::_calcStringStarts()
     decorations.SetSize(numDecorations);
 
     // 0 holds null
-    // The first instruction must be the module
+    // 1 holds the IRModuleInst
     {
         // Check that insts[1] is the module inst
         const Ser::Inst& srcInst = data.m_insts[1];
         SLANG_RELEASE_ASSERT(srcInst.m_op == kIROp_Module);
-        SLANG_ASSERT(srcInst.getNumOperands() == 0);
         SLANG_ASSERT(srcInst.m_payloadType == PayloadType::Empty);
 
-        // We don't need to create the moduleInst, because it's constructed via the createModule call
-        SLANG_ASSERT(module->moduleInst);
-        insts[1] = module->moduleInst;
+        // Create the module inst
+        auto moduleInst = static_cast<IRModuleInst*>(createEmptyInstWithSize(module, kIROp_Module, sizeof(IRModuleInst)));
+        module->moduleInst = moduleInst;
+        moduleInst->module = module;
+
+        // Set the IRModuleInst
+        insts[1] = moduleInst; 
     }
 
     for (int i = 2; i < numInsts; ++i)
@@ -945,11 +1025,11 @@ void IRSerialReader::_calcStringStarts()
             }
             else if (isTextureTypeBase(op))
             {
-                IRTextureTypeBase* inst = static_cast<IRTextureTypeBase*>(createEmptyInstWithSize(module, op, sizeof(IRTextureTypeBase)));
-                SLANG_ASSERT(srcInst.m_payloadType == PayloadType::UInt32);
+                IRTextureTypeBase* inst = static_cast<IRTextureTypeBase*>(createEmptyInst(module, op, 1));
+                SLANG_ASSERT(srcInst.m_payloadType == PayloadType::OperandAndUInt32);
 
                 // Reintroduce the texture type bits into the the
-                const uint32_t other = srcInst.m_payload.m_uint32;
+                const uint32_t other = srcInst.m_payload.m_operandAndUInt32.m_uint32;
                 inst->op = IROp(uint32_t(inst->op) | (other << kIROpMeta_OtherShift));
 
                 insts[i] = inst;
@@ -963,7 +1043,6 @@ void IRSerialReader::_calcStringStarts()
     } 
 
     // Patch up the operands
-    
     for (int i = 1; i < numInsts; ++i)
     {
         const Ser::Inst& srcInst = data.m_insts[i];
@@ -995,7 +1074,6 @@ void IRSerialReader::_calcStringStarts()
     }
     
     // Patch up the children
-
     {
         const int numChildRuns = int(data.m_childRuns.Count());
         for (int i = 0; i < numChildRuns; i++)
@@ -1016,98 +1094,15 @@ void IRSerialReader::_calcStringStarts()
         }
     }
 
-    // Add the decorations
+    // Create the decorations
     for (int i = 0; i < numDecorations; ++i)
     {
-        const Ser::Inst& srcInst = data.m_insts[i + numInsts];
-        IRDecorationOp decorOp = IRDecorationOp(srcInst.m_op - kIROpCount);
-        SLANG_ASSERT(decorOp < kIRDecorationOp_CountOf);
-
-        switch (decorOp)
+        IRDecoration* decor = _createDecoration(data.m_insts[i + numInsts]);
+        if (!decor)
         {
-            case kIRDecorationOp_HighLevelDecl:
-            {
-                auto decor = createEmptyDecoration<IRHighLevelDeclDecoration>(m_module);
-                decorations[i] = decor;
-
-                // TODO!
-                // Decl* decl;
-                break;
-            }
-            case kIRDecorationOp_Layout:
-            {
-                auto decor = createEmptyDecoration<IRLayoutDecoration>(m_module);
-                decorations[i] = decor;
-
-                // TODO!
-                // Layout* layout;
-                break;
-            }
-            case kIRDecorationOp_LoopControl:
-            {
-                auto decor = createEmptyDecoration<IRLoopControlDecoration>(m_module);
-                decorations[i] = decor;
-
-                SLANG_ASSERT(srcInst.m_payloadType == PayloadType::UInt32);
-                decor->mode = IRLoopControl(srcInst.m_payload.m_uint32);
-                
-                break;
-            }
-            case kIRDecorationOp_Target:
-            {
-                auto decor = createEmptyDecoration<IRTargetDecoration>(m_module);
-                decorations[i] = decor;
-
-                SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_1);
-                decor->targetName = getStringRepresentation(srcInst.m_payload.m_stringIndices[0]);
-                break;
-            }
-            case kIRDecorationOp_TargetIntrinsic:
-            {
-                auto decor = createEmptyDecoration<IRTargetIntrinsicDecoration>(m_module);
-                decorations[i] = decor;
-
-                SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_2);
-                decor->targetName = getStringRepresentation(srcInst.m_payload.m_stringIndices[0]);
-                decor->definition = getStringRepresentation(srcInst.m_payload.m_stringIndices[1]);
-                break;
-            }
-            case kIRDecorationOp_GLSLOuterArray:
-            {
-                auto decor = createEmptyDecoration<IRGLSLOuterArrayDecoration>(m_module);
-                decorations[i] = decor;
-
-                SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_1);
-                decor->outerArrayName = getCStr(srcInst.m_payload.m_stringIndices[0]);
-                break;
-            }
-            case kIRDecorationOp_Semantic:
-            {
-                auto decor = createEmptyDecoration<IRSemanticDecoration>(m_module);
-                decorations[i] = decor;
-
-                SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_1);
-                decor->semanticName = getName(srcInst.m_payload.m_stringIndices[0]);
-                break;
-            }
-            case kIRDecorationOp_NameHint:
-            {
-                auto decor = createEmptyDecoration<IRNameHintDecoration>(m_module);
-                decorations[i] = decor;
-
-                SLANG_ASSERT(srcInst.m_payloadType == PayloadType::String_1);
-                decor->name = getName(srcInst.m_payload.m_stringIndices[0]);
-                break;
-            }
-            default:
-            {
-                SLANG_ASSERT(!"Unhandled decoration type");
-                return SLANG_FAIL;
-            }
+            return SLANG_FAIL;
         }
-        
-        // Make sure something is set
-        SLANG_ASSERT(decorations[i]);
+        decorations[i] = decor;
     }
 
     // Associate the decorations with the instructions
@@ -1126,11 +1121,13 @@ void IRSerialReader::_calcStringStarts()
             IRInst* inst = insts[int(run.m_parentIndex)];
             SLANG_ASSERT(int(run.m_startInstIndex) >= decorationBaseIndex && int(run.m_startInstIndex) + run.m_numChildren <= m_serialData->m_insts.Count());
 
+            // Calculate the offset in the decoration list, which index 0, is decorationBaseIndex in instruction indices
+            const int decorStartIndex = int(run.m_startInstIndex) - decorationBaseIndex;
+
             // Go in reverse order so that linked list is in same order as original
             for (int j = int(run.m_numChildren) - 1; j >= 0; --j)
             {
-                IRDecoration* decor = decorations[int(run.m_startInstIndex) + j - decorationBaseIndex];
-
+                IRDecoration* decor = decorations[decorStartIndex + j]; 
                 // And to the linked list on the 
                 decor->next = inst->firstDecoration;
                 inst->firstDecoration = decor;
@@ -1138,13 +1135,11 @@ void IRSerialReader::_calcStringStarts()
         }
     }
 
-    *moduleOut = module.detach();
     return SLANG_OK;
 }
 
 // !!!!!!!!!!!!!!!!!!!!!!!!!!!! Free functions !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
 
-
 Result serializeModule(IRModule* module, Stream* stream)
 {
     IRSerialWriter serializer;
@@ -1160,22 +1155,13 @@ Result serializeModule(IRModule* module, Stream* stream)
     return SLANG_OK;
 }
 
-Result readModule(TranslationUnitRequest* translationUnit, Stream* stream, IRModule** moduleOut)
+Result readModule(Session* session, Stream* stream, RefPtr<IRModule>& moduleOut)
 {
-    *moduleOut = nullptr;
-
     IRSerialData serialData;
     IRSerialReader::readStream(stream, &serialData);
 
-    RefPtr<IRModule> module;
     IRSerialReader reader;
-
-    SLANG_RETURN_ON_FAIL(reader.read(serialData, translationUnit, module.writeRef()));
-
-    *moduleOut = module.detach();
-
-    return SLANG_OK;
+    return reader.read(serialData, session, moduleOut);
 }
 
-
 } // namespace Slang
diff --git a/source/slang/ir-serialize.h b/source/slang/ir-serialize.h
index e949e1239..31e8aa6ee 100644
--- a/source/slang/ir-serialize.h
+++ b/source/slang/ir-serialize.h
@@ -41,37 +41,39 @@ struct IRSerialData
         SizeType m_numChildren;                 ///< The number of children
     };
 
+    struct PayloadInfo
+    {
+        uint8_t m_numOperands;
+        uint8_t m_numStrings;
+    };
+
     // Instruction...
     // We can store SourceLoc values separately. Just store per index information.
     // Parent information is stored in m_childRuns
     // Decoration information is stored in m_decorationRuns
     struct Inst
     {
+        // NOTE! Can't change order or list without changing approprite s_payloadInfos
         enum class PayloadType : uint8_t
         {
             Empty,                          ///< Has no payload (or operands)
             Operand_1,                      ///< 1 Operand
             Operand_2,                      ///< 2 Operands
-            ExternalOperand,                ///< Operands are held externally
+            OperandAndUInt32,               ///< 1 Operand and a single UInt32
+            OperandExternal,                ///< Operands are held externally
             String_1,                       ///< 1 String
             String_2,                       ///< 2 Strings
             UInt32,                         ///< Holds an unsigned 32 bit integral (might represent a type)
-            Float64,
+            Float64,                    
             Int64,
+            
             CountOf,
         };
 
             /// Get the number of operands
-        int getNumOperands() const 
+        SLANG_FORCE_INLINE int getNumOperands() const 
         {
-            switch (m_payloadType)
-            {
-                default: /* fallthru */
-                case PayloadType::Empty: return 0;
-                case PayloadType::Operand_1: return 1;
-                case PayloadType::Operand_2: return 2;
-                case PayloadType::ExternalOperand: return m_payload.m_externalOperand.m_size;
-            }
+            return (m_payloadType == PayloadType::OperandExternal) ? m_payload.m_externalOperand.m_size : s_payloadInfos[int(m_payloadType)].m_numOperands;
         }
 
         uint8_t m_op;                       ///< For now one of IROp 
@@ -86,6 +88,12 @@ struct IRSerialData
             SizeType m_size;                                ///< The amount of entries in that table
         };
 
+        struct OperandAndUInt32
+        {
+            InstIndex m_operand;
+            uint32_t m_uint32;
+        };
+
         union Payload
         {
             double m_float64;
@@ -96,6 +104,7 @@ struct IRSerialData
             InstIndex m_operands[kNumOperands];	            ///< For items that 2 or less operands it can use this.  
             StringIndex m_stringIndices[kNumOperands];
             ExternalOperandPayload m_externalOperand;              ///< Operands are stored in an an index of an operand array 
+            OperandAndUInt32 m_operandAndUInt32;
         };
 
         Payload m_payload;
@@ -119,27 +128,17 @@ struct IRSerialData
         m_decorationBaseIndex = 0;
     }
 
-    int getOperands(const Inst& inst, const InstIndex** operandsOut) const
+    SLANG_FORCE_INLINE int getOperands(const Inst& inst, const InstIndex** operandsOut) const
     {
-        switch (inst.m_payloadType)
+        if (inst.m_payloadType == Inst::PayloadType::OperandExternal)
+        {
+            *operandsOut = m_externalOperands.begin() + int(inst.m_payload.m_externalOperand.m_arrayIndex);
+            return int(inst.m_payload.m_externalOperand.m_size);
+        }
+        else
         {
-            default:
-            case Inst::PayloadType::Empty:
-            {
-                *operandsOut = nullptr;
-                return 0;
-            }
-            case Inst::PayloadType::Operand_1:
-            case Inst::PayloadType::Operand_2:
-            {
-                *operandsOut = inst.m_payload.m_operands;
-                return int(inst.m_payloadType) - int(Inst::PayloadType::Empty);
-            }
-            case Inst::PayloadType::ExternalOperand:
-            {
-                *operandsOut = m_externalOperands.begin() + int(inst.m_payload.m_externalOperand.m_arrayIndex);
-                return int(inst.m_payload.m_externalOperand.m_size);
-            }
+            *operandsOut = inst.m_payload.m_operands;
+            return s_payloadInfos[int(inst.m_payloadType)].m_numOperands;
         }
     }
 
@@ -158,6 +157,8 @@ struct IRSerialData
 
     List<char> m_strings;                       ///< All strings. Indexed into by StringIndex
 
+    static const PayloadInfo s_payloadInfos[int(Inst::PayloadType::CountOf)];
+    
     int m_decorationBaseIndex;                  ///< All decorations insts are at indices >= to this value
 };
 
@@ -247,7 +248,7 @@ struct IRSerialReader
     static Result readStream(Stream* stream, IRSerialData* dataOut);
 
         /// Read a module from serial data
-    Result read(const IRSerialData& data, TranslationUnitRequest* translationUnit, IRModule** moduleOut);
+    Result read(const IRSerialData& data, Session* session, RefPtr<IRModule>& moduleOut);
 
     Name* getName(Ser::StringIndex index);
     String getString(Ser::StringIndex index);
@@ -266,6 +267,7 @@ struct IRSerialReader
     protected:
 
     void _calcStringStarts();
+    IRDecoration* _createDecoration(const Ser::Inst& srcIns);
 
     List<Ser::StringOffset> m_stringStarts;
     List<StringRepresentation*> m_stringRepresentationCache;
@@ -276,7 +278,7 @@ struct IRSerialReader
 
 
 Result serializeModule(IRModule* module, Stream* stream);
-Result readModule(TranslationUnitRequest* translationUnit, Stream* stream, IRModule** moduleOut);
+Result readModule(Session* session, Stream* stream, RefPtr<IRModule>& moduleOut);
 
 } // namespace Slang