[NFC] Modernize code style and comments in DwarfAccelTable

[JDevlieghere]

Summary

This patch modernizes some of the code in DwarfAccelTable.

[aprantl]

Minor nitpicks, general direction is fine of course.

lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp

201

I think these comments are supposed to go into the class definition.

lib/CodeGen/AsmPrinter/DwarfAccelTable.h

90

///

[JDevlieghere]

Address Adrian's comments.

[JDevlieghere]

Part of D42334

lib/CodeGen/AsmPrinter/DwarfAccelTable.h

 // grab the offset in the data for our final match.
 
 namespace llvm {
 
 class AsmPrinter;
 class DwarfDebug;
 
 class DwarfAccelTable {
-  // Helper function to compute the number of buckets needed based on
-  // the number of unique hashes.
+  /// Helper function to compute the number of buckets needed based on the
+  /// number of unique hashes.
   void ComputeBucketCount();
 
   struct TableHeader {
-    uint32_t magic = MagicHash; // 'HASH' magic value to allow endian detection
-    uint16_t version = 1;       // Version number.
-    uint16_t hash_function = dwarf::DW_hash_function_djb;
-    // The hash function enumeration that was used.
-    uint32_t bucket_count = 0;  // The number of buckets in this hash table.
-    uint32_t hashes_count = 0;  // The total number of unique hash values
-                                // and hash data offsets in this table.
-    uint32_t header_data_len;   // The bytes to skip to get to the hash
-                                // indexes (buckets) for correct alignment.
-    // Also written to disk is the implementation specific header data.
 
+    /// Magic number.
+    uint32_t Magic = MagicHash;
+
+    /// Version number.
+    uint16_t Version = 1;
+
+    /// The hash function enumeration that was used.
+    uint16_t HashFunction = dwarf::DW_hash_function_djb;
+
+    /// The number of buckets in this hash table.
+    uint32_t BucketCount = 0;
+
+    /// The total number of unique hash values and hash data offsets in this
+    /// table.
+    uint32_t HashesCount = 0;
+
+    /// The bytes to skip to get to the hash indexes (buckets) for correct
+    /// alignment. Also written to disk is the implementation specific header
+    /// data.
+    uint32_t HeaderDataLen;
+
+    /// The 'HASH' magic value to allow detecting endianness.
     static const uint32_t MagicHash = 0x48415348;
 
-    TableHeader(uint32_t data_len) : header_data_len(data_len) {}
+    TableHeader(uint32_t DataLen) : HeaderDataLen(DataLen) {}
 
 #ifndef NDEBUG
     void print(raw_ostream &OS) {
-      OS << "Magic: " << format("0x%x", magic) << "\n"
-         << "Version: " << version << "\n"
-         << "Hash Function: " << hash_function << "\n"
-         << "Bucket Count: " << bucket_count << "\n"
-         << "Header Data Length: " << header_data_len << "\n";
+      OS << "Magic: " << format("0x%x", Magic) << "\n"
+         << "Version: " << Version << "\n"
+         << "Hash Function: " << HashFunction << "\n"
+         << "Bucket Count: " << BucketCount << "\n"
+         << "Header Data Length: " << HeaderDataLen << "\n";
     }
 
     void dump() { print(dbgs()); }
 #endif
   };
 
 public:
-  // The HeaderData describes the form of each set of data. In general this
-  // is as a list of atoms (atom_count) where each atom contains a type
-  // (AtomType type) of data, and an encoding form (form). In the case of
-  // data that is referenced via DW_FORM_ref_* the die_offset_base is
-  // used to describe the offset for all forms in the list of atoms.
-  // This also serves as a public interface of sorts.
-  // When written to disk this will have the form:
-  //
-  // uint32_t die_offset_base
-  // uint32_t atom_count
-  // atom_count Atoms
-
-  // Make these public so that they can be used as a general interface to
-  // the class.
+  /// The HeaderData describes the form of each set of data. In general this is
+  /// as a list of atoms (atom_count) where each atom contains a type (AtomType
+  /// type) of data, and an encoding form (form). In the case of data that is
+  /// referenced via DW_FORM_ref_* the DieOffsetBase is used to describe the
+  /// offset for all forms in the list of atoms. This also serves as a public
+  /// interface of sorts. When written to disk this will have the form:
+  ///
+  /// uint32_t DieOffsetBase
+  /// uint32_t atom_count
+  /// atom_count Atoms
+  ///
+  /// Make these public so that they can be used as a general interface to the
+  /// class.
   struct Atom {
-    uint16_t type; // enum AtomType
-    uint16_t form; // DWARF DW_FORM_ defines
+    /// enum AtomType.
+    uint16_t Type;
+    /// DWARF DW_FORM_ defines.
+    uint16_t Form;
 
-    constexpr Atom(uint16_t type, uint16_t form) : type(type), form(form) {}
+    constexpr Atom(uint16_t Type, uint16_t Form) : Type(Type), Form(Form) {}
 
 #ifndef NDEBUG
     void print(raw_ostream &OS) {
-      OS << "Type: " << dwarf::AtomTypeString(type) << "\n"
-         << "Form: " << dwarf::FormEncodingString(form) << "\n";
+      OS << "Type: " << dwarf::AtomTypeString(Type) << "\n"
+         << "Form: " << dwarf::FormEncodingString(Form) << "\n";
     }
 
     void dump() { print(dbgs()); }
 #endif
   };
 
 private:
   struct TableHeaderData {
-    uint32_t die_offset_base;
+    uint32_t DieOffsetBase;
     SmallVector<Atom, 3> Atoms;
 
     TableHeaderData(ArrayRef<Atom> AtomList, uint32_t offset = 0)
-        : die_offset_base(offset), Atoms(AtomList.begin(), AtomList.end()) {}
+        : DieOffsetBase(offset), Atoms(AtomList.begin(), AtomList.end()) {}
 
 #ifndef NDEBUG
     void print(raw_ostream &OS) {
-      OS << "die_offset_base: " << die_offset_base << "\n";
+      OS << "DieOffsetBase: " << DieOffsetBase << "\n";
       for (size_t i = 0; i < Atoms.size(); i++)
         Atoms[i].print(OS);
     }
 
     void dump() { print(dbgs()); }
 #endif
   };
 
-  // The data itself consists of a str_offset, a count of the DIEs in the
-  // hash and the offsets to the DIEs themselves.
-  // On disk each data section is ended with a 0 KeyType as the end of the
-  // hash chain.
-  // On output this looks like:
-  // uint32_t str_offset
-  // uint32_t hash_data_count
-  // HashData[hash_data_count]
+  /// The data itself consists of a str_offset, a count of the DIEs in the hash
+  /// and the offsets to the DIEs themselves. On disk each data section is
+  /// ended with a 0 KeyType as the end of the hash chain.
+  ///
+  /// On output this looks like:
+  ///
+  /// uint32_t str_offset
+  /// uint32_t hash_data_count
+  /// HashData[hash_data_count]
 public:
   struct HashDataContents {
     const DIE *Die;   // Offsets
 
     HashDataContents(const DIE *D) : Die(D) {}
 
 #ifndef NDEBUG
     void print(raw_ostream &OS) const {
case 2:
       return 0;
     default:
       break;
     }
     return 0;
   }
 
 private:
-  // String Data
+  /// String Data
   struct DataArray {
     DwarfStringPoolEntryRef Name;
     std::vector<HashDataContents *> Values;
   };
 
   friend struct HashData;
 
   struct HashData {
void print(raw_ostream &OS) {
         C->print(OS);
       }
     }
 
     void dump() { print(dbgs()); }
 #endif
   };
 
-  // Internal Functions
+  /// Emits the header for the table via the AsmPrinter.
   void EmitHeader(AsmPrinter *);
+
+  /// Walk through and emit the buckets for the table. Each index is an offset
+  /// into the list of hashes.
   void EmitBuckets(AsmPrinter *);
+
+  /// Walk through the buckets and emit the individual hashes for each bucket.
   void EmitHashes(AsmPrinter *);
+
+  /// Walk through the buckets and emit the individual offsets for each element
+  /// in each bucket. This is done via a symbol subtraction from the beginning
+  /// of the section. The non-section symbol will be output later when we emit
+  /// the actual data.
   void emitOffsets(AsmPrinter *, const MCSymbol *);
+
+  /// Walk through the buckets and emit the full data for each element in the
+  /// bucket. For the string case emit the dies and the various offsets.
+  /// Terminate each HashData bucket with 0.
   void EmitData(AsmPrinter *, DwarfDebug *D);
 
-  // Allocator for HashData and HashDataContents.
+  /// Allocator for HashData and HashDataContents.
   BumpPtrAllocator Allocator;
 
-  // Output Variables
+  /// Output Variables
   TableHeader Header;
   TableHeaderData HeaderData;
   std::vector<HashData *> Data;
 
   DataForAtomFunction DataForAtom;
 
   using StringEntries = StringMap<DataArray, BumpPtrAllocator &>;
   StringEntries Entries;
 
   using HashList = std::vector<HashData *>;
   HashList Hashes;
 
   using BucketList = std::vector<HashList>;
   BucketList Buckets;
 
-  // Public Implementation
 public:
   DwarfAccelTable(ArrayRef<DwarfAccelTable::Atom>,
                   DataForAtomFunction DataForAtom = DefaultGetDataForAtom);
   DwarfAccelTable(const DwarfAccelTable &) = delete;
   DwarfAccelTable &operator=(const DwarfAccelTable &) = delete;
 
+  /// Add name to the table.
   void AddName(DwarfStringPoolEntryRef Name, const DIE *Die);
+
+  /// Finalize the table. You cannot add more entries once the table was
+  /// finalized.
   void FinalizeTable(AsmPrinter *, StringRef);
+
+  /// Emit the entire data structure to the output file.
   void emit(AsmPrinter *, const MCSymbol *, DwarfDebug *);
 
 #ifndef NDEBUG
   void print(raw_ostream &OS);
   void dump() { print(dbgs()); }
 #endif
 };
 
 } // end namespace llvm
 
 #endif // LLVM_LIB_CODEGEN_ASMPRINTER_DWARFACCELTABLE_H

lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp

 #include <cstddef>
 #include <cstdint>
 #include <iterator>
 #include <limits>
 #include <vector>
 
 using namespace llvm;
 
-// The length of the header data is always going to be 4 + 4 + 4*NumAtoms.
+/// The length of the header data is always 4 + 4 + (4 * NumAtoms).
 DwarfAccelTable::DwarfAccelTable(ArrayRef<DwarfAccelTable::Atom> AtomList,
                                  DataForAtomFunction DataForAtom)
     : Header(8 + (AtomList.size() * 4)), HeaderData(AtomList),
       DataForAtom(std::move(DataForAtom)), Entries(Allocator) {}
 
 void DwarfAccelTable::AddName(DwarfStringPoolEntryRef Name, const DIE *die) {
   assert(Data.empty() && "Already finalized!");
   // If the string is in the list already then add this die to the list
for (size_t i = 0, e = Data.size(); i < e; ++i)
     uniques[i] = Data[i]->HashValue;
   array_pod_sort(uniques.begin(), uniques.end());
   std::vector<uint32_t>::iterator p =
       std::unique(uniques.begin(), uniques.end());
   uint32_t num = std::distance(uniques.begin(), p);
 
   // Then compute the bucket size, minimum of 1 bucket.
   if (num > 1024)
-    Header.bucket_count = num / 4;
+    Header.BucketCount = num / 4;
   else if (num > 16)
-    Header.bucket_count = num / 2;
+    Header.BucketCount = num / 2;
   else
-    Header.bucket_count = num > 0 ? num : 1;
+    Header.BucketCount = num > 0 ? num : 1;
 
-  Header.hashes_count = num;
+  Header.HashesCount = num;
 }
 
 /// Comparison predicate that sorts DIEs by their offset.
 static bool compareDIEs(const DwarfAccelTable::HashDataContents *A,
                         const DwarfAccelTable::HashDataContents *B) {
   return A->Die->getOffset() < B->Die->getOffset();
 }
 
