[DWARF] Add findAttribute to DWARFAbbreviationDeclaration

[ayermolo]

Summary

This API is used by BOLT to patch Abbrev section.

[ayermolo]

Some profiling of an internal service:
llvm-dwarfdump --debug-abbrev
Internal service

[46] .debug_abbrev PROGBITS 0000000000000000 11f847d18 11698d3 00 0 0 1
0:03.31 real, 2.98 user, 0.32 sys, 0 amem, 408924 mmem

BASE
0:03.11 real, 2.85 user, 0.25 sys, 0 amem, 308392 mmem

I tried on clang-14 but it was too noisy.

[ayermolo]

clang-format

[dblaikie]

These values could be computed from the index in AttributeSpecs, right? Maybe that'd be better, rather than adding memory overhead to every use of libDebugInfoDWARF?

[ayermolo]

In D107875#2942329, @dblaikie wrote:

These values could be computed from the index in AttributeSpecs, right? Maybe that'd be better, rather than adding memory overhead to every use of libDebugInfoDWARF?

Sorry it's not clear to me how it can be calculated index in AttributeSpecs.
You mean like iterate over abbrev section and build a map?

[dblaikie]

In D107875#2944723, @ayermolo wrote:

In D107875#2942329, @dblaikie wrote:

These values could be computed from the index in AttributeSpecs, right? Maybe that'd be better, rather than adding memory overhead to every use of libDebugInfoDWARF?

Sorry it's not clear to me how it can be calculated index in AttributeSpecs.
You mean like iterate over abbrev section and build a map?

Sorry, no, I mean this data is computed by the for loop in DWARFAbbreviationDeclaration::getAttributeValue, for instance - so the patch seems like it's proposing a shift in the time/space tradeoff (adding more memory usage to avoid having to do the compute/time work done by that loop in getAttributeValue) & I'm not sure that shift is necessarily justified - I'd be inclined to keep with the status quo and suggest these values should be recomputed when needed by walking the AttributeSpecs in a DWARFAbbreviationDeclaration.

[ayermolo]

Thanks for feedback. Abandoning this diff. @rafaelauler will look on our side how we can change implementation to avoid this.

