Remove some cruft/complexity from IR serialization (#7483)

* Remove some cruft/complexity from IR serialization

This is a very simple cleanup to unnecessary code paths and remove some flexibility that isn't actually needed, to hopefully simplify the task of more completely overhauling the approach to IR serialization in a later change.

The concrete feature that gets removed here is a debug-only feature (which thus shouldn't be affecting any users of Slang) that was added long ago in the life of the compiler as we were working to truly separate the front- and back-ends.
At the time there was a lot of code in the compiler back-end that still made use of AST-level data structures, and thus got in the way of our goal to support separate compilation and linking (such that final code generation can only depend on the IR, and not the AST).
The option was used to cause the Slang IR to be serialized out and then read back in as part of compilation, to try and enforce that only the wanted constructs could pass through that bottleneck.

The idea was only ever half implemented, however, because it made use of a secondary implementation path in IR serialization that supported serializing the "raw" source locations (which are heavily dependent on AST-level information, even down to the number of bytes in source files).
This change removes the feature entirely, since it is no longer useful for its intended purpose, and its presence causes there to be entire second code path for source locations in IR serialization that would need to have test coverage if we wanted to be sure it kept working.

In addition, our pre-existing infrastructure for module serialization had various options that have either stopped being useful, or were not really useful at the time they were introduced.

For example: there are no places in the code today where we attempt to serialize out a module without including both the serialized AST and IR.
If that was a feature that we ever supported, the relevant code got removed at some preceding point without breaking any of our tests or (seemingly) upsetting users.

Similarly, the options being passed into writing of a serialized module included both a flag to control whether source locations should be serialized *and* a pointer to the `SourceManager` to use in that case... but it was only ever meaningful to set both, or neither.
The option has been changed to just be the `SourceManager` pointer, and the name has been updated to reflect its very narrow intended use case.

* format code

* fixup

* regenerate command line reference

---------

Co-authored-by: slangbot <186143334+slangbot@users.noreply.github.com>
Co-authored-by: Yong He <yonghe@outlook.com>

9 files changed, 42 insertions, 140 deletions

diff --git a/source/slang/slang-compiler.cpp b/source/slang/slang-compiler.cpp
index 4ba937992..dc202c3b0 100644
--- a/source/slang/slang-compiler.cpp
+++ b/source/slang/slang-compiler.cpp
@@ -2375,8 +2375,7 @@ SlangResult EndToEndCompileRequest::writeContainerToStream(Stream* stream)
     // If debug information is enabled, enable writing out source locs
     if (_shouldWriteSourceLocs(linkage))
     {
-        options.optionFlags |= SerialOptionFlag::SourceLocation;
-        options.sourceManager = linkage->getSourceManager();
+        options.sourceManagerToUseWhenSerializingSourceLocs = linkage->getSourceManager();
     }
 
     SLANG_RETURN_ON_FAIL(SerialContainerUtil::write(this, options, stream));
@@ -2919,7 +2918,6 @@ bool CodeGenContext::isSpecializationDisabled()
 SLANG_NO_THROW SlangResult SLANG_MCALL Module::serialize(ISlangBlob** outSerializedBlob)
 {
     SerialContainerUtil::WriteOptions writeOptions;
-    writeOptions.sourceManager = getLinkage()->getSourceManager();
     OwnedMemoryStream memoryStream(FileAccess::Write);
     SLANG_RETURN_ON_FAIL(SerialContainerUtil::write(this, writeOptions, &memoryStream));
     *outSerializedBlob = RawBlob::create(
@@ -2932,7 +2930,6 @@ SLANG_NO_THROW SlangResult SLANG_MCALL Module::serialize(ISlangBlob** outSeriali
 SLANG_NO_THROW SlangResult SLANG_MCALL Module::writeToFile(char const* fileName)
 {
     SerialContainerUtil::WriteOptions writeOptions;
-    writeOptions.sourceManager = getLinkage()->getSourceManager();
     FileStream fileStream;
     SLANG_RETURN_ON_FAIL(fileStream.init(fileName, FileMode::Create));
     return SerialContainerUtil::write(this, writeOptions, &fileStream);
diff --git a/source/slang/slang-compiler.h b/source/slang/slang-compiler.h
index cee80f882..7071e4c73 100644
--- a/source/slang/slang-compiler.h
+++ b/source/slang/slang-compiler.h
@@ -2659,10 +2659,6 @@ public:
         return translationUnits[index];
     }
 
-    // If true then generateIR will serialize out IR, and serialize back in again. Making
-    // serialization a bottleneck or firewall between the front end and the backend
-    bool useSerialIRBottleneck = false;
-
     // If true will serialize and de-serialize with debug information
     bool verifyDebugSerialization = false;
 
diff --git a/source/slang/slang-options.cpp b/source/slang/slang-options.cpp
index 28953b4ed..3227e2de4 100644
--- a/source/slang/slang-options.cpp
+++ b/source/slang/slang-options.cpp
@@ -809,10 +809,10 @@ void initCommandOptions(CommandOptions& options)
          "-output-includes",
          nullptr,
          "Print the hierarchy of the processed source files."},
-        {OptionKind::SerialIr,
+        {OptionKind::REMOVED_SerialIR,
          "-serial-ir",
          nullptr,
-         "Serialize the IR between front-end and back-end."},
+         "[REMOVED] Serialize the IR between front-end and back-end."},
         {OptionKind::SkipCodeGen, "-skip-codegen", nullptr, "Skip the code generation phase."},
         {OptionKind::ValidateIr, "-validate-ir", nullptr, "Validate the IR between the phases."},
         {OptionKind::VerbosePaths,
@@ -2394,9 +2394,6 @@ SlangResult OptionsParser::_parse(int argc, char const* const* argv)
         case OptionKind::ReproFileSystem:
             SLANG_RETURN_ON_FAIL(_parseReproFileSystem(arg));
             break;
-        case OptionKind::SerialIr:
-            m_frontEndReq->useSerialIRBottleneck = true;
-            break;
         case OptionKind::VerbosePaths:
             m_requestImpl->getSink()->setFlag(DiagnosticSink::Flag::VerbosePath);
             break;
diff --git a/source/slang/slang-serialize-container.cpp b/source/slang/slang-serialize-container.cpp
index ba8c74ca4..1d7f6d438 100644
--- a/source/slang/slang-serialize-container.cpp
+++ b/source/slang/slang-serialize-container.cpp
@@ -18,7 +18,6 @@ namespace Slang
 struct ModuleEncodingContext
 {
 private:
-    SerialContainerUtil::WriteOptions const& _options;
     Stream* _stream = nullptr;
 
     // The string pool used across the whole of the container
@@ -30,11 +29,12 @@ private:
 
 public:
     ModuleEncodingContext(SerialContainerUtil::WriteOptions const& options, Stream* stream)
-        : _options(options), _stream(stream), _containerStringPool(StringSlicePool::Style::Default)
+        : _stream(stream), _containerStringPool(StringSlicePool::Style::Default)
     {
-        if (options.optionFlags & SerialOptionFlag::SourceLocation)
+        if (options.sourceManagerToUseWhenSerializingSourceLocs)
         {
-            _sourceLocWriter = new SerialSourceLocWriter(options.sourceManager);
+            _sourceLocWriter =
+                new SerialSourceLocWriter(options.sourceManagerToUseWhenSerializingSourceLocs);
         }
 
         _cursor = RIFF::BuildCursor(_riff);
@@ -140,8 +140,7 @@ public:
         IRSerialData serialData;
         IRSerialWriter writer;
 
-        SLANG_RETURN_ON_FAIL(
-            writer.write(irModule, _sourceLocWriter, _options.optionFlags, &serialData));
+        SLANG_RETURN_ON_FAIL(writer.write(irModule, _sourceLocWriter, &serialData));
         SLANG_RETURN_ON_FAIL(IRSerialWriter::writeTo(serialData, _cursor));
 
         return SLANG_OK;
@@ -185,9 +184,6 @@ public:
 
     SlangResult encode(Module* module)
     {
-        if (!(_options.optionFlags & (SerialOptionFlag::IRModule | SerialOptionFlag::ASTModule)))
-            return SLANG_OK;
-
         SLANG_SCOPED_RIFF_BUILDER_LIST_CHUNK(_cursor, SerialBinary::kModuleFourCC);
 
         // The first piece that we write for a module is its header.
@@ -216,31 +212,22 @@ public:
             encodeModuleDependencyPaths(module);
         }
 
-        // If serialization of Slang IR modules is enabled, and there
-        // is IR available for this module, then we we encode it.
+        // If there is IR available for this module, then we we encode it.
         //
-        if ((_options.optionFlags & SerialOptionFlag::IRModule))
+        if (auto irModule = module->getIRModule())
         {
-            if (auto irModule = module->getIRModule())
-            {
-                IRSerialData serialData;
-                IRSerialWriter writer;
-                SLANG_RETURN_ON_FAIL(
-                    writer.write(irModule, _sourceLocWriter, _options.optionFlags, &serialData));
-                SLANG_RETURN_ON_FAIL(IRSerialWriter::writeTo(serialData, _cursor));
-            }
+            IRSerialData serialData;
+            IRSerialWriter writer;
+            SLANG_RETURN_ON_FAIL(writer.write(irModule, _sourceLocWriter, &serialData));
+            SLANG_RETURN_ON_FAIL(IRSerialWriter::writeTo(serialData, _cursor));
         }
 
-        // If serialization of AST information is enabled, and we have AST
-        // information available, then we serialize it here.
+        // If we have AST information available, then we serialize it here.
         //
-        if (_options.optionFlags & SerialOptionFlag::ASTModule)
+        if (auto moduleDecl = module->getModuleDecl())
         {
-            if (auto moduleDecl = module->getModuleDecl())
-            {
-                SLANG_SCOPED_RIFF_BUILDER_LIST_CHUNK(_cursor, PropertyKeys<Module>::ASTModule);
-                writeSerializedModuleAST(_cursor, moduleDecl, _sourceLocWriter);
-            }
+            SLANG_SCOPED_RIFF_BUILDER_LIST_CHUNK(_cursor, PropertyKeys<Module>::ASTModule);
+            writeSerializedModuleAST(_cursor, moduleDecl, _sourceLocWriter);
         }
 
         return SLANG_OK;
@@ -643,16 +630,16 @@ SlangResult decodeModuleIR(
 
         RefPtr<SerialSourceLocWriter> sourceLocWriter;
 
-        if (options.optionFlags & SerialOptionFlag::SourceLocation)
+        if (options.sourceManagerToUseWhenSerializingSourceLocs)
         {
-            sourceLocWriter = new SerialSourceLocWriter(options.sourceManager);
+            sourceLocWriter =
+                new SerialSourceLocWriter(options.sourceManagerToUseWhenSerializingSourceLocs);
         }
 
         {
-            // Write IR out to serialData - copying over SourceLoc information directly
+            // Write IR out to `irData`
             IRSerialWriter writer;
-            SLANG_RETURN_ON_FAIL(
-                writer.write(module, sourceLocWriter, options.optionFlags, &irData));
+            SLANG_RETURN_ON_FAIL(writer.write(module, sourceLocWriter, &irData));
         }
         SLANG_RETURN_ON_FAIL(IRSerialWriter::writeTo(irData, cursor));
 
@@ -672,7 +659,7 @@ SlangResult decodeModuleIR(
     memoryStream.seek(SeekOrigin::Start, 0);
 
     SourceManager workSourceManager;
-    workSourceManager.initialize(options.sourceManager, nullptr);
+    workSourceManager.initialize(options.sourceManagerToUseWhenSerializingSourceLocs, nullptr);
 
     // The read ir module
     RefPtr<IRModule> irReadModule;
@@ -695,7 +682,7 @@ SlangResult decodeModuleIR(
         RefPtr<SerialSourceLocReader> sourceLocReader;
 
         // If we have debug info then find and read it
-        if (options.optionFlags & SerialOptionFlag::SourceLocation)
+        if (options.sourceManagerToUseWhenSerializingSourceLocs)
         {
             auto debugChunk = DebugChunk::find(moduleChunk);
             if (!debugChunk)
@@ -742,23 +729,7 @@ SlangResult decodeModuleIR(
         return SLANG_FAIL;
     }
 
-    if (options.optionFlags & SerialOptionFlag::RawSourceLocation)
-    {
-        SLANG_ASSERT(readInsts[0] == originalInsts[0]);
-        // All the source locs should be identical
-        for (Index i = 1; i < readInsts.getCount(); ++i)
-        {
-            IRInst* origInst = originalInsts[i];
-            IRInst* readInst = readInsts[i];
-
-            if (origInst->sourceLoc.getRaw() != readInst->sourceLoc.getRaw())
-            {
-                SLANG_ASSERT(!"Source locs don't match");
-                return SLANG_FAIL;
-            }
-        }
-    }
-    else if (options.optionFlags & SerialOptionFlag::SourceLocation)
+    if (options.sourceManagerToUseWhenSerializingSourceLocs)
     {
         // They should be on the same line nos
         for (Index i = 1; i < readInsts.getCount(); ++i)
@@ -772,7 +743,9 @@ SlangResult decodeModuleIR(
             }
 
             // Work out the
-            SourceView* origSourceView = options.sourceManager->findSourceView(origInst->sourceLoc);
+            SourceView* origSourceView =
+                options.sourceManagerToUseWhenSerializingSourceLocs->findSourceView(
+                    origInst->sourceLoc);
             SourceView* readSourceView = workSourceManager.findSourceView(readInst->sourceLoc);
 
             // if both are null we are done
diff --git a/source/slang/slang-serialize-container.h b/source/slang/slang-serialize-container.h
index 81d03251f..1608b085c 100644
--- a/source/slang/slang-serialize-container.h
+++ b/source/slang/slang-serialize-container.h
@@ -17,10 +17,16 @@ struct SerialContainerUtil
 {
     struct WriteOptions
     {
-        SerialOptionFlags optionFlags =
-            SerialOptionFlag::ASTModule |
-            SerialOptionFlag::IRModule; ///< Flags controlling what is written
-        SourceManager* sourceManager =
+        /// The source manager that is used by `SourceLoc`s in the input.
+        ///
+        /// If null, source location information will not be serialized
+        /// as part of the output.
+        ///
+        /// If non-null, it must be the `SourceManager` that was used to
+        /// create any `SourceLoc`s that appear in the module(s) that get
+        /// written.
+        ///
+        SourceManager* sourceManagerToUseWhenSerializingSourceLocs =
             nullptr; ///< The source manager used for the SourceLoc in the input
     };
 
diff --git a/source/slang/slang-serialize-ir.cpp b/source/slang/slang-serialize-ir.cpp
index f03f46da8..662f8acf1 100644
--- a/source/slang/slang-serialize-ir.cpp
+++ b/source/slang/slang-serialize-ir.cpp
@@ -108,7 +108,6 @@ Result IRSerialWriter::_calcDebugInfo(SerialSourceLocWriter* sourceLocWriter)
 Result IRSerialWriter::write(
     IRModule* module,
     SerialSourceLocWriter* sourceLocWriter,
-    SerialOptionFlags options,
     IRSerialData* serialData)
 {
     typedef Ser::Inst::PayloadType PayloadType;
@@ -306,23 +305,7 @@ Result IRSerialWriter::write(
         SerialStringTableUtil::encodeStringTable(m_stringSlicePool, serialData->m_stringTable);
     }
 
-    // If the option to use RawSourceLocations is enabled, serialize out as is
-    if (options & SerialOptionFlag::RawSourceLocation)
-    {
-        const Index numInsts = m_insts.getCount();
-        serialData->m_rawSourceLocs.setCount(numInsts);
-
-        Ser::RawSourceLoc* dstLocs = serialData->m_rawSourceLocs.begin();
-        // 0 is null, just mark as no location
-        dstLocs[0] = Ser::RawSourceLoc(0);
-        for (Index i = 1; i < numInsts; ++i)
-        {
-            IRInst* srcInst = m_insts[i];
-            dstLocs[i] = Ser::RawSourceLoc(srcInst->sourceLoc.getRaw());
-        }
-    }
-
-    if ((options & SerialOptionFlag::SourceLocation) && sourceLocWriter)
+    if (sourceLocWriter)
     {
         _calcDebugInfo(sourceLocWriter);
     }
diff --git a/source/slang/slang-serialize-ir.h b/source/slang/slang-serialize-ir.h
index c9fb16c27..f6ab7e9ad 100644
--- a/source/slang/slang-serialize-ir.h
+++ b/source/slang/slang-serialize-ir.h
@@ -25,7 +25,6 @@ struct IRSerialWriter
     Result write(
         IRModule* module,
         SerialSourceLocWriter* sourceLocWriter,
-        SerialOptionFlags flags,
         IRSerialData* serialData);
 
     /// Write to a container
diff --git a/source/slang/slang-serialize-types.h b/source/slang/slang-serialize-types.h
index 09de865db..be49be1ea 100644
--- a/source/slang/slang-serialize-types.h
+++ b/source/slang/slang-serialize-types.h
@@ -13,25 +13,6 @@ namespace Slang
 {
 class Module;
 
-// Options for IR/AST/Debug serialization
-
-struct SerialOptionFlag
-{
-    typedef uint32_t Type;
-    enum Enum : Type
-    {
-        RawSourceLocation =
-            0x01, ///< If set will store directly SourceLoc - only useful if current source locs
-                  ///< will be identical when read in (typically this is *NOT* the case)
-        SourceLocation = 0x02, ///< If set will output SourceLoc information, that can be
-                               ///< reconstructed when read after being stored.
-        ASTModule = 0x04, ///< If set will output AST modules - typically required, but potentially
-                          ///< not desired (for example with obsfucation)
-        IRModule = 0x08,  ///< If set will output IR modules - typically required
-    };
-};
-typedef SerialOptionFlag::Type SerialOptionFlags;
-
 struct SerialStringData
 {
     enum class StringIndex : uint32_t;
diff --git a/source/slang/slang.cpp b/source/slang/slang.cpp
index f2f28e3e3..e149058b1 100644
--- a/source/slang/slang.cpp
+++ b/source/slang/slang.cpp
@@ -637,13 +637,11 @@ SlangResult Session::saveBuiltinModule(
     // We want builtin modules to be saved with their source location
     // information.
     //
-    options.optionFlags |= SerialOptionFlag::SourceLocation;
-    //
     // And in order to work with source locations, the serialization
     // process will also need access to the source manager that
     // can translate locations into their humane format.
     //
-    options.sourceManager = m_builtinLinkage->getSourceManager();
+    options.sourceManagerToUseWhenSerializingSourceLocs = m_builtinLinkage->getSourceManager();
 
     // At this point we can finally delegate down to the next level,
     // which handles the serialization of a Slang module into a
@@ -3619,8 +3617,7 @@ void FrontEndCompileRequest::generateIR()
         {
             SerialContainerUtil::WriteOptions options;
 
-            options.sourceManager = getSourceManager();
-            options.optionFlags |= SerialOptionFlag::SourceLocation;
+            options.sourceManagerToUseWhenSerializingSourceLocs = getSourceManager();
 
             // Verify debug information
             if (SLANG_FAILED(
@@ -3632,33 +3629,6 @@ void FrontEndCompileRequest::generateIR()
             }
         }
 
-        if (useSerialIRBottleneck)
-        {
-            // Keep the obfuscated source map (if there is one)
-            ComPtr<IBoxValue<SourceMap>> obfuscatedSourceMap(irModule->getObfuscatedSourceMap());
-
-            IRSerialData serialData;
-            {
-                // Write IR out to serialData - copying over SourceLoc information directly
-                IRSerialWriter writer;
-                writer.write(irModule, nullptr, SerialOptionFlag::RawSourceLocation, &serialData);
-
-                // Destroy irModule such that memory can be used for newly constructed read
-                // irReadModule
-                irModule = nullptr;
-            }
-            RefPtr<IRModule> irReadModule;
-            {
-                // Read IR back from serialData
-                IRSerialReader reader;
-                reader.read(serialData, getSession(), nullptr, irReadModule);
-            }
-
-            // Set irModule to the read module
-            irModule = irReadModule;
-            irModule->setObfuscatedSourceMap(obfuscatedSourceMap);
-        }
-
         // Set the module on the translation unit
         translationUnit->getModule()->setIRModule(irModule);
     }