[llvm] f2d22b5 - [memprof] Make RecordWriterTrait a non-template class (#87604)

[via llvm-commits]

Author: Kazu Hirata
Date: 2024-04-04T10:09:43-07:00
New Revision: f2d22b5944b3c3f77dd236d89dd419d231243a56

URL: https://github.com/llvm/llvm-project/commit/f2d22b5944b3c3f77dd236d89dd419d231243a56
DIFF: https://github.com/llvm/llvm-project/commit/f2d22b5944b3c3f77dd236d89dd419d231243a56.diff

LOG: [memprof] Make RecordWriterTrait a non-template class (#87604)

commit d89914f30bc7c180fe349a5aa0f03438ae6c20a4
  Author: Kazu Hirata <kazu at google.com>
  Date:   Wed Apr 3 21:48:38 2024 -0700

changed RecordWriterTrait to a template class with IndexedVersion as a
template parameter.  This patch changes the class back to a
non-template one while retaining the ability to serialize multiple
versions.

The reason I changed RecordWriterTrait to a template class was
because, even if RecordWriterTrait had IndexedVersion as a member
variable, RecordWriterTrait::EmitKeyDataLength, being a static
function, would not have access to the variable.

Since OnDiskChainedHashTableGenerator calls EmitKeyDataLength as:

  const std::pair<offset_type, offset_type> &Len =
      InfoObj.EmitKeyDataLength(Out, I->Key, I->Data);

we can make EmitKeyDataLength a member function, but we have one
problem.  InstrProfWriter::writeImpl calls:

  void insert(typename Info::key_type_ref Key,
              typename Info::data_type_ref Data) {
    Info InfoObj;
    insert(Key, Data, InfoObj);
  }

which default-constructs RecordWriterTrait without a specific version
number.  This patch fixes the problem by adjusting
InstrProfWriter::writeImpl to call the other form of insert instead:

  void insert(typename Info::key_type_ref Key,
              typename Info::data_type_ref Data, Info &InfoObj)

To prevent an accidental invocation of the default constructor of
RecordWriterTrait, this patch deletes the default constructor.

Added: 
    

Modified: 
    llvm/include/llvm/ProfileData/MemProf.h
    llvm/lib/ProfileData/InstrProfWriter.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ProfileData/MemProf.h b/llvm/include/llvm/ProfileData/MemProf.h
index 110e6977026413..0431c182276ec6 100644
--- a/llvm/include/llvm/ProfileData/MemProf.h
+++ b/llvm/include/llvm/ProfileData/MemProf.h
@@ -502,7 +502,7 @@ class RecordLookupTrait {
 };
 
 // Trait for writing IndexedMemProfRecord data to the on-disk hash table.
-template <IndexedVersion Version> class RecordWriterTrait {
+class RecordWriterTrait {
 public:
   using key_type = uint64_t;
   using key_type_ref = uint64_t;
@@ -517,12 +517,16 @@ template <IndexedVersion Version> class RecordWriterTrait {
   // we must use a default constructor with no params for the writer trait so we
   // have a public member which must be initialized by the user.
   MemProfSchema *Schema = nullptr;
+  // The MemProf version to use for the serialization.
+  IndexedVersion Version;
 
-  RecordWriterTrait() = default;
+  // We do not support the default constructor, which does not set Version.
+  RecordWriterTrait() = delete;
+  RecordWriterTrait(IndexedVersion V) : Version(V) {}
 
   static hash_value_type ComputeHash(key_type_ref K) { return K; }
 
-  static std::pair<offset_type, offset_type>
+  std::pair<offset_type, offset_type>
   EmitKeyDataLength(raw_ostream &Out, key_type_ref K, data_type_ref V) {
     using namespace support;
 

diff  --git a/llvm/lib/ProfileData/InstrProfWriter.cpp b/llvm/lib/ProfileData/InstrProfWriter.cpp
index a1bc180a53ca34..96ab729f91e4dd 100644
--- a/llvm/lib/ProfileData/InstrProfWriter.cpp
+++ b/llvm/lib/ProfileData/InstrProfWriter.cpp
@@ -558,14 +558,13 @@ Error InstrProfWriter::writeImpl(ProfOStream &OS) {
     }
 
     auto RecordWriter =
-        std::make_unique<memprof::RecordWriterTrait<memprof::Version1>>();
+        std::make_unique<memprof::RecordWriterTrait>(memprof::Version1);
     RecordWriter->Schema = &Schema;
-    OnDiskChainedHashTableGenerator<
-        memprof::RecordWriterTrait<memprof::Version1>>
+    OnDiskChainedHashTableGenerator<memprof::RecordWriterTrait>
         RecordTableGenerator;
     for (auto &I : MemProfRecordData) {
       // Insert the key (func hash) and value (memprof record).
-      RecordTableGenerator.insert(I.first, I.second);
+      RecordTableGenerator.insert(I.first, I.second, *RecordWriter.get());
     }
     // Release the memory of this MapVector as it is no longer needed.
     MemProfRecordData.clear();