<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 8, 2013 at 4:45 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: echristo<br>
Date: Thu Aug  8 18:45:55 2013<br>
New Revision: 188028<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=188028&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=188028&view=rev</a><br>
Log:<br>
Move hash computation code into a separate class and file.<br>
<br>
No functional change intended.<br>
<br>
Added:<br>
    llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp<br>
    llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h<br>
Modified:<br>
    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
<br>
Added: llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp?rev=188028&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp?rev=188028&view=auto</a><br>

==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp (added)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp Thu Aug  8 18:45:55 2013<br>
@@ -0,0 +1,136 @@<br>
+//===-- llvm/CodeGen/DIEHash.cpp - Dwarf Hashing Framework ----------------===//<br>
+//<br>
+//                     The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+// This file contains support for DWARF4 hashing of DIEs.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#define DEBUG_TYPE "dwarfdebug"<br>
+<br>
+#include "DIE.h"<br>
+#include "DIEHash.h"<br>
+#include "DwarfCompileUnit.h"<br>
+#include "llvm/ADT/ArrayRef.h"<br>
+#include "llvm/ADT/StringRef.h"<br>
+#include "llvm/Support/Debug.h"<br>
+#include "llvm/Support/Dwarf.h"<br>
+#include "llvm/Support/Endian.h"<br>
+#include "llvm/Support/MD5.h"<br>
+#include "llvm/Support/raw_ostream.h"<br>
+<br>
+using namespace llvm;<br>
+<br>
+/// \brief Grabs the string in whichever attribute is passed in and returns<br>
+/// a reference to it.<br>
+static StringRef getDIEStringAttr(DIE *Die, uint16_t Attr) {<br>
+  const SmallVectorImpl<DIEValue *> &Values = Die->getValues();<br>
+  const DIEAbbrev &Abbrevs = Die->getAbbrev();<br>
+<br>
+  // Iterate through all the attributes until we find the one we're<br>
+  // looking for, if we can't find it return an empty string.<br>
+  for (size_t i = 0; i < Values.size(); ++i) {<br>
+    if (Abbrevs.getData()[i].getAttribute() == Attr) {<br>
+      DIEValue *V = Values[i];<br>
+      assert(isa<DIEString>(V) && "String requested. Not a string.");<br></blockquote><div><br></div><div>cast already asserts when !isa - so this assert is unnecessary</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

+      DIEString *S = cast<DIEString>(V);<br>
+      return S->getString();<br>
+    }<br>
+  }<br>
+  return StringRef("");<br>
+}<br>
+<br>
+/// \brief Adds the string in \p Str to the hash. This also hashes<br>
+/// a trailing NULL with the string.<br>
+void DIEHash::addString(StringRef Str) {<br>
+  DEBUG(dbgs() << "Adding string " << Str << " to hash.\n");<br>
+  Hash.update(Str);<br>
+  Hash.update(makeArrayRef((uint8_t)'\0'));<br>
+}<br>
+<br>
+// FIXME: The LEB128 routines are copied and only slightly modified out of<br>
+// LEB128.h.<br>
+<br>
+/// \brief Adds the unsigned in \p Value to the hash encoded as a ULEB128.<br>
+void DIEHash::addULEB128(uint64_t Value) {<br>
+  DEBUG(dbgs() << "Adding ULEB128 " << Value << " to hash.\n");<br>
+  do {<br>
+    uint8_t Byte = Value & 0x7f;<br>
+    Value >>= 7;<br>
+    if (Value != 0)<br>
+      Byte |= 0x80; // Mark this byte to show that more bytes will follow.<br>
+    Hash.update(Byte);<br>
+  } while (Value != 0);<br>
+}<br>
+<br>
+/// \brief Including \p Parent adds the context of Parent to the hash..<br>
+void DIEHash::addParentContext(DIE *Parent) {<br>
+<br>
+  DEBUG(dbgs() << "Adding parent context to hash...\n");<br>
+<br>
+  // [7.27.2] For each surrounding type or namespace beginning with the<br>
+  // outermost such construct...<br>
+  SmallVector<DIE *, 1> Parents;<br>
+  while (Parent->getTag() != dwarf::DW_TAG_compile_unit) {<br>
+    Parents.push_back(Parent);<br>
+    Parent = Parent->getParent();<br>
+  }<br>
+<br>
+  // Reverse iterate over our list to go from the outermost construct to the<br>
+  // innermost.<br>
+  for (SmallVectorImpl<DIE *>::reverse_iterator I = Parents.rbegin(),<br>
+                                                E = Parents.rend();<br>
+       I != E; ++I) {<br>
+    DIE *Die = *I;<br>
+<br>
+    // ... Append the letter "C" to the sequence...<br>
+    addULEB128('C');<br>
+<br>
+    // ... Followed by the DWARF tag of the construct...<br>
+    addULEB128(Die->getTag());<br>
+<br>
+    // ... Then the name, taken from the DW_AT_name attribute.<br>
+    StringRef Name = getDIEStringAttr(Die, dwarf::DW_AT_name);<br>
+    DEBUG(dbgs() << "... adding context: " << Name << "\n");<br>
+    if (!Name.empty())<br>
+      addString(Name);<br>
+  }<br>
+}<br>
+<br>
+/// This is based on the type signature computation given in section 7.27 of the<br>
+/// DWARF4 standard. It is the md5 hash of a flattened description of the DIE<br>
+/// with<br>
+/// the exception that we are hashing only the context and the name of the type.<br>
+uint64_t DIEHash::computeDIEODRSignature(DIE *Die) {<br>
+<br>
+  // Add the contexts to the hash. We won't be computing the ODR hash for<br>
+  // function local types so it's safe to use the generic context hashing<br>
+  // algorithm here.<br>
+  // FIXME: If we figure out how to account for linkage in some way we could<br>
+  // actually do this with a slight modification to the parent hash algorithm.<br>
+  DIE *Parent = Die->getParent();<br>
+  if (Parent)<br>
+    addParentContext(Parent);<br>
+<br>
+  // Add the current DIE information.<br>
+<br>
+  // Add the DWARF tag of the DIE.<br>
+  addULEB128(Die->getTag());<br>
+<br>
+  // Add the name of the type to the hash.<br>
+  addString(getDIEStringAttr(Die, dwarf::DW_AT_name));<br>
+<br>
+  // Now get the result.<br>
+  MD5::MD5Result Result;<br>
+  Hash.final(Result);<br>
+<br>
+  // ... take the least significant 8 bytes and return those. Our MD5<br>
+  // implementation always returns its results in little endian, swap bytes<br>
+  // appropriately.<br>
+  return *reinterpret_cast<support::ulittle64_t *>(Result + 8);<br>
+}<br>
<br>
Added: llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h?rev=188028&view=auto" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h?rev=188028&view=auto</a><br>