void DwarfAccelTable::FinalizeTable(AsmPrinter *Asm, StringRef Prefix) {
   // Figure out how many buckets we need, then compute the bucket
   // contents and the final ordering. We'll emit the hashes and offsets
   // by doing a walk during the emission phase. We add temporary
   // symbols to the data so that we can reference them during the offset
   // later, we'll emit them when we emit the data.
   ComputeBucketCount();
 
   // Compute bucket contents and final ordering.
-  Buckets.resize(Header.bucket_count);
+  Buckets.resize(Header.BucketCount);
   for (size_t i = 0, e = Data.size(); i < e; ++i) {
-    uint32_t bucket = Data[i]->HashValue % Header.bucket_count;
+    uint32_t bucket = Data[i]->HashValue % Header.BucketCount;
     Buckets[bucket].push_back(Data[i]);
     Data[i]->Sym = Asm->createTempSymbol(Prefix);
   }
 
   // Sort the contents of the buckets by hash value so that hash
   // collisions end up together. Stable sort makes testing easier and
   // doesn't cost much more.
   for (size_t i = 0; i < Buckets.size(); ++i)
     std::stable_sort(Buckets[i].begin(), Buckets[i].end(),
                      [] (HashData *LHS, HashData *RHS) {
                        return LHS->HashValue < RHS->HashValue;
                      });
 }
 
