diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 77bf0c8251fc2..106363fa83e2b 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -29,6 +29,7 @@ #include "llvm/Frontend/Driver/CodeGenOptions.h" #include "llvm/IR/DataLayout.h" #include "llvm/IR/DebugInfo.h" +#include "llvm/IR/LLVMRemarkStreamer.h" #include "llvm/IR/LegacyPassManager.h" #include "llvm/IR/Module.h" #include "llvm/IR/ModuleSummaryIndex.h" @@ -1384,6 +1385,10 @@ runThinLTOBackend(CompilerInstance &CI, ModuleSummaryIndex *CombinedIndex, Conf.CGFileType = getCodeGenFileType(Action); break; } + + // FIXME: Both ExecuteAction and thinBackend set up optimization remarks for + // the same context. + finalizeLLVMOptimizationRemarks(M->getContext()); if (Error E = thinBackend(Conf, -1, AddStream, *M, *CombinedIndex, ImportList, ModuleToDefinedGVSummaries[M->getModuleIdentifier()], diff --git a/clang/lib/CodeGen/CodeGenAction.cpp b/clang/lib/CodeGen/CodeGenAction.cpp index dc54c97eeae8e..8e3234998df2a 100644 --- a/clang/lib/CodeGen/CodeGenAction.cpp +++ b/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" @@ -259,19 +260,18 @@ void BackendConsumer::HandleTranslationUnit(ASTContext &C) { Ctx.setDefaultTargetCPU(TargetOpts.CPU); Ctx.setDefaultTargetFeatures(llvm::join(TargetOpts.Features, ",")); - Expected> OptRecordFileOrErr = - setupLLVMOptimizationRemarks( - Ctx, CodeGenOpts.OptRecordFile, CodeGenOpts.OptRecordPasses, - CodeGenOpts.OptRecordFormat, CodeGenOpts.DiagnosticsWithHotness, - CodeGenOpts.DiagnosticsHotnessThreshold); + Expected OptRecordFileOrErr = + setupLLVMOptimizationRemarks( + Ctx, CodeGenOpts.OptRecordFile, CodeGenOpts.OptRecordPasses, + CodeGenOpts.OptRecordFormat, CodeGenOpts.DiagnosticsWithHotness, + CodeGenOpts.DiagnosticsHotnessThreshold); if (Error E = OptRecordFileOrErr.takeError()) { reportOptRecordError(std::move(E), Diags, CodeGenOpts); return; } - std::unique_ptr OptRecordFile = - std::move(*OptRecordFileOrErr); + LLVMRemarkFileHandle OptRecordFile = std::move(*OptRecordFileOrErr); if (OptRecordFile && CodeGenOpts.getProfileUse() != llvm::driver::ProfileInstrKind::ProfileNone) @@ -1173,7 +1173,7 @@ void CodeGenAction::ExecuteAction() { Ctx.setDefaultTargetCPU(TargetOpts.CPU); Ctx.setDefaultTargetFeatures(llvm::join(TargetOpts.Features, ",")); - Expected> OptRecordFileOrErr = + Expected OptRecordFileOrErr = setupLLVMOptimizationRemarks( Ctx, CodeGenOpts.OptRecordFile, CodeGenOpts.OptRecordPasses, CodeGenOpts.OptRecordFormat, CodeGenOpts.DiagnosticsWithHotness, @@ -1183,8 +1183,7 @@ void CodeGenAction::ExecuteAction() { reportOptRecordError(std::move(E), Diagnostics, CodeGenOpts); return; } - std::unique_ptr OptRecordFile = - std::move(*OptRecordFileOrErr); + LLVMRemarkFileHandle OptRecordFile = std::move(*OptRecordFileOrErr); emitBackendOutput(CI, CI.getCodeGenOpts(), CI.getTarget().getDataLayoutString(), TheModule.get(), BA, diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp index c3c53d51015a2..db6b98998785c 100644 --- a/flang/lib/Frontend/FrontendActions.cpp +++ b/flang/lib/Frontend/FrontendActions.cpp @@ -1352,7 +1352,7 @@ void CodeGenAction::executeAction() { std::make_unique(remarkConsumer)); // write optimization-record - llvm::Expected> optRecordFileOrErr = + llvm::Expected optRecordFileOrErr = setupLLVMOptimizationRemarks( llvmModule->getContext(), codeGenOpts.OptRecordFile, codeGenOpts.OptRecordPasses, codeGenOpts.OptRecordFormat, @@ -1364,8 +1364,7 @@ void CodeGenAction::executeAction() { return; } - std::unique_ptr optRecordFile = - std::move(*optRecordFileOrErr); + llvm::LLVMRemarkFileHandle optRecordFile = std::move(*optRecordFileOrErr); if (optRecordFile) { optRecordFile->keep(); diff --git a/llvm/docs/Remarks.rst b/llvm/docs/Remarks.rst index c89940f9ff4d5..67ed94d9740f4 100644 --- a/llvm/docs/Remarks.rst +++ b/llvm/docs/Remarks.rst @@ -152,26 +152,6 @@ Other tools that support remarks: .. option:: -opt-remarks-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 +354,11 @@ 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 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. +The bitstream remark container supports multiple types: -.. _bitstreamremarksseparateremarksmeta: +.. _bitstreamremarksfileexternal: -``SeparateRemarksMeta: the metadata emitted separately`` +``RemarksFileExternal: a link to an external remarks file`` This container type expects only a :ref:`META_BLOCK ` containing only: @@ -406,84 +370,33 @@ 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 ` containing only: - - * :ref:`RECORD_META_CONTAINER_INFO ` - * :ref:`RECORD_META_REMARK_VERSION ` +.. _bitstreamremarksfile: - This container type expects 0 or more :ref:`REMARK_BLOCK `. +``RemarksFile: a standalone remarks file`` - 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 - ` entry in the - :ref:`SeparateRemarksMeta ` container. + This container type expects a :ref:`META_BLOCK ` containing only: -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. + * :ref:`RECORD_META_CONTAINER_INFO ` + * :ref:`RECORD_META_REMARK_VERSION ` -The container versions from the separate container should match in order to -have a well-formed file. + Then, this container type expects 1 or more :ref:`REMARK_BLOCK `. + If no remarks are emitted, the meta blocks are also not emitted, so the file is empty. -``Standalone: the metadata and the remark entries emitted together`` + After the remark blocks, another :ref:`META_BLOCK ` is emitted, containing: + * :ref:`RECORD_META_STRTAB ` - This container type expects only a :ref:`META_BLOCK ` containing only: + 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. - * :ref:`RECORD_META_CONTAINER_INFO ` - * :ref:`RECORD_META_REMARK_VERSION ` - * :ref:`RECORD_META_STRTAB ` + Standalone remarks files can be referenced by the + :ref:`RECORD_META_EXTERNAL_FILE ` + entry in the :ref:`RemarksFileExternal + ` container. - This container type expects 0 or more :ref:`REMARK_BLOCK `. - -A complete output of :program:`llvm-bcanalyzer` on the different container types: - -``SeparateRemarksMeta`` - -.. code-block:: none - - - - - blob data = 'pass\\x00key\\x00value\\x00' - blob data = '/path/to/file/name' - - -``SeparateRemarksFile`` - -.. code-block:: none - - - - - - - - - - - - - -``Standalone`` - -.. code-block:: none - - - - - - blob data = 'pass\\x00remark\\x00function\\x00path\\x00key\\x00value\\x00argpath\\x00' - - - - - - - +.. FIXME: Add complete output of :program:`llvm-bcanalyzer` on the different container types (once format changes are completed) opt-viewer ========== diff --git a/llvm/include/llvm/IR/LLVMRemarkStreamer.h b/llvm/include/llvm/IR/LLVMRemarkStreamer.h index 376acdec49fbb..96cccebf0d70e 100644 --- a/llvm/include/llvm/IR/LLVMRemarkStreamer.h +++ b/llvm/include/llvm/IR/LLVMRemarkStreamer.h @@ -17,6 +17,7 @@ #include "llvm/Remarks/Remark.h" #include "llvm/Support/Compiler.h" #include "llvm/Support/Error.h" +#include "llvm/Support/ToolOutputFile.h" #include #include #include @@ -82,20 +83,81 @@ struct LLVMRemarkSetupFormatError LLVMRemarkSetupFormatError>::LLVMRemarkSetupErrorInfo; }; -/// Setup optimization remarks that output to a file. -LLVM_ABI Expected> setupLLVMOptimizationRemarks( +/// RAII handle that manages the lifetime of the ToolOutputFile used to output +/// remarks. On destruction (or when calling releaseFile()), this handle ensures +/// that the optimization remarks are finalized and the RemarkStreamer is +/// correctly deregistered from the LLVMContext. +class LLVMRemarkFileHandle final { + struct Finalizer { + LLVMContext *Context; + + Finalizer(LLVMContext *Ctx) : Context(Ctx) {} + + Finalizer(const Finalizer &) = delete; + Finalizer &operator=(const Finalizer &) = delete; + + Finalizer(Finalizer &&Other) : Context(Other.Context) { + Other.Context = nullptr; + } + + Finalizer &operator=(Finalizer &&Other) { + std::swap(Context, Other.Context); + return *this; + } + + ~Finalizer() { finalize(); } + + LLVM_ABI void finalize(); + }; + + std::unique_ptr OutputFile; + Finalizer Finalize; + +public: + LLVMRemarkFileHandle() : OutputFile(nullptr), Finalize(nullptr) {} + + LLVMRemarkFileHandle(std::unique_ptr OutputFile, + LLVMContext &Ctx) + : OutputFile(std::move(OutputFile)), Finalize(&Ctx) {} + + ToolOutputFile *get() { return OutputFile.get(); } + explicit operator bool() { return bool(OutputFile); } + + /// Finalize remark emission and release the underlying ToolOutputFile. + std::unique_ptr releaseFile() { + finalize(); + return std::move(OutputFile); + } + + void finalize() { Finalize.finalize(); } + + ToolOutputFile &operator*() { return *OutputFile; } + ToolOutputFile *operator->() { return &*OutputFile; } +}; + +/// Set up optimization remarks that output to a file. The LLVMRemarkFileHandle +/// manages the lifetime of the underlying ToolOutputFile to ensure \ref +/// finalizeLLVMOptimizationRemarks() is called before the file is destroyed or +/// released from the handle. The handle must be kept alive until all remarks +/// were emitted through the remark streamer. +LLVM_ABI Expected setupLLVMOptimizationRemarks( LLVMContext &Context, StringRef RemarksFilename, StringRef RemarksPasses, StringRef RemarksFormat, bool RemarksWithHotness, std::optional 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. +/// Set up optimization remarks that output directly to a raw_ostream. +/// \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 RemarksHotnessThreshold = 0); +/// Finalize optimization remarks and deregister the RemarkStreamer from the \p +/// Context. This must be called before closing the (file) stream that was used +/// to set up the remarks. +LLVM_ABI void finalizeLLVMOptimizationRemarks(LLVMContext &Context); + } // end namespace llvm #endif // LLVM_IR_LLVMREMARKSTREAMER_H diff --git a/llvm/include/llvm/LTO/LTO.h b/llvm/include/llvm/LTO/LTO.h index 323c478691a92..3a9a7f7c25859 100644 --- a/llvm/include/llvm/LTO/LTO.h +++ b/llvm/include/llvm/LTO/LTO.h @@ -15,6 +15,7 @@ #ifndef LLVM_LTO_LTO_H #define LLVM_LTO_LTO_H +#include "llvm/IR/LLVMRemarkStreamer.h" #include "llvm/Support/Compiler.h" #include @@ -91,7 +92,7 @@ LLVM_ABI std::string getThinLTOOutputFile(StringRef Path, StringRef OldPrefix, StringRef NewPrefix); /// Setup optimization remarks. -LLVM_ABI Expected> setupLLVMOptimizationRemarks( +LLVM_ABI Expected setupLLVMOptimizationRemarks( LLVMContext &Context, StringRef RemarksFilename, StringRef RemarksPasses, StringRef RemarksFormat, bool RemarksWithHotness, std::optional RemarksHotnessThreshold = 0, int Count = -1); @@ -579,7 +580,7 @@ class LTO { DenseSet DynamicExportSymbols; // Diagnostic optimization remarks file - std::unique_ptr DiagnosticOutputFile; + LLVMRemarkFileHandle DiagnosticOutputFile; }; /// The resolution for a symbol. The linker must provide a SymbolResolution for diff --git a/llvm/include/llvm/LTO/LTOBackend.h b/llvm/include/llvm/LTO/LTOBackend.h index 86b488c764e06..48ad5aa64f61f 100644 --- a/llvm/include/llvm/LTO/LTOBackend.h +++ b/llvm/include/llvm/LTO/LTOBackend.h @@ -65,8 +65,7 @@ thinBackend(const Config &C, unsigned Task, AddStreamFn AddStream, Module &M, AddStreamFn IRAddStream = nullptr, const std::vector &CmdArgs = std::vector()); -LLVM_ABI Error -finalizeOptimizationRemarks(std::unique_ptr DiagOutputFile); +LLVM_ABI Error finalizeOptimizationRemarks(LLVMRemarkFileHandle DiagOutputFile); /// Returns the BitcodeModule that is ThinLTO. LLVM_ABI BitcodeModule *findThinLTOModule(MutableArrayRef BMs); diff --git a/llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h b/llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h index 806d3c5bdfd77..caff198358caa 100644 --- a/llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h +++ b/llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h @@ -244,7 +244,7 @@ struct LTOCodeGenerator { bool ShouldInternalize = EnableLTOInternalization; bool ShouldEmbedUselists = false; bool ShouldRestoreGlobalsLinkage = false; - std::unique_ptr DiagnosticOutputFile; + LLVMRemarkFileHandle DiagnosticOutputFile; std::unique_ptr StatsFile = nullptr; std::string SaveIRBeforeOptPath; diff --git a/llvm/include/llvm/Remarks/BitstreamRemarkContainer.h b/llvm/include/llvm/Remarks/BitstreamRemarkContainer.h index 48a148a3adc13..d4b70e54bf6bc 100644 --- a/llvm/include/llvm/Remarks/BitstreamRemarkContainer.h +++ b/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 diff --git a/llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h b/llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h index 6236800337508..76e2d5b4fd3bc 100644 --- a/llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h +++ b/llvm/include/llvm/Remarks/BitstreamRemarkSerializer.h @@ -27,31 +27,7 @@ struct Remarks; /// Serialize the remarks to LLVM bitstream. /// This class provides ways to emit remarks in the LLVM bitstream format and /// its associated metadata. -/// -/// * The separate model: -/// Separate meta: | Container info -/// | String table -/// | External file -/// -/// Separate remarks: | Container info -/// | Remark version -/// | Remark0 -/// | Remark1 -/// | Remark2 -/// | ... -/// -/// * The standalone model: | Container info -/// | String table -/// | Remark version -/// | Remark0 -/// | Remark1 -/// | Remark2 -/// | ... -/// struct BitstreamRemarkSerializerHelper { - /// Buffer used for encoding the bitstream before writing it to the final - /// stream. - SmallVector Encoded; /// Buffer used to construct records and pass to the bitstream writer. SmallVector R; /// The Bitstream writer. @@ -73,7 +49,8 @@ struct BitstreamRemarkSerializerHelper { uint64_t RecordRemarkArgWithDebugLocAbbrevID = 0; uint64_t RecordRemarkArgWithoutDebugLocAbbrevID = 0; - BitstreamRemarkSerializerHelper(BitstreamRemarkContainerType ContainerType); + BitstreamRemarkSerializerHelper(BitstreamRemarkContainerType ContainerType, + raw_ostream &OS); // Disable copy and move: Bitstream points to Encoded, which needs special // handling during copy/move, but moving the vectors is probably useless @@ -104,20 +81,15 @@ struct BitstreamRemarkSerializerHelper { /// The block info for the remarks block. void setupRemarkBlockInfo(); - /// Emit the metadata for the remarks. - void emitMetaBlock(uint64_t ContainerVersion, - std::optional RemarkVersion, - std::optional StrTab = std::nullopt, - std::optional Filename = std::nullopt); + /// Emit the main metadata at the beginning of the file + void emitMetaBlock(std::optional Filename = std::nullopt); + + /// Emit the remaining metadata at the end of the file. Here we emit metadata + /// that is only known once all remarks were emitted. + void emitLateMetaBlock(const StringTable &StrTab); /// Emit a remark block. The string table is required. - void emitRemarkBlock(const Remark &Remark, StringTable &StrTab); - /// Finalize the writing to \p OS. - void flushToStream(raw_ostream &OS); - /// Finalize the writing to a buffer. - /// The contents of the buffer remain valid for the lifetime of the object. - /// Any call to any other function in this class will invalidate the buffer. - StringRef getBuffer(); + void emitRemark(const Remark &Remark, StringTable &StrTab); }; /// Implementation of the remark serializer using LLVM bitstream. @@ -127,68 +99,57 @@ struct BitstreamRemarkSerializer : public RemarkSerializer { /// 2) The metadata block that contains various information about the remarks /// in the file. /// 3) A number of remark blocks. + /// 4) Another metadata block for metadata that is only finalized once all + /// remarks were emitted (e.g. StrTab) - /// We need to set up 1) and 2) first, so that we can emit 3) after. This flag - /// is used to emit the first two blocks only once. - bool DidSetUp = false; - /// The helper to emit bitstream. - BitstreamRemarkSerializerHelper Helper; + /// The helper to emit bitstream. This is nullopt when the Serializer has not + /// been setup yet. + std::optional Helper; /// Construct a serializer that will create its own string table. - BitstreamRemarkSerializer(raw_ostream &OS, SerializerMode Mode); + BitstreamRemarkSerializer(raw_ostream &OS); /// Construct a serializer with a pre-filled string table. - BitstreamRemarkSerializer(raw_ostream &OS, SerializerMode Mode, - StringTable StrTab); + BitstreamRemarkSerializer(raw_ostream &OS, StringTable StrTab); + + ~BitstreamRemarkSerializer() override; /// Emit a remark to the stream. This also emits the metadata associated to - /// the remarks based on the SerializerMode specified at construction. - /// This writes the serialized output to the provided stream. + /// the remarks. This writes the serialized output to the provided stream. void emit(const Remark &Remark) override; + + /// Finalize emission of remarks. This emits the late metadata block and + /// flushes internal buffers. It is safe to call this function multiple times, + /// and it is automatically executed on destruction of the Serializer. + void finalize() override; + /// The metadata serializer associated to this remark serializer. Based on the /// container type of the current serializer, the container type of the /// metadata serializer will change. - std::unique_ptr metaSerializer( - raw_ostream &OS, - std::optional ExternalFilename = std::nullopt) override; + std::unique_ptr + metaSerializer(raw_ostream &OS, StringRef ExternalFilename) override; static bool classof(const RemarkSerializer *S) { return S->SerializerFormat == Format::Bitstream; } + +private: + void setup(); }; /// Serializer of metadata for bitstream remarks. struct BitstreamMetaSerializer : public MetaSerializer { - /// This class can be used with [1] a pre-constructed - /// BitstreamRemarkSerializerHelper, or with [2] one that is owned by the meta - /// serializer. In case of [1], we need to be able to store a reference to the - /// object, while in case of [2] we need to store the whole object. - std::optional TmpHelper; - /// The actual helper, that can point to \p TmpHelper or to an external helper - /// object. - BitstreamRemarkSerializerHelper *Helper = nullptr; - - std::optional StrTab; - std::optional ExternalFilename; + std::optional Helper; + + StringRef ExternalFilename; /// Create a new meta serializer based on \p ContainerType. - BitstreamMetaSerializer( - raw_ostream &OS, BitstreamRemarkContainerType ContainerType, - std::optional StrTab = std::nullopt, - std::optional ExternalFilename = std::nullopt) - : MetaSerializer(OS), TmpHelper(std::nullopt), Helper(nullptr), - StrTab(StrTab), ExternalFilename(ExternalFilename) { - TmpHelper.emplace(ContainerType); - Helper = &*TmpHelper; + BitstreamMetaSerializer(raw_ostream &OS, + BitstreamRemarkContainerType ContainerType, + StringRef ExternalFilename) + : MetaSerializer(OS), ExternalFilename(ExternalFilename) { + Helper.emplace(ContainerType, OS); } - /// Create a new meta serializer based on a previously built \p Helper. - BitstreamMetaSerializer( - raw_ostream &OS, BitstreamRemarkSerializerHelper &Helper, - std::optional StrTab = std::nullopt, - std::optional ExternalFilename = std::nullopt) - : MetaSerializer(OS), TmpHelper(std::nullopt), Helper(&Helper), - StrTab(StrTab), ExternalFilename(ExternalFilename) {} - void emit() override; }; diff --git a/llvm/include/llvm/Remarks/RemarkSerializer.h b/llvm/include/llvm/Remarks/RemarkSerializer.h index 05ef14ae5566b..1785152b87c70 100644 --- a/llvm/include/llvm/Remarks/RemarkSerializer.h +++ b/llvm/include/llvm/Remarks/RemarkSerializer.h @@ -26,16 +26,6 @@ namespace remarks { struct Remark; -enum class SerializerMode { - Separate, // A mode where the metadata is serialized separately from the - // remarks. Typically, this is used when the remarks need to be - // streamed to a side file and the metadata is embedded into the - // final result of the compilation. - Standalone // A mode where everything can be retrieved in the same - // file/buffer. Typically, this is used for storing remarks for - // later use. -}; - struct MetaSerializer; /// This is the base class for a remark serializer. @@ -45,24 +35,27 @@ struct RemarkSerializer { Format SerializerFormat; /// The open raw_ostream that the remark diagnostics are emitted to. raw_ostream &OS; - /// The serialization mode. - SerializerMode Mode; /// The string table containing all the unique strings used in the output. /// The table can be serialized to be consumed after the compilation. std::optional StrTab; - RemarkSerializer(Format SerializerFormat, raw_ostream &OS, - SerializerMode Mode) - : SerializerFormat(SerializerFormat), OS(OS), Mode(Mode) {} + RemarkSerializer(Format SerializerFormat, raw_ostream &OS) + : SerializerFormat(SerializerFormat), OS(OS) {} - /// This is just an interface. virtual ~RemarkSerializer() = default; + + /// Finalize remark emission (e.g. finish writing metadata, flush internal + /// buffers). It is safe to call this function multiple times, and it should + /// have the same behavior as destructing the RemarkSerializer. + /// After finalizing, the behavior of emit is unspecified. + virtual void finalize() {} + /// Emit a remark to the stream. virtual void emit(const Remark &Remark) = 0; + /// Return the corresponding metadata serializer. virtual std::unique_ptr - metaSerializer(raw_ostream &OS, - std::optional ExternalFilename = std::nullopt) = 0; + metaSerializer(raw_ostream &OS, StringRef ExternalFilename) = 0; }; /// This is the base class for a remark metadata serializer. @@ -79,13 +72,12 @@ struct MetaSerializer { /// Create a remark serializer. LLVM_ABI Expected> -createRemarkSerializer(Format RemarksFormat, SerializerMode Mode, - raw_ostream &OS); +createRemarkSerializer(Format RemarksFormat, raw_ostream &OS); /// Create a remark serializer that uses a pre-filled string table. LLVM_ABI Expected> -createRemarkSerializer(Format RemarksFormat, SerializerMode Mode, - raw_ostream &OS, remarks::StringTable StrTab); +createRemarkSerializer(Format RemarksFormat, raw_ostream &OS, + remarks::StringTable StrTab); } // end namespace remarks } // end namespace llvm diff --git a/llvm/include/llvm/Remarks/RemarkStreamer.h b/llvm/include/llvm/Remarks/RemarkStreamer.h index 5b1cc81cdbf50..dd5bfcbc7ff99 100644 --- a/llvm/include/llvm/Remarks/RemarkStreamer.h +++ b/llvm/include/llvm/Remarks/RemarkStreamer.h @@ -52,6 +52,7 @@ class RemarkStreamer final { public: RemarkStreamer(std::unique_ptr RemarkSerializer, std::optional Filename = std::nullopt); + ~RemarkStreamer(); /// Return the filename that the remark diagnostics are emitted to. std::optional getFilename() const { @@ -61,6 +62,14 @@ class RemarkStreamer final { raw_ostream &getStream() { return RemarkSerializer->OS; } /// Return the serializer used for this stream. remarks::RemarkSerializer &getSerializer() { return *RemarkSerializer; } + + /// Release the underlying RemarkSerializer. Destructing the RemarkStreamer + /// will assert that the RemarkStreamer has been released, to ensure that the + /// remarks were properly finalized. + std::unique_ptr releaseSerializer() { + return std::move(RemarkSerializer); + } + /// Set a pass filter based on a regex \p Filter. /// Returns an error if the regex is invalid. Error setFilter(StringRef Filter); diff --git a/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h b/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h index d80464c0fe74a..69b8f9f000e1d 100644 --- a/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h +++ b/llvm/include/llvm/Remarks/YAMLRemarkSerializer.h @@ -36,28 +36,22 @@ struct LLVM_ABI YAMLRemarkSerializer : public RemarkSerializer { /// The YAML streamer. yaml::Output YAMLOutput; - YAMLRemarkSerializer(raw_ostream &OS, SerializerMode Mode, - std::optional StrTab = std::nullopt); + YAMLRemarkSerializer(raw_ostream &OS); + YAMLRemarkSerializer(raw_ostream &OS, StringTable StrTabIn); void emit(const Remark &Remark) override; - std::unique_ptr metaSerializer( - raw_ostream &OS, - std::optional ExternalFilename = std::nullopt) override; + std::unique_ptr + metaSerializer(raw_ostream &OS, StringRef ExternalFilename) override; static bool classof(const RemarkSerializer *S) { return S->SerializerFormat == Format::YAML; } - -protected: - YAMLRemarkSerializer(Format SerializerFormat, raw_ostream &OS, - SerializerMode Mode, - std::optional StrTab = std::nullopt); }; struct LLVM_ABI YAMLMetaSerializer : public MetaSerializer { - std::optional ExternalFilename; + StringRef ExternalFilename; - YAMLMetaSerializer(raw_ostream &OS, std::optional ExternalFilename) + YAMLMetaSerializer(raw_ostream &OS, StringRef ExternalFilename) : MetaSerializer(OS), ExternalFilename(ExternalFilename) {} void emit() override; diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp index cd14a4f57f760..57fdc8b7c5ffc 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp @@ -78,6 +78,7 @@ #include "llvm/IR/GlobalValue.h" #include "llvm/IR/GlobalVariable.h" #include "llvm/IR/Instruction.h" +#include "llvm/IR/LLVMRemarkStreamer.h" #include "llvm/IR/Mangler.h" #include "llvm/IR/Metadata.h" #include "llvm/IR/Module.h" @@ -2508,6 +2509,8 @@ void AsmPrinter::emitGlobalIFunc(Module &M, const GlobalIFunc &GI) { void AsmPrinter::emitRemarksSection(remarks::RemarkStreamer &RS) { if (!RS.needsSection()) return; + if (!RS.getFilename()) + return; MCSection *RemarksSection = OutContext.getObjectFileInfo()->getRemarksSection(); @@ -2518,20 +2521,16 @@ void AsmPrinter::emitRemarksSection(remarks::RemarkStreamer &RS) { return; } - remarks::RemarkSerializer &RemarkSerializer = RS.getSerializer(); - - std::optional> Filename; - if (std::optional FilenameRef = RS.getFilename()) { - Filename = *FilenameRef; - sys::fs::make_absolute(*Filename); - assert(!Filename->empty() && "The filename can't be empty."); - } + SmallString<128> Filename = *RS.getFilename(); + sys::fs::make_absolute(Filename); + assert(!Filename.empty() && "The filename can't be empty."); std::string Buf; raw_string_ostream OS(Buf); + + remarks::RemarkSerializer &RemarkSerializer = RS.getSerializer(); std::unique_ptr MetaSerializer = - Filename ? RemarkSerializer.metaSerializer(OS, Filename->str()) - : RemarkSerializer.metaSerializer(OS); + RemarkSerializer.metaSerializer(OS, Filename); MetaSerializer->emit(); // Switch to the remarks section. diff --git a/llvm/lib/IR/LLVMRemarkStreamer.cpp b/llvm/lib/IR/LLVMRemarkStreamer.cpp index 71f8d4a4b1c7c..9e1e45998f2f1 100644 --- a/llvm/lib/IR/LLVMRemarkStreamer.cpp +++ b/llvm/lib/IR/LLVMRemarkStreamer.cpp @@ -92,7 +92,7 @@ char LLVMRemarkSetupFileError::ID = 0; char LLVMRemarkSetupPatternError::ID = 0; char LLVMRemarkSetupFormatError::ID = 0; -Expected> llvm::setupLLVMOptimizationRemarks( +Expected llvm::setupLLVMOptimizationRemarks( LLVMContext &Context, StringRef RemarksFilename, StringRef RemarksPasses, StringRef RemarksFormat, bool RemarksWithHotness, std::optional RemarksHotnessThreshold) { @@ -102,7 +102,7 @@ Expected> llvm::setupLLVMOptimizationRemarks( Context.setDiagnosticsHotnessThreshold(RemarksHotnessThreshold); if (RemarksFilename.empty()) - return nullptr; + return LLVMRemarkFileHandle(); Expected Format = remarks::parseFormat(RemarksFormat); if (Error E = Format.takeError()) @@ -119,24 +119,35 @@ Expected> llvm::setupLLVMOptimizationRemarks( return make_error(errorCodeToError(EC)); Expected> RemarkSerializer = - remarks::createRemarkSerializer( - *Format, remarks::SerializerMode::Separate, RemarksFile->os()); + remarks::createRemarkSerializer(*Format, RemarksFile->os()); if (Error E = RemarkSerializer.takeError()) return make_error(std::move(E)); - // Create the main remark streamer. - Context.setMainRemarkStreamer(std::make_unique( - std::move(*RemarkSerializer), RemarksFilename)); + auto RS = std::make_unique( + std::move(*RemarkSerializer), RemarksFilename); + + if (!RemarksPasses.empty()) + if (Error E = RS->setFilter(RemarksPasses)) { + RS->releaseSerializer(); + return make_error(std::move(E)); + } + + // Install the main remark streamer. Only install this after setting the + // filter, because this might fail. + Context.setMainRemarkStreamer(std::move(RS)); // Create LLVM's optimization remarks streamer. Context.setLLVMRemarkStreamer( std::make_unique(*Context.getMainRemarkStreamer())); - if (!RemarksPasses.empty()) - if (Error E = Context.getMainRemarkStreamer()->setFilter(RemarksPasses)) - return make_error(std::move(E)); + return LLVMRemarkFileHandle{std::move(RemarksFile), Context}; +} - return std::move(RemarksFile); +void LLVMRemarkFileHandle::Finalizer::finalize() { + if (!Context) + return; + finalizeLLVMOptimizationRemarks(*Context); + Context = nullptr; } Error llvm::setupLLVMOptimizationRemarks( @@ -153,22 +164,34 @@ Error llvm::setupLLVMOptimizationRemarks( return make_error(std::move(E)); Expected> RemarkSerializer = - remarks::createRemarkSerializer(*Format, - remarks::SerializerMode::Separate, OS); + remarks::createRemarkSerializer(*Format, OS); if (Error E = RemarkSerializer.takeError()) return make_error(std::move(E)); - // Create the main remark streamer. - Context.setMainRemarkStreamer( - std::make_unique(std::move(*RemarkSerializer))); + auto RS = + std::make_unique(std::move(*RemarkSerializer)); + + if (!RemarksPasses.empty()) + if (Error E = RS->setFilter(RemarksPasses)) { + RS->releaseSerializer(); + return make_error(std::move(E)); + } + + // Install the main remark streamer. Only install this after setting the + // filter, because this might fail. + Context.setMainRemarkStreamer(std::move(RS)); // Create LLVM's optimization remarks streamer. Context.setLLVMRemarkStreamer( std::make_unique(*Context.getMainRemarkStreamer())); - if (!RemarksPasses.empty()) - if (Error E = Context.getMainRemarkStreamer()->setFilter(RemarksPasses)) - return make_error(std::move(E)); - return Error::success(); } + +void llvm::finalizeLLVMOptimizationRemarks(LLVMContext &Context) { + Context.setLLVMRemarkStreamer(nullptr); + if (auto *RS = Context.getMainRemarkStreamer()) { + RS->releaseSerializer(); + Context.setMainRemarkStreamer(nullptr); + } +} diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index ce9ecc35e1922..7b252627d73f9 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -1290,11 +1290,11 @@ void lto::updateMemProfAttributes(Module &Mod, Error LTO::runRegularLTO(AddStreamFn AddStream) { llvm::TimeTraceScope timeScope("Run regular LTO"); + LLVMContext &CombinedCtx = RegularLTO.CombinedModule->getContext(); // Setup optimization remarks. auto DiagFileOrErr = lto::setupLLVMOptimizationRemarks( - RegularLTO.CombinedModule->getContext(), Conf.RemarksFilename, - Conf.RemarksPasses, Conf.RemarksFormat, Conf.RemarksWithHotness, - Conf.RemarksHotnessThreshold); + CombinedCtx, Conf.RemarksFilename, Conf.RemarksPasses, Conf.RemarksFormat, + Conf.RemarksWithHotness, Conf.RemarksHotnessThreshold); LLVM_DEBUG(dbgs() << "Running regular LTO\n"); if (!DiagFileOrErr) return DiagFileOrErr.takeError(); @@ -2177,7 +2177,7 @@ Error LTO::runThinLTO(AddStreamFn AddStream, FileCache Cache, return RunBackends(SecondRoundLTO.get()); } -Expected> lto::setupLLVMOptimizationRemarks( +Expected lto::setupLLVMOptimizationRemarks( LLVMContext &Context, StringRef RemarksFilename, StringRef RemarksPasses, StringRef RemarksFormat, bool RemarksWithHotness, std::optional RemarksHotnessThreshold, int Count) { diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp index ce42fc526beac..c126e8efe82b3 100644 --- a/llvm/lib/LTO/LTOBackend.cpp +++ b/llvm/lib/LTO/LTOBackend.cpp @@ -540,12 +540,12 @@ static Expected initAndLookupTarget(const Config &C, return T; } -Error lto::finalizeOptimizationRemarks( - std::unique_ptr DiagOutputFile) { +Error lto::finalizeOptimizationRemarks(LLVMRemarkFileHandle DiagOutputFile) { // Make sure we flush the diagnostic remarks file in case the linker doesn't // call the global destructors before exiting. if (!DiagOutputFile) return Error::success(); + DiagOutputFile.finalize(); DiagOutputFile->keep(); DiagOutputFile->os().flush(); return Error::success(); @@ -640,7 +640,7 @@ Error lto::thinBackend(const Config &Conf, unsigned Task, AddStreamFn AddStream, auto OptimizeAndCodegen = [&](Module &Mod, TargetMachine *TM, - std::unique_ptr DiagnosticOutputFile) { + LLVMRemarkFileHandle DiagnosticOutputFile) { // Perform optimization and code generation for ThinLTO. if (!opt(Conf, TM, Task, Mod, /*IsThinLTO=*/true, /*ExportSummary=*/nullptr, /*ImportSummary=*/&CombinedIndex, diff --git a/llvm/lib/LTO/LTOCodeGenerator.cpp b/llvm/lib/LTO/LTOCodeGenerator.cpp index d8a96f73110fd..8aa404da15286 100644 --- a/llvm/lib/LTO/LTOCodeGenerator.cpp +++ b/llvm/lib/LTO/LTOCodeGenerator.cpp @@ -545,6 +545,7 @@ void LTOCodeGenerator::finishOptimizationRemarks() { if (DiagnosticOutputFile) { DiagnosticOutputFile->keep(); // FIXME: LTOCodeGenerator dtor is not invoked on Darwin + DiagnosticOutputFile.finalize(); DiagnosticOutputFile->os().flush(); } } diff --git a/llvm/lib/Remarks/BitstreamRemarkParser.cpp b/llvm/lib/Remarks/BitstreamRemarkParser.cpp index d40b40dfb2ba0..33eedd6042c37 100644 --- a/llvm/lib/Remarks/BitstreamRemarkParser.cpp +++ b/llvm/lib/Remarks/BitstreamRemarkParser.cpp @@ -197,14 +197,9 @@ Error BitstreamRemarkParserHelper::parseNext() { Loc.reset(); Args.clear(); - if (Error E = expectBlock()) - return E; return parseBlock(); } -BitstreamParserHelper::BitstreamParserHelper(StringRef Buffer) - : Stream(Buffer) {} - Error BitstreamParserHelper::expectMagic() { std::array Result; for (unsigned I = 0; I < 4; ++I) @@ -244,14 +239,57 @@ Error BitstreamParserHelper::parseBlockInfoBlock() { return Error::success(); } -Error BitstreamParserHelper::advanceToMetaBlock() { +Error BitstreamParserHelper::parseMeta() { if (Error E = expectMagic()) return E; if (Error E = parseBlockInfoBlock()) return E; + + // Parse early meta block. + if (Error E = MetaHelper.expectBlock()) + return E; + if (Error E = MetaHelper.parseBlock()) + return E; + + // Skip all Remarks blocks. + while (!Stream.AtEndOfStream()) { + auto MaybeBlockID = expectSubBlock(Stream); + if (!MaybeBlockID) + return MaybeBlockID.takeError(); + if (*MaybeBlockID == META_BLOCK_ID) + break; + if (*MaybeBlockID != REMARK_BLOCK_ID) + return error("Unexpected block between meta blocks."); + // Remember first remark block. + if (!RemarkStartBitPos) + RemarkStartBitPos = Stream.GetCurrentBitNo(); + if (Error E = Stream.SkipBlock()) + return E; + } + + // Late meta block is optional if there are no remarks. + if (Stream.AtEndOfStream()) + return Error::success(); + + // Parse late meta block. + if (Error E = MetaHelper.parseBlock()) + return E; return Error::success(); } +Error BitstreamParserHelper::parseRemark() { + if (RemarkStartBitPos) { + RemarkStartBitPos.reset(); + } else { + auto MaybeBlockID = expectSubBlock(Stream); + if (!MaybeBlockID) + return MaybeBlockID.takeError(); + if (*MaybeBlockID != REMARK_BLOCK_ID) + return make_error(); + } + return RemarksHelper->parseNext(); +} + Expected> remarks::createBitstreamParserFromMeta( StringRef Buf, std::optional ExternalFilePrependPath) { @@ -263,45 +301,52 @@ remarks::createBitstreamParserFromMeta( return std::move(Parser); } +BitstreamRemarkParser::BitstreamRemarkParser(StringRef Buf) + : RemarkParser(Format::Bitstream), ParserHelper(Buf) {} + Expected> BitstreamRemarkParser::next() { - if (ParserHelper.atEndOfStream()) - return make_error(); + if (!IsMetaReady) { + // Container is completely empty. + if (ParserHelper->Stream.AtEndOfStream()) + return make_error(); - if (!ReadyToParseRemarks) { if (Error E = parseMeta()) return std::move(E); - ReadyToParseRemarks = true; + IsMetaReady = true; + + // Container has meta, but no remarks blocks. + if (!ParserHelper->RemarkStartBitPos) + return error( + "Container is non-empty, but does not contain any remarks blocks."); + + if (Error E = + ParserHelper->Stream.JumpToBit(*ParserHelper->RemarkStartBitPos)) + return std::move(E); + ParserHelper->RemarksHelper.emplace(ParserHelper->Stream); } - return parseRemark(); + if (Error E = ParserHelper->parseRemark()) + return std::move(E); + return processRemark(); } Error BitstreamRemarkParser::parseMeta() { - if (Error E = ParserHelper.advanceToMetaBlock()) - return E; - - BitstreamMetaParserHelper MetaHelper(ParserHelper.Stream); - if (Error E = MetaHelper.expectBlock()) + if (Error E = ParserHelper->parseMeta()) return E; - if (Error E = MetaHelper.parseBlock()) - return E; - - if (Error E = processCommonMeta(MetaHelper)) + if (Error E = processCommonMeta()) return E; switch (ContainerType) { - case BitstreamRemarkContainerType::Standalone: - return processStandaloneMeta(MetaHelper); - case BitstreamRemarkContainerType::SeparateRemarksFile: - return processSeparateRemarksFileMeta(MetaHelper); - case BitstreamRemarkContainerType::SeparateRemarksMeta: - return processSeparateRemarksMetaMeta(MetaHelper); + case BitstreamRemarkContainerType::RemarksFileExternal: + return processExternalFilePath(); + case BitstreamRemarkContainerType::RemarksFile: + return processFileContainerMeta(); } llvm_unreachable("Unknown BitstreamRemarkContainerType enum"); } -Error BitstreamRemarkParser::processCommonMeta( - BitstreamMetaParserHelper &Helper) { +Error BitstreamRemarkParser::processCommonMeta() { + auto &Helper = ParserHelper->MetaHelper; if (!Helper.Container) return Helper.error("Missing container info."); auto &Container = *Helper.Container; @@ -313,7 +358,16 @@ Error BitstreamRemarkParser::processCommonMeta( return Error::success(); } -Error BitstreamRemarkParser::processStrTab(BitstreamMetaParserHelper &Helper) { +Error BitstreamRemarkParser::processFileContainerMeta() { + if (Error E = processRemarkVersion()) + return E; + if (Error E = processStrTab()) + return E; + return Error::success(); +} + +Error BitstreamRemarkParser::processStrTab() { + auto &Helper = ParserHelper->MetaHelper; if (!Helper.StrTabBuf) return Helper.error("Missing string table."); // Parse and assign the string table. @@ -321,26 +375,25 @@ Error BitstreamRemarkParser::processStrTab(BitstreamMetaParserHelper &Helper) { return Error::success(); } -Error BitstreamRemarkParser::processRemarkVersion( - BitstreamMetaParserHelper &Helper) { +Error BitstreamRemarkParser::processRemarkVersion() { + auto &Helper = ParserHelper->MetaHelper; if (!Helper.RemarkVersion) return Helper.error("Missing remark version."); RemarkVersion = *Helper.RemarkVersion; return Error::success(); } -Error BitstreamRemarkParser::processExternalFilePath( - BitstreamMetaParserHelper &Helper) { +Error BitstreamRemarkParser::processExternalFilePath() { + auto &Helper = ParserHelper->MetaHelper; if (!Helper.ExternalFilePath) return Helper.error("Missing external file path."); - StringRef ExternalFilePath = *Helper.ExternalFilePath; SmallString<80> FullPath(ExternalFilePrependPath); - sys::path::append(FullPath, ExternalFilePath); + sys::path::append(FullPath, *Helper.ExternalFilePath); // External file: open the external file, parse it, check if its metadata - // matches the one from the separate metadata, then replace the current parser - // with the one parsing the remarks. + // matches the one from the separate metadata, then replace the current + // parser with the one parsing the remarks. ErrorOr> BufferOrErr = MemoryBuffer::getFile(FullPath); if (std::error_code EC = BufferOrErr.getError()) @@ -353,58 +406,19 @@ Error BitstreamRemarkParser::processExternalFilePath( return make_error(); // Create a separate parser used for parsing the separate file. - ParserHelper = BitstreamParserHelper(TmpRemarkBuffer->getBuffer()); - // Advance and check until we can parse the meta block. - if (Error E = ParserHelper.advanceToMetaBlock()) - return E; - // Parse the meta from the separate file. - // Note: here we overwrite the BlockInfo with the one from the file. This will - // be used to parse the rest of the file. - BitstreamMetaParserHelper SeparateMetaHelper(ParserHelper.Stream); - if (Error E = SeparateMetaHelper.expectBlock()) - return E; - if (Error E = SeparateMetaHelper.parseBlock()) - return E; - - if (Error E = processCommonMeta(SeparateMetaHelper)) - return E; - - if (ContainerType != BitstreamRemarkContainerType::SeparateRemarksFile) - return SeparateMetaHelper.error("Wrong container type in external file."); - - // Process the meta from the separate file. - return processSeparateRemarksFileMeta(SeparateMetaHelper); -} - -Error BitstreamRemarkParser::processStandaloneMeta( - BitstreamMetaParserHelper &Helper) { - if (Error E = processStrTab(Helper)) + ParserHelper.emplace(TmpRemarkBuffer->getBuffer()); + if (Error E = parseMeta()) return E; - return processRemarkVersion(Helper); -} -Error BitstreamRemarkParser::processSeparateRemarksFileMeta( - BitstreamMetaParserHelper &Helper) { - return processRemarkVersion(Helper); -} + if (ContainerType != BitstreamRemarkContainerType::RemarksFile) + return ParserHelper->MetaHelper.error( + "Wrong container type in external file."); -Error BitstreamRemarkParser::processSeparateRemarksMetaMeta( - BitstreamMetaParserHelper &Helper) { - if (Error E = processStrTab(Helper)) - return E; - return processExternalFilePath(Helper); -} - -Expected> BitstreamRemarkParser::parseRemark() { - BitstreamRemarkParserHelper RemarkHelper(ParserHelper.Stream); - if (Error E = RemarkHelper.parseNext()) - return std::move(E); - - return processRemark(RemarkHelper); + return Error::success(); } -Expected> -BitstreamRemarkParser::processRemark(BitstreamRemarkParserHelper &Helper) { +Expected> BitstreamRemarkParser::processRemark() { + auto &Helper = *ParserHelper->RemarksHelper; std::unique_ptr Result = std::make_unique(); Remark &R = *Result; @@ -491,5 +505,3 @@ BitstreamRemarkParser::processRemark(BitstreamRemarkParserHelper &Helper) { return std::move(Result); } -llvm::remarks::BitstreamRemarkParser::BitstreamRemarkParser(StringRef Buf) - : RemarkParser(Format::Bitstream), ParserHelper(Buf) {} diff --git a/llvm/lib/Remarks/BitstreamRemarkParser.h b/llvm/lib/Remarks/BitstreamRemarkParser.h index d756e3296a871..4f66c47bb4b29 100644 --- a/llvm/lib/Remarks/BitstreamRemarkParser.h +++ b/llvm/lib/Remarks/BitstreamRemarkParser.h @@ -187,35 +187,49 @@ struct BitstreamParserHelper { BitstreamCursor Stream; /// The block info block. BitstreamBlockInfo BlockInfo; + + /// Helper to parse the metadata blocks in this bitstream. + BitstreamMetaParserHelper MetaHelper; + /// Helper to parse the remark blocks in this bitstream. Only needed + /// for ContainerType RemarksFile. + std::optional RemarksHelper; + /// The position of the first remark block we encounter after + /// the initial metadata block. + std::optional RemarkStartBitPos; + /// Start parsing at \p Buffer. - BitstreamParserHelper(StringRef Buffer); + BitstreamParserHelper(StringRef Buffer) + : Stream(Buffer), MetaHelper(Stream), RemarksHelper(Stream) {} + /// Parse and validate the magic number. Error expectMagic(); - /// Advance to the meta block - Error advanceToMetaBlock(); /// Parse the block info block containing all the abbrevs. /// This needs to be called before calling any other parsing function. Error parseBlockInfoBlock(); - /// Return true if the parser reached the end of the stream. - bool atEndOfStream() { return Stream.AtEndOfStream(); } + + /// Parse all metadata blocks in the file. This populates the meta helper. + Error parseMeta(); + /// Parse the next remark. This populates the remark helper data. + Error parseRemark(); }; /// Parses and holds the state of the latest parsed remark. struct BitstreamRemarkParser : public RemarkParser { /// The buffer to parse. - BitstreamParserHelper ParserHelper; + std::optional ParserHelper; /// The string table used for parsing strings. std::optional StrTab; /// Temporary remark buffer used when the remarks are stored separately. std::unique_ptr TmpRemarkBuffer; + /// Whether the metadata has already been parsed, so we can continue parsing + /// remarks. + bool IsMetaReady = false; /// The common metadata used to decide how to parse the buffer. /// This is filled when parsing the metadata block. uint64_t ContainerVersion = 0; uint64_t RemarkVersion = 0; BitstreamRemarkContainerType ContainerType = - BitstreamRemarkContainerType::Standalone; - /// Wether the parser is ready to parse remarks. - bool ReadyToParseRemarks = false; + BitstreamRemarkContainerType::RemarksFile; /// Create a parser that expects to find a string table embedded in the /// stream. @@ -230,20 +244,15 @@ struct BitstreamRemarkParser : public RemarkParser { /// Parse and process the metadata of the buffer. Error parseMeta(); - /// Parse a Bitstream remark. - Expected> parseRemark(); - private: - Error processCommonMeta(BitstreamMetaParserHelper &Helper); - Error processStandaloneMeta(BitstreamMetaParserHelper &Helper); - Error processSeparateRemarksFileMeta(BitstreamMetaParserHelper &Helper); - Error processSeparateRemarksMetaMeta(BitstreamMetaParserHelper &Helper); - Error processExternalFilePath(BitstreamMetaParserHelper &Helper); - Error processStrTab(BitstreamMetaParserHelper &Helper); - Error processRemarkVersion(BitstreamMetaParserHelper &Helper); - - Expected> - processRemark(BitstreamRemarkParserHelper &Helper); + Error processCommonMeta(); + Error processFileContainerMeta(); + Error processExternalFilePath(); + + Expected> processRemark(); + + Error processStrTab(); + Error processRemarkVersion(); }; Expected> createBitstreamParserFromMeta( diff --git a/llvm/lib/Remarks/BitstreamRemarkSerializer.cpp b/llvm/lib/Remarks/BitstreamRemarkSerializer.cpp index b2627196bce62..abd436e0ee561 100644 --- a/llvm/lib/Remarks/BitstreamRemarkSerializer.cpp +++ b/llvm/lib/Remarks/BitstreamRemarkSerializer.cpp @@ -12,25 +12,23 @@ //===----------------------------------------------------------------------===// #include "llvm/Remarks/BitstreamRemarkSerializer.h" +#include "llvm/ADT/ScopeExit.h" #include "llvm/Remarks/Remark.h" +#include #include using namespace llvm; using namespace llvm::remarks; BitstreamRemarkSerializerHelper::BitstreamRemarkSerializerHelper( - BitstreamRemarkContainerType ContainerType) - : Bitstream(Encoded), ContainerType(ContainerType) {} - -static void push(SmallVectorImpl &R, StringRef Str) { - append_range(R, Str); -} + BitstreamRemarkContainerType ContainerType, raw_ostream &OS) + : Bitstream(OS), ContainerType(ContainerType) {} static void setRecordName(unsigned RecordID, BitstreamWriter &Bitstream, SmallVectorImpl &R, StringRef Str) { R.clear(); R.push_back(RecordID); - push(R, Str); + append_range(R, Str); Bitstream.EmitRecord(bitc::BLOCKINFO_CODE_SETRECORDNAME, R); } @@ -41,7 +39,7 @@ static void initBlock(unsigned BlockID, BitstreamWriter &Bitstream, Bitstream.EmitRecord(bitc::BLOCKINFO_CODE_SETBID, R); R.clear(); - push(R, Str); + append_range(R, Str); Bitstream.EmitRecord(bitc::BLOCKINFO_CODE_BLOCKNAME, R); } @@ -200,75 +198,64 @@ void BitstreamRemarkSerializerHelper::setupBlockInfo() { Bitstream.Emit(static_cast(C), 8); Bitstream.EnterBlockInfoBlock(); + auto ExitBlock = make_scope_exit([&] { Bitstream.ExitBlock(); }); // Setup the main metadata. Depending on the container type, we'll setup the // required records next. setupMetaBlockInfo(); switch (ContainerType) { - case BitstreamRemarkContainerType::SeparateRemarksMeta: - // Needs a string table that the separate remark file is using. - setupMetaStrTab(); + case BitstreamRemarkContainerType::RemarksFileExternal: // Needs to know where the external remarks file is. setupMetaExternalFile(); - break; - case BitstreamRemarkContainerType::SeparateRemarksFile: - // Contains remarks: emit the version. - setupMetaRemarkVersion(); - // Contains remarks: emit the remark abbrevs. - setupRemarkBlockInfo(); - break; - case BitstreamRemarkContainerType::Standalone: + return; + case BitstreamRemarkContainerType::RemarksFile: // Contains remarks: emit the version. setupMetaRemarkVersion(); // Needs a string table. setupMetaStrTab(); // Contains remarks: emit the remark abbrevs. setupRemarkBlockInfo(); - break; + return; } - - Bitstream.ExitBlock(); + llvm_unreachable("Unexpected BitstreamRemarkContainerType"); } void BitstreamRemarkSerializerHelper::emitMetaBlock( - uint64_t ContainerVersion, std::optional RemarkVersion, - std::optional StrTab, std::optional Filename) { // Emit the meta block Bitstream.EnterSubblock(META_BLOCK_ID, 3); + auto ExitBlock = make_scope_exit([&] { Bitstream.ExitBlock(); }); // The container version and type. R.clear(); R.push_back(RECORD_META_CONTAINER_INFO); - R.push_back(ContainerVersion); + R.push_back(CurrentContainerVersion); R.push_back(static_cast(ContainerType)); Bitstream.EmitRecordWithAbbrev(RecordMetaContainerInfoAbbrevID, R); switch (ContainerType) { - case BitstreamRemarkContainerType::SeparateRemarksMeta: - assert(StrTab != std::nullopt && *StrTab != nullptr); - emitMetaStrTab(**StrTab); + case BitstreamRemarkContainerType::RemarksFileExternal: assert(Filename != std::nullopt); emitMetaExternalFile(*Filename); - break; - case BitstreamRemarkContainerType::SeparateRemarksFile: - assert(RemarkVersion != std::nullopt); - emitMetaRemarkVersion(*RemarkVersion); - break; - case BitstreamRemarkContainerType::Standalone: - assert(RemarkVersion != std::nullopt); - emitMetaRemarkVersion(*RemarkVersion); - assert(StrTab != std::nullopt && *StrTab != nullptr); - emitMetaStrTab(**StrTab); - break; + return; + case BitstreamRemarkContainerType::RemarksFile: + emitMetaRemarkVersion(CurrentRemarkVersion); + return; } + llvm_unreachable("Unexpected BitstreamRemarkContainerType"); +} +void BitstreamRemarkSerializerHelper::emitLateMetaBlock( + const StringTable &StrTab) { + // Emit the late meta block (after all remarks are serialized) + Bitstream.EnterSubblock(META_BLOCK_ID, 3); + emitMetaStrTab(StrTab); Bitstream.ExitBlock(); } -void BitstreamRemarkSerializerHelper::emitRemarkBlock(const Remark &Remark, - StringTable &StrTab) { +void BitstreamRemarkSerializerHelper::emitRemark(const Remark &Remark, + StringTable &StrTab) { Bitstream.EnterSubblock(REMARK_BLOCK_ID, 4); R.clear(); @@ -317,73 +304,49 @@ void BitstreamRemarkSerializerHelper::emitRemarkBlock(const Remark &Remark, Bitstream.ExitBlock(); } -void BitstreamRemarkSerializerHelper::flushToStream(raw_ostream &OS) { - OS.write(Encoded.data(), Encoded.size()); - Encoded.clear(); -} - -StringRef BitstreamRemarkSerializerHelper::getBuffer() { - return StringRef(Encoded.data(), Encoded.size()); -} - -BitstreamRemarkSerializer::BitstreamRemarkSerializer(raw_ostream &OS, - SerializerMode Mode) - : RemarkSerializer(Format::Bitstream, OS, Mode), - Helper(BitstreamRemarkContainerType::SeparateRemarksFile) { - assert(Mode == SerializerMode::Separate && - "For SerializerMode::Standalone, a pre-filled string table needs to " - "be provided."); - // We always use a string table with bitstream. +BitstreamRemarkSerializer::BitstreamRemarkSerializer(raw_ostream &OS) + : RemarkSerializer(Format::Bitstream, OS) { StrTab.emplace(); } BitstreamRemarkSerializer::BitstreamRemarkSerializer(raw_ostream &OS, - SerializerMode Mode, StringTable StrTabIn) - : RemarkSerializer(Format::Bitstream, OS, Mode), - Helper(Mode == SerializerMode::Separate - ? BitstreamRemarkContainerType::SeparateRemarksFile - : BitstreamRemarkContainerType::Standalone) { + : RemarkSerializer(Format::Bitstream, OS) { StrTab = std::move(StrTabIn); } -void BitstreamRemarkSerializer::emit(const Remark &Remark) { - if (!DidSetUp) { - // Emit the metadata that is embedded in the remark file. - // If we're in standalone mode, serialize the string table as well. - bool IsStandalone = - Helper.ContainerType == BitstreamRemarkContainerType::Standalone; - BitstreamMetaSerializer MetaSerializer( - OS, Helper, - IsStandalone ? &*StrTab - : std::optional(std::nullopt)); - MetaSerializer.emit(); - DidSetUp = true; - } +BitstreamRemarkSerializer::~BitstreamRemarkSerializer() { finalize(); } + +void BitstreamRemarkSerializer::setup() { + if (Helper) + return; + Helper.emplace(BitstreamRemarkContainerType::RemarksFile, OS); + Helper->setupBlockInfo(); + Helper->emitMetaBlock(); +} - assert(DidSetUp && - "The Block info block and the meta block were not emitted yet."); - Helper.emitRemarkBlock(Remark, *StrTab); +void BitstreamRemarkSerializer::finalize() { + if (!Helper) + return; + Helper->emitLateMetaBlock(*StrTab); + Helper = std::nullopt; +} - Helper.flushToStream(OS); +void BitstreamRemarkSerializer::emit(const Remark &Remark) { + setup(); + Helper->emitRemark(Remark, *StrTab); } -std::unique_ptr BitstreamRemarkSerializer::metaSerializer( - raw_ostream &OS, std::optional ExternalFilename) { - assert(Helper.ContainerType != - BitstreamRemarkContainerType::SeparateRemarksMeta); - bool IsStandalone = - Helper.ContainerType == BitstreamRemarkContainerType::Standalone; +std::unique_ptr +BitstreamRemarkSerializer::metaSerializer(raw_ostream &OS, + StringRef ExternalFilename) { return std::make_unique( - OS, - IsStandalone ? BitstreamRemarkContainerType::Standalone - : BitstreamRemarkContainerType::SeparateRemarksMeta, - &*StrTab, ExternalFilename); + OS, BitstreamRemarkContainerType::RemarksFileExternal, ExternalFilename); } void BitstreamMetaSerializer::emit() { + assert(Helper && "BitstreamMetaSerializer emitted multiple times"); Helper->setupBlockInfo(); - Helper->emitMetaBlock(CurrentContainerVersion, CurrentRemarkVersion, StrTab, - ExternalFilename); - Helper->flushToStream(OS); + Helper->emitMetaBlock(ExternalFilename); + Helper = std::nullopt; } diff --git a/llvm/lib/Remarks/RemarkLinker.cpp b/llvm/lib/Remarks/RemarkLinker.cpp index b00419bd4e51b..f0feeccbfe1b8 100644 --- a/llvm/lib/Remarks/RemarkLinker.cpp +++ b/llvm/lib/Remarks/RemarkLinker.cpp @@ -108,7 +108,7 @@ Error RemarkLinker::link(const object::ObjectFile &Obj, Format RemarkFormat) { Error RemarkLinker::serialize(raw_ostream &OS, Format RemarksFormat) const { Expected> MaybeSerializer = - createRemarkSerializer(RemarksFormat, SerializerMode::Standalone, OS, + createRemarkSerializer(RemarksFormat, OS, std::move(const_cast(StrTab))); if (!MaybeSerializer) return MaybeSerializer.takeError(); diff --git a/llvm/lib/Remarks/RemarkSerializer.cpp b/llvm/lib/Remarks/RemarkSerializer.cpp index df1da53d7c8a6..80388b4c47cb0 100644 --- a/llvm/lib/Remarks/RemarkSerializer.cpp +++ b/llvm/lib/Remarks/RemarkSerializer.cpp @@ -18,34 +18,32 @@ using namespace llvm; using namespace llvm::remarks; Expected> -remarks::createRemarkSerializer(Format RemarksFormat, SerializerMode Mode, - raw_ostream &OS) { +remarks::createRemarkSerializer(Format RemarksFormat, raw_ostream &OS) { switch (RemarksFormat) { case Format::Unknown: case Format::Auto: return createStringError(std::errc::invalid_argument, "Invalid remark serializer format."); case Format::YAML: - return std::make_unique(OS, Mode); + return std::make_unique(OS); case Format::Bitstream: - return std::make_unique(OS, Mode); + return std::make_unique(OS); } llvm_unreachable("Unknown remarks::Format enum"); } Expected> -remarks::createRemarkSerializer(Format RemarksFormat, SerializerMode Mode, - raw_ostream &OS, remarks::StringTable StrTab) { +remarks::createRemarkSerializer(Format RemarksFormat, raw_ostream &OS, + remarks::StringTable StrTab) { switch (RemarksFormat) { case Format::Unknown: case Format::Auto: return createStringError(std::errc::invalid_argument, "Invalid remark serializer format."); case Format::YAML: - return std::make_unique(OS, Mode, std::move(StrTab)); + return std::make_unique(OS, std::move(StrTab)); case Format::Bitstream: - return std::make_unique(OS, Mode, - std::move(StrTab)); + return std::make_unique(OS, std::move(StrTab)); } llvm_unreachable("Unknown remarks::Format enum"); } diff --git a/llvm/lib/Remarks/RemarkStreamer.cpp b/llvm/lib/Remarks/RemarkStreamer.cpp index bb62c8b5c2fdc..d9be2f1fcb6a4 100644 --- a/llvm/lib/Remarks/RemarkStreamer.cpp +++ b/llvm/lib/Remarks/RemarkStreamer.cpp @@ -12,6 +12,7 @@ #include "llvm/Remarks/RemarkStreamer.h" #include "llvm/Support/CommandLine.h" +#include #include using namespace llvm; @@ -31,6 +32,14 @@ RemarkStreamer::RemarkStreamer( Filename(FilenameIn ? std::optional(FilenameIn->str()) : std::nullopt) {} +RemarkStreamer::~RemarkStreamer() { + // Ensure that llvm::finalizeOptimizationRemarks was called before the + // RemarkStreamer is destroyed. + assert(!RemarkSerializer && + "RemarkSerializer must be released before RemarkStreamer is " + "destroyed. Ensure llvm::finalizeOptimizationRemarks is called."); +} + Error RemarkStreamer::setFilter(StringRef Filter) { Regex R = Regex(Filter); std::string RegexError; @@ -57,16 +66,7 @@ bool RemarkStreamer::needsSection() const { assert(EnableRemarksSection == cl::BOU_UNSET); - // We only need a section if we're in separate mode. - if (RemarkSerializer->Mode != remarks::SerializerMode::Separate) - return false; - - // Only some formats need a section: - // * bitstream - switch (RemarkSerializer->SerializerFormat) { - case remarks::Format::Bitstream: - return true; - default: - return false; - } + // Enable remark sections by default for bitstream remarks (so dsymutil can + // find all remarks for a linked binary) + return RemarkSerializer->SerializerFormat == Format::Bitstream; } diff --git a/llvm/lib/Remarks/YAMLRemarkParser.cpp b/llvm/lib/Remarks/YAMLRemarkParser.cpp index 5ff42fe6b9a9c..baad378d72bd4 100644 --- a/llvm/lib/Remarks/YAMLRemarkParser.cpp +++ b/llvm/lib/Remarks/YAMLRemarkParser.cpp @@ -385,7 +385,11 @@ Expected YAMLRemarkParser::parseArg(yaml::Node &Node) { if (!ValueStr) return error("argument value is missing.", *ArgMap); - return Argument{*KeyStr, *ValueStr, Loc}; + Argument Arg; + Arg.Key = *KeyStr; + Arg.Val = *ValueStr; + Arg.Loc = Loc; + return Arg; } Expected> YAMLRemarkParser::next() { diff --git a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp index 846a72182d8f0..f8b610dd7f73f 100644 --- a/llvm/lib/Remarks/YAMLRemarkSerializer.cpp +++ b/llvm/lib/Remarks/YAMLRemarkSerializer.cpp @@ -19,8 +19,6 @@ using namespace llvm; using namespace llvm::remarks; -// Use the same keys whether we use a string table or not (respectively, T is an -// unsigned or a StringRef). static void mapRemarkHeader(yaml::IO &io, StringRef PassName, StringRef RemarkName, std::optional RL, StringRef FunctionName, @@ -131,10 +129,13 @@ template <> struct MappingTraits { LLVM_YAML_IS_SEQUENCE_VECTOR(Argument) -YAMLRemarkSerializer::YAMLRemarkSerializer(raw_ostream &OS, SerializerMode Mode, - std::optional StrTabIn) - : RemarkSerializer(Format::YAML, OS, Mode), - YAMLOutput(OS, reinterpret_cast(this)) { +YAMLRemarkSerializer::YAMLRemarkSerializer(raw_ostream &OS) + : RemarkSerializer(Format::YAML, OS), + YAMLOutput(OS, reinterpret_cast(this)) {} + +YAMLRemarkSerializer::YAMLRemarkSerializer(raw_ostream &OS, + StringTable StrTabIn) + : YAMLRemarkSerializer(OS) { StrTab = std::move(StrTabIn); } @@ -145,8 +146,9 @@ void YAMLRemarkSerializer::emit(const Remark &Remark) { YAMLOutput << R; } -std::unique_ptr YAMLRemarkSerializer::metaSerializer( - raw_ostream &OS, std::optional ExternalFilename) { +std::unique_ptr +YAMLRemarkSerializer::metaSerializer(raw_ostream &OS, + StringRef ExternalFilename) { return std::make_unique(OS, ExternalFilename); } @@ -186,6 +188,5 @@ void YAMLMetaSerializer::emit() { support::endian::write64le(StrTabSizeBuf.data(), StrTabSize); OS.write(StrTabSizeBuf.data(), StrTabSizeBuf.size()); - if (ExternalFilename) - emitExternalFile(OS, *ExternalFilename); + emitExternalFile(OS, ExternalFilename); } diff --git a/llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll b/llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll index b65ed66fcce65..b0a238ff8efee 100644 --- a/llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll +++ b/llvm/test/Transforms/Inline/optimization-remarks-passed-yaml.ll @@ -8,6 +8,11 @@ ; RUN: -pass-remarks-with-hotness 2>&1 | FileCheck %s ; RUN: cat %t | FileCheck -check-prefixes=YAML,YAML-NO-ANNOTATE %s +; RUN: opt < %s -S -passes=inline -pass-remarks-output=%t.bitstream -pass-remarks=inline \ +; RUN: -pass-remarks-missed=inline -pass-remarks-analysis=inline \ +; RUN: -pass-remarks-with-hotness -pass-remarks-format=bitstream 2>&1 | FileCheck %s +; RUN: llvm-remarkutil bitstream2yaml %t.bitstream | FileCheck -check-prefixes=YAML,YAML-NO-ANNOTATE %s + ; RUN: opt < %s -S -passes=inliner-wrapper -pass-remarks-output=%t -pass-remarks=inline \ ; RUN: -pass-remarks-missed=inline -pass-remarks-analysis=inline \ ; RUN: -annotate-inline-phase=false \ diff --git a/llvm/test/tools/dsymutil/ARM/remarks-linking-bundle-empty.test b/llvm/test/tools/dsymutil/ARM/remarks-linking-bundle-empty.test new file mode 100644 index 0000000000000..0a89fa1ddee3c --- /dev/null +++ b/llvm/test/tools/dsymutil/ARM/remarks-linking-bundle-empty.test @@ -0,0 +1,13 @@ +RUN: rm -rf %t +RUN: mkdir -p %t +RUN: cat %p/../Inputs/remarks/basic.macho.remarks.empty.arm64 > %t/basic.macho.remarks.empty.arm64 + +RUN: dsymutil -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/basic.macho.remarks.empty.arm64 + +Check that the remark file in the bundle does not exist: +RUN: not cat %t/basic.macho.remarks.empty.arm64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.arm64 2>&1 + +RUN: dsymutil --linker parallel -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/basic.macho.remarks.empty.arm64 + +Check that the remark file in the bundle does not exist: +RUN: not cat %t/basic.macho.remarks.empty.arm64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.empty.arm64 2>&1 diff --git a/llvm/test/tools/dsymutil/ARM/remarks-linking-bundle.test b/llvm/test/tools/dsymutil/ARM/remarks-linking-bundle.test new file mode 100644 index 0000000000000..e1b04455b0d9d --- /dev/null +++ b/llvm/test/tools/dsymutil/ARM/remarks-linking-bundle.test @@ -0,0 +1,81 @@ +RUN: rm -rf %t +RUN: mkdir -p %t/private/tmp/remarks +RUN: cat %p/../Inputs/remarks/basic.macho.remarks.arm64> %t/basic.macho.remarks.arm64 +RUN: llvm-remarkutil yaml2bitstream %p/../Inputs/private/tmp/remarks/basic1.macho.remarks.arm64.opt.yaml -o %t/private/tmp/remarks/basic1.macho.remarks.arm64.opt.bitstream +RUN: llvm-remarkutil yaml2bitstream %p/../Inputs/private/tmp/remarks/basic2.macho.remarks.arm64.opt.yaml -o %t/private/tmp/remarks/basic2.macho.remarks.arm64.opt.bitstream +RUN: llvm-remarkutil yaml2bitstream %p/../Inputs/private/tmp/remarks/basic3.macho.remarks.arm64.opt.yaml -o %t/private/tmp/remarks/basic3.macho.remarks.arm64.opt.bitstream + +RUN: dsymutil -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%t %t/basic.macho.remarks.arm64 + +Check that the remark file in the bundle exists and is sane: +RUN: llvm-bcanalyzer -dump %t/basic.macho.remarks.arm64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.arm64 | FileCheck %s + +RUN: dsymutil --linker parallel -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%t %t/basic.macho.remarks.arm64 + +Check that the remark file in the bundle exists and is sane: +RUN: llvm-bcanalyzer -dump %t/basic.macho.remarks.arm64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.arm64 | FileCheck %s + +Now emit it in a different format: YAML. +RUN: dsymutil -remarks-output-format=yaml -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%t %t/basic.macho.remarks.arm64 +RUN: cat %t/basic.macho.remarks.arm64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.arm64 | FileCheck %s --check-prefix=CHECK-YAML + +RUN: dsymutil --linker parallel -remarks-output-format=yaml -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%t %t/basic.macho.remarks.arm64 +RUN: cat %t/basic.macho.remarks.arm64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.arm64 | FileCheck %s --check-prefix=CHECK-YAML + +CHECK: %t/basic.macho.remarks.empty.x86_64 - -RUN: dsymutil -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/basic.macho.remarks.empty.x86_64 - -Check that the remark file in the bundle does not exist: -RUN: not cat %t/basic.macho.remarks.empty.x86_64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.empty.x86_64 2>&1 - -RUN: dsymutil --linker parallel -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/basic.macho.remarks.empty.x86_64 - -Check that the remark file in the bundle does not exist: -RUN: not cat %t/basic.macho.remarks.empty.x86_64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.empty.x86_64 2>&1 diff --git a/llvm/test/tools/dsymutil/X86/remarks-linking-bundle.test b/llvm/test/tools/dsymutil/X86/remarks-linking-bundle.test deleted file mode 100644 index d85cd54c8f640..0000000000000 --- a/llvm/test/tools/dsymutil/X86/remarks-linking-bundle.test +++ /dev/null @@ -1,67 +0,0 @@ -RUN: rm -rf %t -RUN: mkdir -p %t -RUN: cat %p/../Inputs/remarks/basic.macho.remarks.x86_64 > %t/basic.macho.remarks.x86_64 - -RUN: dsymutil -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/basic.macho.remarks.x86_64 - -Check that the remark file in the bundle exists and is sane: -RUN: llvm-bcanalyzer -dump %t/basic.macho.remarks.x86_64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.x86_64 | FileCheck %s - -RUN: dsymutil --linker parallel -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/basic.macho.remarks.x86_64 - -Check that the remark file in the bundle exists and is sane: -RUN: llvm-bcanalyzer -dump %t/basic.macho.remarks.x86_64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.x86_64 | FileCheck %s - -Now emit it in a different format: YAML. -RUN: dsymutil -remarks-output-format=yaml -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/basic.macho.remarks.x86_64 -RUN: cat %t/basic.macho.remarks.x86_64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.x86_64 | FileCheck %s --check-prefix=CHECK-YAML - -RUN: dsymutil --linker parallel -remarks-output-format=yaml -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/basic.macho.remarks.x86_64 -RUN: cat %t/basic.macho.remarks.x86_64.dSYM/Contents/Resources/Remarks/basic.macho.remarks.x86_64 | FileCheck %s --check-prefix=CHECK-YAML - -CHECK: %t/fat.macho.remarks.x86 +RUN: llvm-remarkutil yaml2bitstream %p/../Inputs/private/tmp/remarks/fat.macho.remarks.x86_64.opt.yaml -o %t/private/tmp/remarks/fat.macho.remarks.x86_64.opt.bitstream +RUN: llvm-remarkutil yaml2bitstream %p/../Inputs/private/tmp/remarks/fat.macho.remarks.x86_64h.opt.yaml -o %t/private/tmp/remarks/fat.macho.remarks.x86_64h.opt.bitstream -RUN: dsymutil -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/fat.macho.remarks.x86 +RUN: dsymutil -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%t %t/fat.macho.remarks.x86 Check that the remark files in the bundle exist and are all sane: RUN: llvm-bcanalyzer -dump %t/fat.macho.remarks.x86.dSYM/Contents/Resources/Remarks/fat.macho.remarks.x86-x86_64h | FileCheck %s RUN: llvm-bcanalyzer -dump %t/fat.macho.remarks.x86.dSYM/Contents/Resources/Remarks/fat.macho.remarks.x86-x86_64 | FileCheck %s -RUN: llvm-bcanalyzer -dump %t/fat.macho.remarks.x86.dSYM/Contents/Resources/Remarks/fat.macho.remarks.x86-i386 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-i386 -RUN: dsymutil --linker parallel -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%p/../Inputs %t/fat.macho.remarks.x86 +RUN: dsymutil --linker parallel -oso-prepend-path=%p/../Inputs -remarks-prepend-path=%t %t/fat.macho.remarks.x86 Check that the remark files in the bundle exist and are all sane: RUN: llvm-bcanalyzer -dump %t/fat.macho.remarks.x86.dSYM/Contents/Resources/Remarks/fat.macho.remarks.x86-x86_64h | FileCheck %s RUN: llvm-bcanalyzer -dump %t/fat.macho.remarks.x86.dSYM/Contents/Resources/Remarks/fat.macho.remarks.x86-x86_64 | FileCheck %s -RUN: llvm-bcanalyzer -dump %t/fat.macho.remarks.x86.dSYM/Contents/Resources/Remarks/fat.macho.remarks.x86-i386 | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-i386 CHECK: &1 -o - | FileCheck %s --check-prefix=ERR RUN: llvm-remarkutil yaml2bitstream %p/Inputs/two-remarks.yaml -o %t.bitstream RUN: llvm-remarkutil bitstream2yaml %t.bitstream -o - | FileCheck %s -strict-whitespace +; ERR: error: Unsupported remark container version (expected: 1, read: 0). Please upgrade/downgrade your toolchain to read this container. + ; CHECK: --- !Analysis ; CHECK-NEXT: Pass: prologepilog ; CHECK-NEXT: Name: StackSize diff --git a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp index 138c5d0a513ed..b91c27e6a0f86 100644 --- a/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp +++ b/llvm/tools/dsymutil/DwarfLinkerForBinary.cpp @@ -291,6 +291,7 @@ ErrorOr> DwarfLinkerForBinary::loadObject( [&](StringRef FileName) { BinHolder.eraseObjectEntry(FileName); }); Error E = RL.link(*ErrorOrObj); + // FIXME: Remark parsing errors are not propagated to the user. if (Error NewE = handleErrors( std::move(E), [&](std::unique_ptr EC) -> Error { return remarksErrorHandler(Obj, *this, std::move(EC)); diff --git a/llvm/tools/llc/llc.cpp b/llvm/tools/llc/llc.cpp index b3d7185e7f144..a2327fbc3b66a 100644 --- a/llvm/tools/llc/llc.cpp +++ b/llvm/tools/llc/llc.cpp @@ -387,13 +387,13 @@ int main(int argc, char **argv) { // Set a diagnostic handler that doesn't exit on the first error Context.setDiagnosticHandler(std::make_unique()); - Expected> RemarksFileOrErr = + Expected RemarksFileOrErr = setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses, RemarksFormat, RemarksWithHotness, RemarksHotnessThreshold); if (Error E = RemarksFileOrErr.takeError()) reportError(std::move(E), RemarksFilename); - std::unique_ptr RemarksFile = std::move(*RemarksFileOrErr); + LLVMRemarkFileHandle RemarksFile = std::move(*RemarksFileOrErr); if (InputLanguage != "" && InputLanguage != "ir" && InputLanguage != "mir") reportError("input language must be '', 'IR' or 'MIR'"); diff --git a/llvm/tools/llvm-remarkutil/RemarkConvert.cpp b/llvm/tools/llvm-remarkutil/RemarkConvert.cpp index 207c5e0a8048b..203c8266b077d 100644 --- a/llvm/tools/llvm-remarkutil/RemarkConvert.cpp +++ b/llvm/tools/llvm-remarkutil/RemarkConvert.cpp @@ -80,8 +80,8 @@ static Error tryReserializeYAML2Bitstream( if (!MaybeOF) return MaybeOF.takeError(); auto OF = std::move(*MaybeOF); - auto MaybeSerializer = createRemarkSerializer( - OutputFormat, SerializerMode::Standalone, OF->os(), std::move(StrTab)); + auto MaybeSerializer = + createRemarkSerializer(OutputFormat, OF->os(), std::move(StrTab)); if (!MaybeSerializer) return MaybeSerializer.takeError(); auto Serializer = std::move(*MaybeSerializer); @@ -110,8 +110,7 @@ static Error tryBitstream2YAML() { if (!MaybeOF) return MaybeOF.takeError(); auto OF = std::move(*MaybeOF); - auto MaybeSerializer = createRemarkSerializer( - OutputFormat, SerializerMode::Standalone, OF->os()); + auto MaybeSerializer = createRemarkSerializer(OutputFormat, OF->os()); if (!MaybeSerializer) return MaybeSerializer.takeError(); diff --git a/llvm/tools/opt/optdriver.cpp b/llvm/tools/opt/optdriver.cpp index 26902b213571f..d4fa6eb50cda7 100644 --- a/llvm/tools/opt/optdriver.cpp +++ b/llvm/tools/opt/optdriver.cpp @@ -510,7 +510,7 @@ extern "C" int optMain( if (!DisableDITypeMap) Context.enableDebugTypeODRUniquing(); - Expected> RemarksFileOrErr = + Expected RemarksFileOrErr = setupLLVMOptimizationRemarks(Context, RemarksFilename, RemarksPasses, RemarksFormat, RemarksWithHotness, RemarksHotnessThreshold); @@ -518,7 +518,7 @@ extern "C" int optMain( errs() << toString(std::move(E)) << '\n'; return 1; } - std::unique_ptr RemarksFile = std::move(*RemarksFileOrErr); + LLVMRemarkFileHandle RemarksFile = std::move(*RemarksFileOrErr); // Load the input module... auto SetDataLayout = [&](StringRef IRTriple, diff --git a/llvm/unittests/Remarks/BitstreamRemarksFormatTest.cpp b/llvm/unittests/Remarks/BitstreamRemarksFormatTest.cpp index ea61691f4c835..ddf744521ef13 100644 --- a/llvm/unittests/Remarks/BitstreamRemarksFormatTest.cpp +++ b/llvm/unittests/Remarks/BitstreamRemarksFormatTest.cpp @@ -21,7 +21,7 @@ TEST(BitstreamRemarksFormat, Magic) { // This should be updated whenever any of the tests below are modified. TEST(BitstreamRemarksFormat, ContainerVersion) { - EXPECT_EQ(remarks::CurrentContainerVersion, 0UL); + EXPECT_EQ(remarks::CurrentContainerVersion, 1UL); } // The values of the current blocks should not change over time. diff --git a/llvm/unittests/Remarks/BitstreamRemarksParsingTest.cpp b/llvm/unittests/Remarks/BitstreamRemarksParsingTest.cpp index 6234931b3bece..f5973f5431c9c 100644 --- a/llvm/unittests/Remarks/BitstreamRemarksParsingTest.cpp +++ b/llvm/unittests/Remarks/BitstreamRemarksParsingTest.cpp @@ -14,7 +14,7 @@ using namespace llvm; -template void parseGood(const char (&Buf)[N]) { +template static void parseGood(const char (&Buf)[N]) { // 1. Parse the YAML remark -> FromYAMLRemark // 2. Serialize it to bitstream -> BSStream // 3. Parse it back -> FromBSRemark @@ -48,11 +48,11 @@ template void parseGood(const char (&Buf)[N]) { std::string BSBuf; raw_string_ostream BSStream(BSBuf); Expected> BSSerializer = - remarks::createRemarkSerializer(remarks::Format::Bitstream, - remarks::SerializerMode::Standalone, - BSStream, std::move(BSStrTab)); + remarks::createRemarkSerializer(remarks::Format::Bitstream, BSStream, + std::move(BSStrTab)); EXPECT_FALSE(errorToBool(BSSerializer.takeError())); (*BSSerializer)->emit(*FromYAMLRemark); + (*BSSerializer)->finalize(); // 3. Expected> MaybeBSParser = @@ -256,11 +256,11 @@ TEST(BitstreamRemarks, ContentsCAPI) { std::string BSBuf; raw_string_ostream BSStream(BSBuf); Expected> BSSerializer = - remarks::createRemarkSerializer(remarks::Format::Bitstream, - remarks::SerializerMode::Standalone, - BSStream, std::move(BSStrTab)); + remarks::createRemarkSerializer(remarks::Format::Bitstream, BSStream, + std::move(BSStrTab)); EXPECT_FALSE(errorToBool(BSSerializer.takeError())); (*BSSerializer)->emit(ToSerializeRemark); + (*BSSerializer)->finalize(); StringRef Buf = BSStream.str(); LLVMRemarkParserRef Parser = diff --git a/llvm/unittests/Remarks/BitstreamRemarksSerializerTest.cpp b/llvm/unittests/Remarks/BitstreamRemarksSerializerTest.cpp index 8113d35b3aff8..3b460965fdb23 100644 --- a/llvm/unittests/Remarks/BitstreamRemarksSerializerTest.cpp +++ b/llvm/unittests/Remarks/BitstreamRemarksSerializerTest.cpp @@ -7,8 +7,8 @@ //===----------------------------------------------------------------------===// #include "llvm/Bitcode/BitcodeAnalyzer.h" -#include "llvm/Remarks/BitstreamRemarkSerializer.h" #include "llvm/Remarks/Remark.h" +#include "llvm/Remarks/RemarkSerializer.h" #include "llvm/Support/raw_ostream.h" #include "gtest/gtest.h" #include @@ -34,23 +34,24 @@ static void checkAnalyze(StringRef Input, StringRef Expected) { EXPECT_EQ(OutputOS.str(), Expected); } -static void check(remarks::SerializerMode Mode, const remarks::Remark &R, - StringRef ExpectedR, std::optional ExpectedMeta, - std::optional StrTab) { +static void check(const remarks::Remark &R, StringRef ExpectedR, + std::optional ExpectedMeta = std::nullopt, + std::optional StrTab = std::nullopt) { // Emit the remark. std::string InputBuf; raw_string_ostream InputOS(InputBuf); Expected> MaybeSerializer = [&] { if (StrTab) - return createRemarkSerializer(remarks::Format::Bitstream, Mode, InputOS, + return createRemarkSerializer(remarks::Format::Bitstream, InputOS, std::move(*StrTab)); else - return createRemarkSerializer(remarks::Format::Bitstream, Mode, InputOS); + return createRemarkSerializer(remarks::Format::Bitstream, InputOS); }(); EXPECT_FALSE(errorToBool(MaybeSerializer.takeError())); std::unique_ptr Serializer = std::move(*MaybeSerializer); Serializer->emit(R); + Serializer->finalize(); // Analyze the serialized remark. checkAnalyze(InputOS.str(), ExpectedR); @@ -66,20 +67,6 @@ static void check(remarks::SerializerMode Mode, const remarks::Remark &R, } } -static void check(const remarks::Remark &R, StringRef ExpectedR, - StringRef ExpectedMeta, - std::optional StrTab = std::nullopt) { - return check(remarks::SerializerMode::Separate, R, ExpectedR, ExpectedMeta, - std::move(StrTab)); -} - -static void -checkStandalone(const remarks::Remark &R, StringRef ExpectedR, - std::optional StrTab = std::nullopt) { - return check(remarks::SerializerMode::Standalone, R, ExpectedR, - /*ExpectedMeta=*/std::nullopt, std::move(StrTab)); -} - TEST(BitstreamRemarkSerializer, SeparateRemarkFileNoOptionals) { remarks::Remark R; R.RemarkType = remarks::Type::Missed; @@ -89,19 +76,21 @@ TEST(BitstreamRemarkSerializer, SeparateRemarkFileNoOptionals) { check(R, "\n" "\n" - " \n" + " \n" " \n" "\n" "\n" " \n" - "\n", - "\n" - "\n" - " \n" - " blob data = " + "\n" + "\n" + " blob data = " "'remark\\x00pass\\x00function\\x00'\n" - " blob data = " - "'" EXTERNALFILETESTPATH"'\n" + "\n", + "\n" + "\n" + " \n" + " blob data = " + "'" EXTERNALFILETESTPATH "'\n" "\n"); } @@ -118,19 +107,21 @@ TEST(BitstreamRemarkSerializer, SeparateRemarkFileNoOptionalsSeparateStrTab) { check(R, "\n" "\n" - " \n" + " \n" " \n" "\n" "\n" " \n" - "\n", - "\n" - "\n" - " \n" - " blob data = " + "\n" + "\n" + " blob data = " "'function\\x00pass\\x00remark\\x00'\n" - " blob data = " - "'" EXTERNALFILETESTPATH"'\n" + "\n", + "\n" + "\n" + " \n" + " blob data = " + "'" EXTERNALFILETESTPATH "'\n" "\n", std::move(StrTab)); } @@ -148,20 +139,22 @@ TEST(BitstreamRemarkSerializer, SeparateRemarkFileDebugLoc) { check(R, "\n" "\n" - " \n" + " \n" " \n" "\n" "\n" " \n" " \n" - "\n", - "\n" - "\n" - " \n" - " blob data = " + "\n" + "\n" + " blob data = " "'remark\\x00pass\\x00function\\x00path\\x00'\n" - " blob data = " - "'" EXTERNALFILETESTPATH"'\n" + "\n", + "\n" + "\n" + " \n" + " blob data = " + "'" EXTERNALFILETESTPATH "'\n" "\n"); } @@ -175,20 +168,22 @@ TEST(BitstreamRemarkSerializer, SeparateRemarkFileHotness) { check(R, "\n" "\n" - " \n" + " \n" " \n" "\n" "\n" " \n" " \n" - "\n", - "\n" - "\n" - " \n" - " blob data = " + "\n" + "\n" + " blob data = " "'remark\\x00pass\\x00function\\x00'\n" - " blob data = " - "'" EXTERNALFILETESTPATH"'\n" + "\n", + "\n" + "\n" + " \n" + " blob data = " + "'" EXTERNALFILETESTPATH "'\n" "\n"); } @@ -204,20 +199,22 @@ TEST(BitstreamRemarkSerializer, SeparateRemarkFileArgNoDebugLoc) { check(R, "\n" "\n" - " \n" + " \n" " \n" "\n" "\n" " \n" " \n" - "\n", - "\n" - "\n" - " \n" - " blob data = " + "\n" + "\n" + " blob data = " "'remark\\x00pass\\x00function\\x00key\\x00value\\x00'\n" - " blob data = " - "'" EXTERNALFILETESTPATH"'\n" + "\n", + "\n" + "\n" + " \n" + " blob data = " + "'" EXTERNALFILETESTPATH "'\n" "\n"); } @@ -237,21 +234,23 @@ TEST(BitstreamRemarkSerializer, SeparateRemarkFileArgDebugLoc) { check(R, "\n" "\n" - " \n" + " \n" " \n" "\n" "\n" " \n" " \n" - "\n", - "\n" - "\n" - " \n" - " blob data = " + "\n" + "\n" + " blob data = " "'remark\\x00pass\\x00function\\x00key\\x00value\\x00path\\x00'\n" - " blob data = " - "'" EXTERNALFILETESTPATH"'\n" + "\n", + "\n" + "\n" + " \n" + " blob data = " + "'" EXTERNALFILETESTPATH "'\n" "\n"); } @@ -276,7 +275,7 @@ TEST(BitstreamRemarkSerializer, SeparateRemarkFileAll) { check(R, "\n" "\n" - " \n" + " \n" " \n" "\n" "\n" @@ -285,14 +284,17 @@ TEST(BitstreamRemarkSerializer, SeparateRemarkFileAll) { " \n" " \n" - "\n", - "\n" - "\n" - " \n" - " blob data = " + "\n" + "\n" + " blob data = " "'remark\\x00pass\\x00function\\x00path\\x00key\\x00value\\x00argpa" - "th\\x00'\n blob data = " - "'" EXTERNALFILETESTPATH"'\n" + "th\\x00'\n" + "\n", + "\n" + "\n" + " \n" + " blob data = " + "'" EXTERNALFILETESTPATH "'\n" "\n"); } @@ -323,15 +325,12 @@ TEST(BitstreamRemarkSerializer, Standalone) { R.Args.back().Loc->SourceFilePath = "argpath"; R.Args.back().Loc->SourceLine = 11; R.Args.back().Loc->SourceColumn = 66; - checkStandalone( + check( R, "\n" - "\n" - " \n" + "\n" + " \n" " \n" - " blob data = " - "'pass\\x00remark\\x00function\\x00path\\x00key\\x00value\\x00argpath\\x0" - "0'\n" "\n" "\n" " \n" @@ -339,6 +338,11 @@ TEST(BitstreamRemarkSerializer, Standalone) { " \n" " \n" - "\n", - std::move(StrTab)); + "\n" + "\n" + " blob data = " + "'pass\\x00remark\\x00function\\x00path\\x00key\\x00value\\x00argpath\\x0" + "0'\n" + "\n", + std::nullopt, std::move(StrTab)); } diff --git a/llvm/unittests/Remarks/RemarksLinkingTest.cpp b/llvm/unittests/Remarks/RemarksLinkingTest.cpp index 89de9e8f4f95d..54942ff681b47 100644 --- a/llvm/unittests/Remarks/RemarksLinkingTest.cpp +++ b/llvm/unittests/Remarks/RemarksLinkingTest.cpp @@ -133,16 +133,18 @@ TEST(Remarks, LinkingGoodBitstream) { "...\n", remarks::Format::Bitstream, "\n" - "\n" - " \n" + "\n" + " \n" " \n" - " blob data = " - "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00'\n" "\n" "\n" " \n" " \n" - "\n"); + "\n" + "\n" + " blob data = " + "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00'\n" + "\n"); // Check that we keep remarks without debug info. check(remarks::Format::YAML, @@ -153,15 +155,17 @@ TEST(Remarks, LinkingGoodBitstream) { "...\n", remarks::Format::Bitstream, "\n" - "\n" - " \n" + "\n" + " \n" " \n" - " blob data = " - "'inline\\x00NoDefinition\\x00foo\\x00'\n" "\n" "\n" " \n" - "\n"); + "\n" + "\n" + " blob data = " + "'inline\\x00NoDefinition\\x00foo\\x00'\n" + "\n"); // Check that we deduplicate remarks. check(remarks::Format::YAML, @@ -179,16 +183,18 @@ TEST(Remarks, LinkingGoodBitstream) { "...\n", remarks::Format::Bitstream, "\n" - "\n" - " \n" + "\n" + " \n" " \n" - " blob data = " - "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00'\n" "\n" "\n" " \n" " \n" - "\n"); + "\n" + "\n" + " blob data = " + "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00'\n" + "\n"); } TEST(Remarks, LinkingGoodStrTab) { @@ -209,11 +215,9 @@ TEST(Remarks, LinkingGoodStrTab) { "...\n", remarks::Format::Bitstream, "\n" - "\n" - " \n" + "\n" + " \n" " \n" - " blob data = " - "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00Ok\\x00'\n" "\n" "\n" " \n" @@ -222,7 +226,11 @@ TEST(Remarks, LinkingGoodStrTab) { "\n" " \n" " \n" - "\n"); + "\n" + "\n" + " blob data = " + "'inline\\x00NoDefinition\\x00foo\\x00file.c\\x00Ok\\x00'\n" + "\n"); } // Check that we propagate parsing errors. diff --git a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp index 7e994ac4d58bc..112cd92285685 100644 --- a/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp +++ b/llvm/unittests/Remarks/YAMLRemarksSerializerTest.cpp @@ -23,23 +23,23 @@ using namespace llvm; static void check(remarks::Format SerializerFormat, - remarks::SerializerMode Mode, ArrayRef Rs, - StringRef ExpectedR, std::optional ExpectedMeta, + ArrayRef Rs, StringRef ExpectedR, + std::optional ExpectedMeta, std::optional StrTab = std::nullopt) { std::string Buf; raw_string_ostream OS(Buf); Expected> MaybeS = [&] { if (StrTab) - return createRemarkSerializer(SerializerFormat, Mode, OS, - std::move(*StrTab)); + return createRemarkSerializer(SerializerFormat, OS, std::move(*StrTab)); else - return createRemarkSerializer(SerializerFormat, Mode, OS); + return createRemarkSerializer(SerializerFormat, OS); }(); EXPECT_FALSE(errorToBool(MaybeS.takeError())); std::unique_ptr S = std::move(*MaybeS); for (const remarks::Remark &R : Rs) S->emit(R); + S->finalize(); EXPECT_EQ(OS.str(), ExpectedR); if (ExpectedMeta) { @@ -54,8 +54,7 @@ static void check(remarks::Format SerializerFormat, static void check(remarks::Format SerializerFormat, const remarks::Remark &R, StringRef ExpectedR, StringRef ExpectedMeta, std::optional StrTab = std::nullopt) { - return check(SerializerFormat, remarks::SerializerMode::Separate, - ArrayRef(&R, &R + 1), ExpectedR, ExpectedMeta, + return check(SerializerFormat, ArrayRef(&R, &R + 1), ExpectedR, ExpectedMeta, std::move(StrTab)); } @@ -63,8 +62,7 @@ static void checkStandalone(remarks::Format SerializerFormat, const remarks::Remark &R, StringRef ExpectedR, std::optional StrTab = std::nullopt) { - return check(SerializerFormat, remarks::SerializerMode::Standalone, - ArrayRef(&R, &R + 1), ExpectedR, + return check(SerializerFormat, ArrayRef(&R, &R + 1), ExpectedR, /*ExpectedMeta=*/std::nullopt, std::move(StrTab)); } diff --git a/mlir/include/mlir/Remark/RemarkStreamer.h b/mlir/include/mlir/Remark/RemarkStreamer.h index 8bfd176d9bade..170d6b439a442 100644 --- a/mlir/include/mlir/Remark/RemarkStreamer.h +++ b/mlir/include/mlir/Remark/RemarkStreamer.h @@ -26,14 +26,15 @@ class LLVMRemarkStreamer final : public MLIRRemarkStreamerBase { createToFile(llvm::StringRef path, llvm::remarks::Format fmt); void streamOptimizationRemark(const Remark &remark) override; - void finalize() override {} + void finalize() override; ~LLVMRemarkStreamer() override; private: LLVMRemarkStreamer() = default; - std::unique_ptr remarkStreamer; std::unique_ptr file; + // RemarkStreamer must be destructed before file is destroyed! + std::unique_ptr remarkStreamer; }; } // namespace mlir::remark::detail diff --git a/mlir/lib/Remark/RemarkStreamer.cpp b/mlir/lib/Remark/RemarkStreamer.cpp index 8e3544ff2c34c..d213a1a2068d6 100644 --- a/mlir/lib/Remark/RemarkStreamer.cpp +++ b/mlir/lib/Remark/RemarkStreamer.cpp @@ -20,8 +20,7 @@ LLVMRemarkStreamer::createToFile(llvm::StringRef path, if (ec) return failure(); - auto serOr = llvm::remarks::createRemarkSerializer( - fmt, llvm::remarks::SerializerMode::Separate, f->os()); + auto serOr = llvm::remarks::createRemarkSerializer(fmt, f->os()); if (!serOr) { llvm::consumeError(serOr.takeError()); return failure(); @@ -50,6 +49,12 @@ LLVMRemarkStreamer::~LLVMRemarkStreamer() { if (file && remarkStreamer) file->keep(); } + +void LLVMRemarkStreamer::finalize() { + if (!remarkStreamer) + return; + remarkStreamer->releaseSerializer(); +} } // namespace mlir::remark::detail namespace mlir::remark { diff --git a/offload/plugins-nextgen/common/src/JIT.cpp b/offload/plugins-nextgen/common/src/JIT.cpp index 07ef05e7e9d38..881e27dad384c 100644 --- a/offload/plugins-nextgen/common/src/JIT.cpp +++ b/offload/plugins-nextgen/common/src/JIT.cpp @@ -180,9 +180,10 @@ Expected> JITEngine::backend(Module &M, const std::string &ComputeUnitKind, unsigned OptLevel) { - auto RemarksFileOrErr = setupLLVMOptimizationRemarks( - M.getContext(), /*RemarksFilename=*/"", /*RemarksPasses=*/"", - /*RemarksFormat=*/"", /*RemarksWithHotness=*/false); + Expected RemarksFileOrErr = + setupLLVMOptimizationRemarks( + M.getContext(), /*RemarksFilename=*/"", /*RemarksPasses=*/"", + /*RemarksFormat=*/"", /*RemarksWithHotness=*/false); if (Error E = RemarksFileOrErr.takeError()) return std::move(E); if (*RemarksFileOrErr)