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>

1 files changed, 26 insertions, 53 deletions

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