==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h (added)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h Thu Aug  8 18:45:55 2013<br>
@@ -0,0 +1,46 @@<br>
+//===-- llvm/CodeGen/DIEHash.h - Dwarf Hashing Framework -------*- C++ -*--===//<br>
+//<br>
+//                     The LLVM Compiler Infrastructure<br>
+//<br>
+// This file is distributed under the University of Illinois Open Source<br>
+// License. See LICENSE.TXT for details.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+//<br>
+// This file contains support for DWARF4 hashing of DIEs.<br>
+//<br>
+//===----------------------------------------------------------------------===//<br>
+<br>
+#include "llvm/Support/MD5.h"<br>
+<br>
+namespace llvm {<br>
+<br>
+class CompileUnit;<br>
+<br>
+/// \brief An object containing the capability of hashing and adding hash<br>
+/// attributes onto a DIE.<br>
+class DIEHash {<br>
+public:<br>
+  /// \brief Initializes. The hash is default initialized.<br>
+  DIEHash() {}<br></blockquote><div><br></div><div>You could leave this ctor implicit. Though I'm not sure about this whole class as an abstraction - it has one stateless function. Why isn't that just a free function?</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+  /// \brief Computes the ODR signature<br>
+  uint64_t computeDIEODRSignature(DIE *Die);</blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+  // Helper routines to process parts of a DIE.<br>
+ private:<br>
+  /// \brief Adds the parent context of \param Die to the hash.<br>
+  void addParentContext(DIE *Die);<br>
+<br>
+  // Routines that add DIEValues to the hash.<br>
+private:<br>
+  /// \brief Encodes and adds \param Value to the hash as a ULEB128.<br>
+  void addULEB128(uint64_t Value);<br>
+<br>
+  /// \brief Adds \param Str to the hash and includes a NULL byte.<br>
+  void addString(StringRef Str);<br>
+<br>
+private:<br>
+  MD5 Hash;<br>
+};<br>
+}<br>
<br>
Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=188028&r1=188027&r2=188028&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=188028&r1=188027&r2=188028&view=diff</a><br>