-// Emits the header for the table via the AsmPrinter.
 void DwarfAccelTable::EmitHeader(AsmPrinter *Asm) {
   Asm->OutStreamer->AddComment("Header Magic");
-  Asm->EmitInt32(Header.magic);
+  Asm->EmitInt32(Header.Magic);
   Asm->OutStreamer->AddComment("Header Version");
-  Asm->EmitInt16(Header.version);
+  Asm->EmitInt16(Header.Version);
   Asm->OutStreamer->AddComment("Header Hash Function");
-  Asm->EmitInt16(Header.hash_function);
+  Asm->EmitInt16(Header.HashFunction);
   Asm->OutStreamer->AddComment("Header Bucket Count");
-  Asm->EmitInt32(Header.bucket_count);
+  Asm->EmitInt32(Header.BucketCount);
   Asm->OutStreamer->AddComment("Header Hash Count");
-  Asm->EmitInt32(Header.hashes_count);
+  Asm->EmitInt32(Header.HashesCount);
   Asm->OutStreamer->AddComment("Header Data Length");
-  Asm->EmitInt32(Header.header_data_len);
+  Asm->EmitInt32(Header.HeaderDataLen);
   Asm->OutStreamer->AddComment("HeaderData Die Offset Base");
-  Asm->EmitInt32(HeaderData.die_offset_base);
+  Asm->EmitInt32(HeaderData.DieOffsetBase);
   Asm->OutStreamer->AddComment("HeaderData Atom Count");
   Asm->EmitInt32(HeaderData.Atoms.size());
   for (size_t i = 0; i < HeaderData.Atoms.size(); i++) {
     Atom A = HeaderData.Atoms[i];
-    Asm->OutStreamer->AddComment(dwarf::AtomTypeString(A.type));
-    Asm->EmitInt16(A.type);
-    Asm->OutStreamer->AddComment(dwarf::FormEncodingString(A.form));
-    Asm->EmitInt16(A.form);
+    Asm->OutStreamer->AddComment(dwarf::AtomTypeString(A.Type));
+    Asm->EmitInt16(A.Type);
+    Asm->OutStreamer->AddComment(dwarf::FormEncodingString(A.Form));
+    Asm->EmitInt16(A.Form);
   }
 }
 
-// Walk through and emit the buckets for the table. Each index is
-// an offset into the list of hashes.
 void DwarfAccelTable::EmitBuckets(AsmPrinter *Asm) {
   unsigned index = 0;
   for (size_t i = 0, e = Buckets.size(); i < e; ++i) {
     Asm->OutStreamer->AddComment("Bucket " + Twine(i));
     if (!Buckets[i].empty())
       Asm->EmitInt32(index);
     else
       Asm->EmitInt32(std::numeric_limits<uint32_t>::max());
     // Buckets point in the list of hashes, not to the data. Do not
     // increment the index multiple times in case of hash collisions.
     uint64_t PrevHash = std::numeric_limits<uint64_t>::max();
     for (auto *HD : Buckets[i]) {
       uint32_t HashValue = HD->HashValue;
       if (PrevHash != HashValue)
         ++index;
       PrevHash = HashValue;
     }
   }
 }
 