llvm/include/llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h

 
 class DWARFFormValue;
 class DWARFUnit;
 class raw_ostream;
 
 class DWARFAbbreviationDeclaration {
 public:
   struct AttributeSpec {
-    AttributeSpec(dwarf::Attribute A, dwarf::Form F, int64_t Value)
-        : Attr(A), Form(F), Value(Value) {
+    AttributeSpec(dwarf::Attribute A, dwarf::Form F, int64_t Value,
+                  uint32_t AttrOffset = -1U, uint32_t FormOffset = -1U)
+        : Attr(A), Form(F), AttrOffset(AttrOffset), FormOffset(FormOffset),
+          Value(Value) {
       assert(isImplicitConst());
     }
-    AttributeSpec(dwarf::Attribute A, dwarf::Form F, Optional<uint8_t> ByteSize)
-        : Attr(A), Form(F) {
+    AttributeSpec(dwarf::Attribute A, dwarf::Form F, Optional<uint8_t> ByteSize,
+                  uint32_t AttrOffset = -1U, uint32_t FormOffset = -1U)
+        : Attr(A), Form(F), AttrOffset(AttrOffset), FormOffset(FormOffset) {
       assert(!isImplicitConst());
       this->ByteSize.HasByteSize = ByteSize.hasValue();
       if (this->ByteSize.HasByteSize)
         this->ByteSize.ByteSize = *ByteSize;
     }
 
     dwarf::Attribute Attr;
     dwarf::Form Form;
+    uint32_t AttrOffset;
+    uint32_t FormOffset;
 
   private:
     /// The following field is used for ByteSize for non-implicit_const
     /// attributes and as value for implicit_const ones, indicated by
     /// Form == DW_FORM_implicit_const.
     /// The following cases are distinguished:
     /// * Form != DW_FORM_implicit_const and HasByteSize is true:
     ///     ByteSize contains the fixed size in bytes for the Form in this
bool getAttrIsImplicitConstByIndex(uint32_t idx) const {
     return AttributeSpecs[idx].isImplicitConst();
   }
 
   int64_t getAttrImplicitConstValueByIndex(uint32_t idx) const {
     assert(idx < AttributeSpecs.size());
     return AttributeSpecs[idx].getImplicitConstValue();
   }
 
+  const AttributeSpec *findAttribute(dwarf::Attribute Attr) const;
+
   /// Get the index of the specified attribute.
   ///
   /// Searches the this abbreviation declaration for the index of the specified
   /// attribute.
   ///
   /// \param attr DWARF attribute to search for.
   /// \returns Optional index of the attribute if found, None otherwise.
   Optional<uint32_t> findAttributeIndex(dwarf::Attribute attr) const;

llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp

DWARFAbbreviationDeclaration::extract(DataExtractor Data,
   HasChildren = (ChildrenByte == DW_CHILDREN_yes);
   // Assign a value to our optional FixedAttributeSize member variable. If
   // this member variable still has a value after the while loop below, then
   // all attribute data in this abbreviation declaration has a fixed byte size.
   FixedAttributeSize = FixedSizeInfo();
 
   // Read all of the abbreviation attributes and forms.
   while (true) {
+    uint32_t AOff = *OffsetPtr;
     auto A = static_cast<Attribute>(Data.getULEB128(OffsetPtr));
+    uint32_t FOff = *OffsetPtr;
     auto F = static_cast<Form>(Data.getULEB128(OffsetPtr));
     if (A && F) {
       bool IsImplicitConst = (F == DW_FORM_implicit_const);
       if (IsImplicitConst) {
         int64_t V = Data.getSLEB128(OffsetPtr);
-        AttributeSpecs.push_back(AttributeSpec(A, F, V));
+        AttributeSpecs.push_back(AttributeSpec(A, F, V, AOff, FOff));
         continue;
       }
       Optional<uint8_t> ByteSize;
       // If this abbrevation still has a fixed byte size, then update the
       // FixedAttributeSize as needed.
       switch (F) {
       case DW_FORM_addr:
         if (FixedAttributeSize)
if (A && F) {
         }
         // Indicate we no longer have a fixed byte size for this
         // abbreviation by clearing the FixedAttributeSize optional value
         // so it doesn't have a value.
         FixedAttributeSize.reset();
         break;
       }
       // Record this attribute and its fixed size if it has one.
-      AttributeSpecs.push_back(AttributeSpec(A, F, ByteSize));
+      AttributeSpecs.push_back(AttributeSpec(A, F, ByteSize, AOff, FOff));
     } else if (A == 0 && F == 0) {
       // We successfully reached the end of this abbreviation declaration
       // since both attribute and form are zero.
       break;
     } else {
       // Attribute and form pairs must either both be non-zero, in which case
       // they are added to the abbreviation declaration, or both be zero to
       // terminate the abbrevation declaration. In this case only one was
for (const AttributeSpec &Spec : AttributeSpecs) {
     OS << formatv("\t{0}\t{1}", Spec.Attr, Spec.Form);
     if (Spec.isImplicitConst())
       OS << '\t' << Spec.getImplicitConstValue();
     OS << '\n';
   }
   OS << '\n';
 }
 
+const DWARFAbbreviationDeclaration::AttributeSpec *
+DWARFAbbreviationDeclaration::findAttribute(dwarf::Attribute Attr) const {
+  for (uint32_t i = 0, e = AttributeSpecs.size(); i != e; ++i) {
+    if (AttributeSpecs[i].Attr == Attr)
+      return &AttributeSpecs[i];
+  }
+  return nullptr;
+}
+
 Optional<uint32_t>
 DWARFAbbreviationDeclaration::findAttributeIndex(dwarf::Attribute Attr) const {
   for (uint32_t i = 0, e = AttributeSpecs.size(); i != e; ++i) {
     if (AttributeSpecs[i].Attr == Attr)
       return i;
   }
   return None;
 }