==============================================================================<br>
--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Thu Aug  8 18:45:55 2013<br>
@@ -14,6 +14,7 @@<br>
 #define DEBUG_TYPE "dwarfdebug"<br>
 #include "DwarfDebug.h"<br>
 #include "DIE.h"<br>
+#include "DIEHash.h"<br>
 #include "DwarfAccelTable.h"<br>
 #include "DwarfCompileUnit.h"<br>
 #include "llvm/ADT/STLExtras.h"<br>
@@ -962,8 +963,7 @@ void DwarfDebug::collectDeadVariables()<br>
   DeleteContainerSeconds(DeadFnScopeMap);<br>
 }<br>
<br>
-// Type Signature [7.27] computation code.<br>
-typedef ArrayRef<uint8_t> HashValue;<br>
+// Type Signature [7.27] and ODR Hash code.<br>
<br>
 /// \brief Grabs the string in whichever attribute is passed in and returns<br>
 /// a reference to it. Returns "" if the attribute doesn't exist.<br>
@@ -976,100 +976,6 @@ static StringRef getDIEStringAttr(DIE *D<br>
   return StringRef("");<br>
 }<br>
<br>
-/// \brief Adds the string in \p Str to the hash in \p Hash. This also hashes<br>
-/// a trailing NULL with the string.<br>
-static void addStringToHash(MD5 &Hash, StringRef Str) {<br>
-  DEBUG(dbgs() << "Adding string " << Str << " to hash.\n");<br>
-  Hash.update(Str);<br>
-  Hash.update(makeArrayRef((uint8_t)'\0'));<br>
-}<br>
-<br>
-// FIXME: These are copied and only slightly modified out of LEB128.h.<br>
-<br>
-/// \brief Adds the unsigned in \p N to the hash in \p Hash. This also encodes<br>
-/// the unsigned as a ULEB128.<br>
-static void addULEB128ToHash(MD5 &Hash, uint64_t Value) {<br>
-  DEBUG(dbgs() << "Adding ULEB128 " << Value << " to hash.\n");<br>
-  do {<br>
-    uint8_t Byte = Value & 0x7f;<br>
-    Value >>= 7;<br>
-    if (Value != 0)<br>
-      Byte |= 0x80; // Mark this byte to show that more bytes will follow.<br>
-    Hash.update(Byte);<br>
-  } while (Value != 0);<br>
-}<br>
-<br>
-/// \brief Including \p Parent adds the context of Parent to \p Hash.<br>
-static void addParentContextToHash(MD5 &Hash, DIE *Parent) {<br>
-<br>
-  DEBUG(dbgs() << "Adding parent context to hash...\n");<br>
-<br>
-  // [7.27.2] For each surrounding type or namespace beginning with the<br>
-  // outermost such construct...<br>
-  SmallVector<DIE *, 1> Parents;<br>
-  while (Parent->getTag() != dwarf::DW_TAG_compile_unit) {<br>
-    Parents.push_back(Parent);<br>
-    Parent = Parent->getParent();<br>
-  }<br>
-<br>
-  // Reverse iterate over our list to go from the outermost construct to the<br>
-  // innermost.<br>
-  for (SmallVectorImpl<DIE *>::reverse_iterator I = Parents.rbegin(),<br>
-                                                E = Parents.rend();<br>
-       I != E; ++I) {<br>
-    DIE *Die = *I;<br>
-<br>
-    // ... Append the letter "C" to the sequence...<br>
-    addULEB128ToHash(Hash, 'C');<br>
-<br>
-    // ... Followed by the DWARF tag of the construct...<br>
-    addULEB128ToHash(Hash, Die->getTag());<br>
-<br>
-    // ... Then the name, taken from the DW_AT_name attribute.<br>
-    StringRef Name = getDIEStringAttr(Die, dwarf::DW_AT_name);<br>
-    DEBUG(dbgs() << "... adding context: " << Name << "\n");<br>
-    if (!Name.empty())<br>
-      addStringToHash(Hash, Name);<br>
-  }<br>
-}<br>
-<br>
-/// This is based on the type signature computation given in section 7.27 of the<br>
-/// DWARF4 standard. It is the md5 hash of a flattened description of the DIE with<br>
-/// the exception that we are hashing only the context and the name of the type.<br>
-static void addDIEODRSignature(MD5 &Hash, CompileUnit *CU, DIE *Die) {<br>
-<br>
-  // Add the contexts to the hash. We won't be computing the ODR hash for<br>
-  // function local types so it's safe to use the generic context hashing<br>
-  // algorithm here.<br>
-  // FIXME: If we figure out how to account for linkage in some way we could<br>
-  // actually do this with a slight modification to the parent hash algorithm.<br>
-  DIE *Parent = Die->getParent();<br>
-  if (Parent)<br>
-    addParentContextToHash(Hash, Parent);<br>
-<br>
-  // Add the current DIE information.<br>
-<br>
-  // Add the DWARF tag of the DIE.<br>
-  addULEB128ToHash(Hash, Die->getTag());<br>
-<br>
-  // Add the name of the type to the hash.<br>
-  addStringToHash(Hash, getDIEStringAttr(Die, dwarf::DW_AT_name));<br>
-<br>
-  // Now get the result.<br>
-  MD5::MD5Result Result;<br>
-  Hash.final(Result);<br>
-<br>
-  // ... take the least significant 8 bytes and store those as the attribute.<br>
-  // Our MD5 implementation always returns its results in little endian, swap<br>
-  // bytes appropriately.<br>
-  uint64_t Signature = *reinterpret_cast<support::ulittle64_t *>(Result + 8);<br>
-<br>
-  // FIXME: This should be added onto the type unit, not the type, but this<br>
-  // works as an intermediate stage.<br>
-  CU->addUInt(Die, dwarf::DW_AT_GNU_odr_signature, dwarf::DW_FORM_data8,<br>
-              Signature);<br>
-}<br>
-<br>
 /// Return true if the current DIE is contained within an anonymous namespace.<br>
 static bool isContainedInAnonNamespace(DIE *Die) {<br>
   DIE *Parent = Die->getParent();<br>
@@ -1090,7 +996,7 @@ static bool shouldAddODRHash(CompileUnit<br>
   return CU->getLanguage() == dwarf::DW_LANG_C_plus_plus &&<br>
          getDIEStringAttr(Die, dwarf::DW_AT_name) != "" &&<br>
          !isContainedInAnonNamespace(Die);<br>
- }<br>
+}<br>
<br>
 void DwarfDebug::finalizeModuleInfo() {<br>
   // Collect info for variables that were optimized out.<br>
@@ -1111,12 +1017,16 @@ void DwarfDebug::finalizeModuleInfo() {<br>
   // out type.<br>
   // FIXME: Do type splitting.<br>
   for (unsigned i = 0, e = TypeUnits.size(); i != e; ++i) {<br>
-    MD5 Hash;<br>
     DIE *Die = TypeUnits[i];<br>
+    DIEHash Hash;<br>
     // If we've requested ODR hashes and it's applicable for an ODR hash then<br>
     // add the ODR signature now.<br>
+    // FIXME: This should be added onto the type unit, not the type, but this<br>
+    // works as an intermediate stage.<br>
     if (GenerateODRHash && shouldAddODRHash(CUMap.begin()->second, Die))<br>
-      addDIEODRSignature(Hash, CUMap.begin()->second, Die);<br>
+      CUMap.begin()->second->addUInt(Die, dwarf::DW_AT_GNU_odr_signature,<br>
+                                     dwarf::DW_FORM_data8,<br>
+                                     Hash.computeDIEODRSignature(Die));<br>
   }<br>
<br>
    // Compute DIE offsets and sizes.<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>