[Remarks] Restructure bitstream remarks to be fully standalone

clang/lib/CodeGen/CodeGenAction.cpp

@@ -29,6 +29,7 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "llvm/ADT/Hashing.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/CodeGen/MachineOptimizationRemarkEmitter.h"
 #include "llvm/Demangle/Demangle.h"
@@ -272,6 +273,8 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) {
 
   std::unique_ptr<llvm::ToolOutputFile> OptRecordFile =
     std::move(*OptRecordFileOrErr);
+  auto FinalizeRemarks =
+      make_scope_exit([&]() { llvm::finalizeLLVMOptimizationRemarks(Ctx); });
 
   if (OptRecordFile && CodeGenOpts.getProfileUse() !=
                            llvm::driver::ProfileInstrKind::ProfileNone)
@@ -1185,6 +1188,8 @@ void CodeGenAction::ExecuteAction() {
   }
   std::unique_ptr<llvm::ToolOutputFile> OptRecordFile =
       std::move(*OptRecordFileOrErr);
+  auto FinalizeRemarks =
+      make_scope_exit([&]() { llvm::finalizeLLVMOptimizationRemarks(Ctx); });
 
   emitBackendOutput(CI, CI.getCodeGenOpts(),
                     CI.getTarget().getDataLayoutString(), TheModule.get(), BA,

flang/lib/Frontend/FrontendActions.cpp

@@ -1356,6 +1356,8 @@ void CodeGenAction::executeAction() {
 
   std::unique_ptr<llvm::ToolOutputFile> optRecordFile =
       std::move(*optRecordFileOrErr);
+  auto FinalizeRemarks =
+      make_scope_exit([&]() { llvm::finalizeLLVMOptimizationRemarks(Ctx); });
 
   if (optRecordFile) {
     optRecordFile->keep();

llvm/docs/Remarks.rst

@@ -152,28 +152,6 @@ Other tools that support remarks:
     .. option:: -opt-remarks-format=<format>
     .. option:: -opt-remarks-with-hotness
 
-Serialization modes
-===================
-
-There are two modes available for serializing remarks:
-
-``Separate``
-
-    In this mode, the remarks and the metadata are serialized separately. The
-    client is responsible for parsing the metadata first, then use the metadata
-    to correctly parse the remarks.
-
-``Standalone``
-
-    In this mode, the remarks and the metadata are serialized to the same
-    stream. The metadata will always come before the remarks.
-
-    The compiler does not support emitting standalone remarks. This mode is
-    more suited for post-processing tools like linkers, that can merge the
-    remarks for one whole project.
-
-.. _yamlremarks:
-
 YAML remarks
 ============
 
@@ -374,27 +352,10 @@ This block can contain the following records:
 The remark container
 --------------------
 
-Bitstream remarks are designed to be used in two different modes:
-
-``The separate mode``
-
-    The separate mode is the mode that is typically used during compilation. It
-    provides a way to serialize the remark entries to a stream while some
-    metadata is kept in memory to be emitted in the product of the compilation
-    (typically, an object file).
+The bitstream remark container supports multiple types:
 
-``The standalone mode``
-
-    The standalone mode is typically stored and used after the distribution of
-    a program. It contains all the information that allows the parsing of all
-    the remarks without having any external dependencies.
-
-In order to support multiple modes, the format introduces the concept of a
-bitstream remark container type.
-
-.. _bitstreamremarksseparateremarksmeta:
-
-``SeparateRemarksMeta: the metadata emitted separately``
+.. _bitstreamremarksfileexternal:
+``RemarksFileExternal: a link to an external remarks file``
 
     This container type expects only a :ref:`META_BLOCK <bitstreamremarksmetablock>` containing only:
 
@@ -406,28 +367,32 @@ bitstream remark container type.
     clients to retrieve remarks and their associated metadata directly from
     intermediate products.
 
-``SeparateRemarksFile: the remark entries emitted separately``
+    The container versions of the external separate container should match in order to
+    have a well-formed file.
 
-    This container type expects only a :ref:`META_BLOCK <bitstreamremarksmetablock>` containing only:
+.. _bitstreamremarksfile:
+``RemarksFile: a standalone remarks file``
+
+    This container type expects a :ref:`META_BLOCK <bitstreamremarksmetablock>` containing only:
 
     * :ref:`RECORD_META_CONTAINER_INFO <bitstreamremarksrecordmetacontainerinfo>`
     * :ref:`RECORD_META_REMARK_VERSION <bitstreamremarksrecordmetaremarkversion>`
 
-    This container type expects 0 or more :ref:`REMARK_BLOCK <bitstreamremarksremarkblock>`.
+    This container type expects 1 or more :ref:`REMARK_BLOCK <bitstreamremarksremarkblock>`.
+    If no remarks are emitted, the meta blocks are also not emitted, so the file is empty.
+
+    After the remark blocks, another :ref:`META_BLOCK <bitstreamremarksmetablock>` is emitted, containing:
+    * :ref:`RECORD_META_STRTAB <bitstreamremarksrecordmetastrtab>`
+
+    When the parser reads this container type, it jumps to the end of the file
+    to read the string table before parsing the individual remarks.
 
-    Typically, this is emitted in a side-file alongside an object file, and is
-    made to be able to stream to without increasing the memory consumption of
-    the compiler. This is referenced by the :ref:`RECORD_META_EXTERNAL_FILE
-    <bitstreamremarksrecordmetaexternalfile>` entry in the
-    :ref:`SeparateRemarksMeta <bitstreamremarksseparateremarksmeta>` container.
+    Standalone remarks files can be referenced by the
+    :ref:`RECORD_META_EXTERNAL_FILE <bitstreamremarksrecordmetaexternalfile>`
+    entry in the :ref:`RemarksFileExternal
+    <_bitstreamremarksfileexternal>` container.
 
-When the parser tries to parse a container that contains the metadata for the
-separate remarks, it should parse the version and type, then keep the string
-table in memory while opening the external file, validating its metadata and
-parsing the remark entries.
 
-The container versions from the separate container should match in order to
-have a well-formed file.
 
 ``Standalone: the metadata and the remark entries emitted together``
 
@@ -439,51 +404,7 @@ have a well-formed file.
 
     This container type expects 0 or more :ref:`REMARK_BLOCK <bitstreamremarksremarkblock>`.
 
-A complete output of :program:`llvm-bcanalyzer` on the different container types:
-
-``SeparateRemarksMeta``
-
-.. code-block:: none
-
-    <BLOCKINFO_BLOCK/>
-    <Meta BlockID=8 NumWords=13 BlockCodeSize=3>
-      <Container info codeid=1 abbrevid=4 op0=5 op1=0/>
-      <String table codeid=3 abbrevid=5/> blob data = 'pass\\x00key\\x00value\\x00'
-      <External File codeid=4 abbrevid=6/> blob data = '/path/to/file/name'
-    </Meta>
-
-``SeparateRemarksFile``
-
-.. code-block:: none
-
-    <BLOCKINFO_BLOCK/>
-    <Meta BlockID=8 NumWords=3 BlockCodeSize=3>
-      <Container info codeid=1 abbrevid=4 op0=0 op1=1/>
-      <Remark version codeid=2 abbrevid=5 op0=0/>
-    </Meta>
-    <Remark BlockID=9 NumWords=8 BlockCodeSize=4>
-      <Remark header codeid=5 abbrevid=4 op0=2 op1=0 op2=1 op3=2/>
-      <Remark debug location codeid=6 abbrevid=5 op0=3 op1=99 op2=55/>
-      <Remark hotness codeid=7 abbrevid=6 op0=999999999/>
-      <Argument with debug location codeid=8 abbrevid=7 op0=4 op1=5 op2=6 op3=11 op4=66/>
-    </Remark>
-
-``Standalone``
-
-.. code-block:: none
-
-    <BLOCKINFO_BLOCK/>
-    <Meta BlockID=8 NumWords=15 BlockCodeSize=3>
-      <Container info codeid=1 abbrevid=4 op0=5 op1=2/>
-      <Remark version codeid=2 abbrevid=5 op0=30/>
-      <String table codeid=3 abbrevid=6/> blob data = 'pass\\x00remark\\x00function\\x00path\\x00key\\x00value\\x00argpath\\x00'
-    </Meta>
-    <Remark BlockID=9 NumWords=8 BlockCodeSize=4>
-      <Remark header codeid=5 abbrevid=4 op0=2 op1=1 op2=0 op3=2/>
-      <Remark debug location codeid=6 abbrevid=5 op0=3 op1=99 op2=55/>
-      <Remark hotness codeid=7 abbrevid=6 op0=999999999/>
-      <Argument with debug location codeid=8 abbrevid=7 op0=4 op1=5 op2=6 op3=11 op4=66/>
-    </Remark>
+.. FIXME: Add complete output of :program:`llvm-bcanalyzer` on the different container types (once format changes are completed)
 
 opt-viewer
 ==========

llvm/include/llvm/IR/LLVMRemarkStreamer.h

@@ -82,20 +82,26 @@ struct LLVMRemarkSetupFormatError
       LLVMRemarkSetupFormatError>::LLVMRemarkSetupErrorInfo;
 };
 
-/// Setup optimization remarks that output to a file.
+/// Setup optimization remarks that output to a file. The returned
+/// ToolOutputFile must be kept open for writing until
+/// \ref finalizeLLVMOptimizationRemarks() is called.
 LLVM_ABI Expected<std::unique_ptr<ToolOutputFile>> setupLLVMOptimizationRemarks(
     LLVMContext &Context, StringRef RemarksFilename, StringRef RemarksPasses,
     StringRef RemarksFormat, bool RemarksWithHotness,
     std::optional<uint64_t> RemarksHotnessThreshold = 0);
 
 /// Setup optimization remarks that output directly to a raw_ostream.
-/// \p OS is managed by the caller and should be open for writing as long as \p
-/// Context is streaming remarks to it.
+/// \p OS is managed by the caller and must be open for writing until
+/// \ref finalizeLLVMOptimizationRemarks() is called.
 LLVM_ABI Error setupLLVMOptimizationRemarks(
     LLVMContext &Context, raw_ostream &OS, StringRef RemarksPasses,
     StringRef RemarksFormat, bool RemarksWithHotness,
     std::optional<uint64_t> RemarksHotnessThreshold = 0);
 
+/// Finalize optimization remarks. This must be called before closing the
+/// (file) stream that was used to setup the remarks.
+LLVM_ABI void finalizeLLVMOptimizationRemarks(LLVMContext &Context);
+
 } // end namespace llvm
 
 #endif // LLVM_IR_LLVMREMARKSTREAMER_H

llvm/include/llvm/LTO/LTOBackend.h

@@ -65,8 +65,8 @@ thinBackend(const Config &C, unsigned Task, AddStreamFn AddStream, Module &M,
             AddStreamFn IRAddStream = nullptr,
             const std::vector<uint8_t> &CmdArgs = std::vector<uint8_t>());
 
-LLVM_ABI Error
-finalizeOptimizationRemarks(std::unique_ptr<ToolOutputFile> DiagOutputFile);
+LLVM_ABI Error finalizeOptimizationRemarks(
+    LLVMContext &Context, std::unique_ptr<ToolOutputFile> DiagOutputFile);
 
 /// Returns the BitcodeModule that is ThinLTO.
 LLVM_ABI BitcodeModule *findThinLTOModule(MutableArrayRef<BitcodeModule> BMs);

llvm/include/llvm/Remarks/BitstreamRemarkContainer.h

@@ -23,35 +23,35 @@ namespace remarks {
 
 /// The current version of the remark container.
 /// Note: this is different from the version of the remark entry.
-constexpr uint64_t CurrentContainerVersion = 0;
+constexpr uint64_t CurrentContainerVersion = 1;
 /// The magic number used for identifying remark blocks.
 constexpr StringLiteral ContainerMagic("RMRK");
 
 /// Type of the remark container.
-/// The remark container has two modes:
-/// * separate: the metadata is separate from the remarks and points to the
-///             auxiliary file that contains the remarks.
-/// * standalone: the metadata and the remarks are emitted together.
 enum class BitstreamRemarkContainerType {
-  /// The metadata emitted separately.
-  /// This will contain the following:
-  /// * Container version and type
-  /// * String table
-  /// * External file
-  SeparateRemarksMeta,
-  /// The remarks emitted separately.
-  /// This will contain the following:
-  /// * Container version and type
-  /// * Remark version
-  SeparateRemarksFile,
-  /// Everything is emitted together.
-  /// This will contain the following:
-  /// * Container version and type
-  /// * Remark version
-  /// * String table
-  Standalone,
-  First = SeparateRemarksMeta,
-  Last = Standalone,
+  /// Emit a link to an external remarks file
+  /// (usually as a section of the object file, to enable discovery of all
+  /// remarks files from the final linked object file)
+  /// RemarksFileExternal:
+  ///   | Meta:
+  ///   | | Container info
+  ///   | | External file
+  RemarksFileExternal,
+  /// Emit metadata and remarks into a file
+  /// RemarksFile:
+  ///   | Meta:
+  ///   | | Container info
+  ///   | | Remark version
+  ///   | Remarks:
+  ///   | | Remark0
+  ///   | | Remark1
+  ///   | | Remark2
+  ///   | | ...
+  ///   | Late Meta:
+  ///   | | String table
+  RemarksFile,
+  First = RemarksFileExternal,
+  Last = RemarksFile
 };
 
 /// The possible blocks that will be encountered in a bitstream remark
@@ -67,8 +67,8 @@ enum BlockIDs {
   REMARK_BLOCK_ID
 };
 
-constexpr StringRef MetaBlockName = StringRef("Meta", 4);
-constexpr StringRef RemarkBlockName = StringRef("Remark", 6);
+constexpr StringLiteral MetaBlockName("Meta");
+constexpr StringLiteral RemarkBlockName("Remark");
 
 /// The possible records that can be encountered in the previously described
 /// blocks.
@@ -89,16 +89,16 @@ enum RecordIDs {
   RECORD_LAST = RECORD_REMARK_ARG_WITHOUT_DEBUGLOC
 };
 
-constexpr StringRef MetaContainerInfoName = StringRef("Container info", 14);
-constexpr StringRef MetaRemarkVersionName = StringRef("Remark version", 14);
-constexpr StringRef MetaStrTabName = StringRef("String table", 12);
-constexpr StringRef MetaExternalFileName = StringRef("External File", 13);
-constexpr StringRef RemarkHeaderName = StringRef("Remark header", 13);
-constexpr StringRef RemarkDebugLocName = StringRef("Remark debug location", 21);
-constexpr StringRef RemarkHotnessName = StringRef("Remark hotness", 14);
-constexpr StringRef RemarkArgWithDebugLocName =
-    StringRef("Argument with debug location", 28);
-constexpr StringRef RemarkArgWithoutDebugLocName = StringRef("Argument", 8);
+constexpr StringLiteral MetaContainerInfoName("Container info");
+constexpr StringLiteral MetaRemarkVersionName("Remark version");
+constexpr StringLiteral MetaStrTabName("String table");
+constexpr StringLiteral MetaExternalFileName("External File");
+constexpr StringLiteral RemarkHeaderName("Remark header");
+constexpr StringLiteral RemarkDebugLocName("Remark debug location");
+constexpr StringLiteral RemarkHotnessName("Remark hotness");
+constexpr StringLiteral
+    RemarkArgWithDebugLocName("Argument with debug location");
+constexpr StringLiteral RemarkArgWithoutDebugLocName("Argument");
 
 } // end namespace remarks
 } // end namespace llvm