-// Walk through the buckets and emit the individual hashes for each
-// bucket.
 void DwarfAccelTable::EmitHashes(AsmPrinter *Asm) {
   uint64_t PrevHash = std::numeric_limits<uint64_t>::max();
   for (size_t i = 0, e = Buckets.size(); i < e; ++i) {
     for (HashList::const_iterator HI = Buckets[i].begin(),
                                   HE = Buckets[i].end();
          HI != HE; ++HI) {
       uint32_t HashValue = (*HI)->HashValue;
       if (PrevHash == HashValue)
         continue;
       Asm->OutStreamer->AddComment("Hash in Bucket " + Twine(i));
       Asm->EmitInt32(HashValue);
       PrevHash = HashValue;
     }
   }
 }
 
-// Walk through the buckets and emit the individual offsets for each
-// element in each bucket. This is done via a symbol subtraction from the
-// beginning of the section. The non-section symbol will be output later
-// when we emit the actual data.
 void DwarfAccelTable::emitOffsets(AsmPrinter *Asm, const MCSymbol *SecBegin) {
   uint64_t PrevHash = std::numeric_limits<uint64_t>::max();
   for (size_t i = 0, e = Buckets.size(); i < e; ++i) {
     for (HashList::const_iterator HI = Buckets[i].begin(),
                                   HE = Buckets[i].end();
          HI != HE; ++HI) {
       uint32_t HashValue = (*HI)->HashValue;
       if (PrevHash == HashValue)
         continue;
       PrevHash = HashValue;
       Asm->OutStreamer->AddComment("Offset in Bucket " + Twine(i));
       MCContext &Context = Asm->OutStreamer->getContext();
       const MCExpr *Sub = MCBinaryExpr::createSub(
           MCSymbolRefExpr::create((*HI)->Sym, Context),
           MCSymbolRefExpr::create(SecBegin, Context), Context);
       Asm->OutStreamer->EmitValue(Sub, sizeof(uint32_t));
     }
   }
 }
 
-// Walk through the buckets and emit the full data for each element in
-// the bucket. For the string case emit the dies and the various offsets.
-// Terminate each HashData bucket with 0.
 void DwarfAccelTable::EmitData(AsmPrinter *Asm, DwarfDebug *D) {
   for (size_t i = 0, e = Buckets.size(); i < e; ++i) {
     uint64_t PrevHash = std::numeric_limits<uint64_t>::max();
     for (HashList::const_iterator HI = Buckets[i].begin(),
                                   HE = Buckets[i].end();
          HI != HE; ++HI) {
       // Terminate the previous entry if there is no hash collision
       // with the current one.
       if (PrevHash != std::numeric_limits<uint64_t>::max() &&
           PrevHash != (*HI)->HashValue)
         Asm->EmitInt32(0);
       // Remember to emit the label for our offset.
       Asm->OutStreamer->EmitLabel((*HI)->Sym);
       Asm->OutStreamer->AddComment((*HI)->Str);
       Asm->emitDwarfStringOffset((*HI)->Data.Name);
       Asm->OutStreamer->AddComment("Num DIEs");
       Asm->EmitInt32((*HI)->Data.Values.size());
       for (HashDataContents *HD : (*HI)->Data.Values) {
         // Emit data for Atoms.
         unsigned AtomIdx = 0;
         for (auto Atom : HeaderData.Atoms) {
           uint64_t I = DataForAtom(*HD, AtomIdx);
           Asm->OutStreamer->EmitIntValue(
-              I, DIEInteger(I).SizeOf(Asm, (dwarf::Form)Atom.form));
+              I, DIEInteger(I).SizeOf(Asm, (dwarf::Form)Atom.Form));
           AtomIdx++;
         }
       }
       PrevHash = (*HI)->HashValue;
     }
     // Emit the final end marker for the bucket.
     if (!Buckets[i].empty())
       Asm->EmitInt32(0);
   }
 }
 
-// Emit the entire data structure to the output file.
 void DwarfAccelTable::emit(AsmPrinter *Asm, const MCSymbol *SecBegin,
                            DwarfDebug *D) {
   // Emit the header.
   EmitHeader(Asm);
 
   // Emit the buckets.
   EmitBuckets(Asm);