[llvm] r238369 - AsmPrinter: Stop exposing underlying DIEValue list, NFC

David Blaikie dblaikie at gmail.com
Wed May 27 16:19:28 PDT 2015


On Wed, May 27, 2015 at 3:44 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

> Author: dexonsmith
> Date: Wed May 27 17:44:06 2015
> New Revision: 238369
>
> URL: http://llvm.org/viewvc/llvm-project?rev=238369&view=rev
> Log:
> AsmPrinter: Stop exposing underlying DIEValue list, NFC
>
> Change the `DIE` API to hide the implementation of the list of
> `DIEValue`s.
>
> Modified:
>     llvm/trunk/include/llvm/CodeGen/DIE.h
>     llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfFile.cpp
>     llvm/trunk/tools/dsymutil/DwarfLinker.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/DIE.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/DIE.h?rev=238369&r1=238368&r2=238369&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/DIE.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/DIE.h Wed May 27 17:44:06 2015
> @@ -499,7 +499,16 @@ public:
>    const std::vector<std::unique_ptr<DIE>> &getChildren() const {
>      return Children;
>    }
> -  const SmallVectorImpl<DIEValue> &getValues() const { return Values; }
> +
> +  typedef SmallVectorImpl<DIEValue>::const_iterator value_iterator;
> +  typedef iterator_range<value_iterator> value_range;
> +
> +  value_iterator begin_values() const { return Values.begin(); }
> +  value_iterator end_values() const { return Values.end(); }
>

Is this the prevailing naming convention for exposing a named range? I
think we have a fair few "foo_begin/foo_end" & not sure I've seen much
"end_foo/begin_foo", but I could be wrong.

(alternatively - (optionally add range-based APIs for anything you
currently need begin/end for, and) skip begin/end in favor of using the
range accessor below)


> +  value_range values() const {
> +    return llvm::make_range(begin_values(), end_values());
> +  }
> +
>    void setValue(unsigned I, DIEValue New) {
>      assert(I < Values.size());
>      Values[I] = New;
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp?rev=238369&r1=238368&r2=238369&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp Wed May 27
> 17:44:06 2015
> @@ -262,23 +262,21 @@ void AsmPrinter::emitDwarfDIE(const DIE
>                              dwarf::TagString(Die.getTag()));
>    EmitULEB128(Die.getAbbrevNumber());
>
> -  const SmallVectorImpl<DIEValue> &Values = Die.getValues();
> -
>    // Emit the DIE attribute values.
> -  for (unsigned i = 0, N = Values.size(); i < N; ++i) {
> -    dwarf::Attribute Attr = Values[i].getAttribute();
> -    dwarf::Form Form = Values[i].getForm();
> +  for (const auto &V : Die.values()) {
> +    dwarf::Attribute Attr = V.getAttribute();
> +    dwarf::Form Form = V.getForm();
>      assert(Form && "Too many attributes for DIE (check abbreviation)");
>
>      if (isVerbose()) {
>        OutStreamer->AddComment(dwarf::AttributeString(Attr));
>        if (Attr == dwarf::DW_AT_accessibility)
>          OutStreamer->AddComment(
> -
> dwarf::AccessibilityString(Values[i].getDIEInteger().getValue()));
> +            dwarf::AccessibilityString(V.getDIEInteger().getValue()));
>      }
>
>      // Emit an attribute using the defined form.
> -    Values[i].EmitValue(this, Form);
> +    V.EmitValue(this, Form);
>    }
>
>    // Emit the DIE children if any.
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp?rev=238369&r1=238368&r2=238369&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp Wed May 27 17:44:06 2015
> @@ -136,13 +136,11 @@ const DIE *DIE::getUnitOrNull() const {
>  }
>
>  DIEValue DIE::findAttribute(dwarf::Attribute Attribute) const {
> -  const SmallVectorImpl<DIEValue> &Values = getValues();
> -
>    // Iterate through all the attributes until we find the one we're
>    // looking for, if we can't find it return NULL.
> -  for (size_t i = 0; i < Values.size(); ++i)
> -    if (Values[i].getAttribute() == Attribute)
> -      return Values[i];
> +  for (const auto &V : values())
> +    if (V.getAttribute() == Attribute)
> +      return V;
>    return DIEValue();
>  }
>
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp?rev=238369&r1=238368&r2=238369&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.cpp Wed May 27 17:44:06 2015
> @@ -31,14 +31,12 @@ using namespace llvm;
>  /// \brief Grabs the string in whichever attribute is passed in and
> returns
>  /// a reference to it.
>  static StringRef getDIEStringAttr(const DIE &Die, uint16_t Attr) {
> -  const auto &Values = Die.getValues();
> -
>    // Iterate through all the attributes until we find the one we're
>    // looking for, if we can't find it return an empty string.
> -  for (size_t i = 0; i < Values.size(); ++i) {
> -    if (Values[i].getAttribute() == Attr)
> -      return Values[i].getDIEString().getString();
> -  }
> +  for (const auto &V : Die.values())
> +    if (V.getAttribute() == Attr)
> +      return V.getDIEString().getString();
> +
>    return StringRef("");
>  }
>
> @@ -118,18 +116,16 @@ void DIEHash::addParentContext(const DIE
>
>  // Collect all of the attributes for a particular DIE in single structure.
>  void DIEHash::collectAttributes(const DIE &Die, DIEAttrs &Attrs) {
> -  const SmallVectorImpl<DIEValue> &Values = Die.getValues();
> -
>  #define COLLECT_ATTR(NAME)
>      \
>    case dwarf::NAME:
>       \
> -    Attrs.NAME = Values[i];
>       \
> +    Attrs.NAME = V;
>       \
>      break
>
> -  for (size_t i = 0, e = Values.size(); i != e; ++i) {
> +  for (const auto &V : Die.values()) {
>      DEBUG(dbgs() << "Attribute: "
> -                 << dwarf::AttributeString(Values[i].getAttribute())
> +                 << dwarf::AttributeString(V.getAttribute())
>                   << " added.\n");
> -    switch (Values[i].getAttribute()) {
> +    switch (V.getAttribute()) {
>        COLLECT_ATTR(DW_AT_name);
>        COLLECT_ATTR(DW_AT_accessibility);
>        COLLECT_ATTR(DW_AT_address_class);
> @@ -267,9 +263,9 @@ void DIEHash::hashDIEEntry(dwarf::Attrib
>
>  // Hash all of the values in a block like set of values. This assumes that
>  // all of the data is going to be added as integers.
> -void DIEHash::hashBlockData(const SmallVectorImpl<DIEValue> &Values) {
> -  for (auto I = Values.begin(), E = Values.end(); I != E; ++I)
> -    Hash.update((uint64_t)I->getDIEInteger().getValue());
> +void DIEHash::hashBlockData(const DIE::value_range &Values) {
> +  for (const auto &V : Values)
> +    Hash.update((uint64_t)V.getDIEInteger().getValue());
>  }
>
>  // Hash the contents of a loclistptr class.
> @@ -342,10 +338,10 @@ void DIEHash::hashAttribute(DIEValue Val
>      addULEB128(dwarf::DW_FORM_block);
>      if (Value.getType() == DIEValue::isBlock) {
>        addULEB128(Value.getDIEBlock().ComputeSize(AP));
> -      hashBlockData(Value.getDIEBlock().getValues());
> +      hashBlockData(Value.getDIEBlock().values());
>      } else if (Value.getType() == DIEValue::isLoc) {
>        addULEB128(Value.getDIELoc().ComputeSize(AP));
> -      hashBlockData(Value.getDIELoc().getValues());
> +      hashBlockData(Value.getDIELoc().values());
>      } else {
>        // We could add the block length, but that would take
>        // a bit of work and not add a lot of uniqueness
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h?rev=238369&r1=238368&r2=238369&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIEHash.h Wed May 27 17:44:06 2015
> @@ -128,7 +128,7 @@ private:
>
>    /// \brief Hashes the data in a block like DIEValue, e.g. DW_FORM_block
> or
>    /// DW_FORM_exprloc.
> -  void hashBlockData(const SmallVectorImpl<DIEValue> &Values);
> +  void hashBlockData(const DIE::value_range &Values);
>
>    /// \brief Hashes the contents pointed to in the .debug_loc section.
>    void hashLocList(const DIELocList &LocList);
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=238369&r1=238368&r2=238369&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Wed May 27
> 17:44:06 2015
> @@ -242,7 +242,7 @@ void DwarfCompileUnit::initStmtList() {
>    MCSymbol *LineTableStartSym =
>        Asm->OutStreamer->getDwarfLineTableSymbol(getUniqueID());
>
> -  stmtListIndex = UnitDie.getValues().size();
> +  stmtListIndex = std::distance(UnitDie.begin_values(),
> UnitDie.end_values());
>
>    // DW_AT_stmt_list is a offset of line number information for this
>    // compile unit in debug_line section. For split dwarf this is
> @@ -255,7 +255,7 @@ void DwarfCompileUnit::initStmtList() {
>  }
>
>  void DwarfCompileUnit::applyStmtList(DIE &D) {
> -  D.addValue(UnitDie.getValues()[stmtListIndex]);
> +  D.addValue(UnitDie.begin_values()[stmtListIndex]);
>  }
>
>  void DwarfCompileUnit::attachLowHighPC(DIE &D, const MCSymbol *Begin,
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfFile.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfFile.cpp?rev=238369&r1=238368&r2=238369&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfFile.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfFile.cpp Wed May 27 17:44:06
> 2015
> @@ -97,13 +97,10 @@ unsigned DwarfFile::computeSizeAndOffset
>    // Start the size with the size of abbreviation code.
>    Offset += getULEB128Size(Die.getAbbrevNumber());
>
> -  const SmallVectorImpl<DIEValue> &Values = Die.getValues();
> -  const SmallVectorImpl<DIEAbbrevData> &AbbrevData = Abbrev.getData();
> -
>    // Size the DIE attribute values.
> -  for (unsigned i = 0, N = Values.size(); i < N; ++i)
> +  for (const auto &V : Die.values())
>      // Size attribute value.
> -    Offset += Values[i].SizeOf(Asm, AbbrevData[i].getForm());
> +    Offset += V.SizeOf(Asm, V.getForm());
>
>    // Get the children.
>    const auto &Children = Die.getChildren();
>
> Modified: llvm/trunk/tools/dsymutil/DwarfLinker.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=238369&r1=238368&r2=238369&view=diff
>
> ==============================================================================
> --- llvm/trunk/tools/dsymutil/DwarfLinker.cpp (original)
> +++ llvm/trunk/tools/dsymutil/DwarfLinker.cpp Wed May 27 17:44:06 2015
> @@ -68,11 +68,13 @@ struct PatchLocation {
>
>    PatchLocation() : Die(nullptr), Index(0) {}
>    PatchLocation(DIE &Die, unsigned Index) : Die(&Die), Index(Index) {}
> +  PatchLocation(DIE &Die)
> +      : Die(&Die), Index(std::distance(Die.begin_values(),
> Die.end_values())) {}
>
>    void set(uint64_t New) const {
>      assert(Die);
> -    assert(Index < Die->getValues().size());
> -    const auto &Old = Die->getValues()[Index];
> +    assert(Index < std::distance(Die->begin_values(), Die->end_values()));
> +    const auto &Old = Die->begin_values()[Index];
>      assert(Old.getType() == DIEValue::isInteger);
>      Die->setValue(Index,
>                    DIEValue(Old.getAttribute(), Old.getForm(),
> DIEInteger(New)));
> @@ -80,9 +82,9 @@ struct PatchLocation {
>
>    uint64_t get() const {
>      assert(Die);
> -    assert(Index < Die->getValues().size());
> -    assert(Die->getValues()[Index].getType() == DIEValue::isInteger);
> -    return Die->getValues()[Index].getDIEInteger().getValue();
> +    assert(Index < std::distance(Die->begin_values(), Die->end_values()));
> +    assert(Die->begin_values()[Index].getType() == DIEValue::isInteger);
> +    return Die->begin_values()[Index].getDIEInteger().getValue();
>    }
>  };
>
> @@ -1839,8 +1841,7 @@ unsigned DwarfLinker::cloneDieReferenceA
>      } else {
>        // A forward reference. Note and fixup later.
>        Attr = 0xBADDEF;
> -      Unit.noteForwardReference(NewRefDie, RefUnit,
> -                                PatchLocation(Die,
> Die.getValues().size()));
> +      Unit.noteForwardReference(NewRefDie, RefUnit, PatchLocation(Die));
>      }
>      Die.addValue(dwarf::Attribute(AttrSpec.Attr), dwarf::DW_FORM_ref_addr,
>                   DIEInteger(Attr));
> @@ -1956,14 +1957,13 @@ unsigned DwarfLinker::cloneScalarAttribu
>    }
>    DIEInteger Attr(Value);
>    if (AttrSpec.Attr == dwarf::DW_AT_ranges)
> -    Unit.noteRangeAttribute(Die, PatchLocation(Die,
> Die.getValues().size()));
> +    Unit.noteRangeAttribute(Die, PatchLocation(Die));
>    // A more generic way to check for location attributes would be
>    // nice, but it's very unlikely that any other attribute needs a
>    // location list.
>    else if (AttrSpec.Attr == dwarf::DW_AT_location ||
>             AttrSpec.Attr == dwarf::DW_AT_frame_base)
> -    Unit.noteLocationAttribute(PatchLocation(Die, Die.getValues().size()),
> -                               Info.PCOffset);
> +    Unit.noteLocationAttribute(PatchLocation(Die), Info.PCOffset);
>    else if (AttrSpec.Attr == dwarf::DW_AT_declaration && Value)
>      Info.IsDeclaration = true;
>
> @@ -2329,13 +2329,13 @@ void DwarfLinker::patchLineTableForUnit(
>
>    // Update the cloned DW_AT_stmt_list with the correct debug_line offset.
>    if (auto *OutputDIE = Unit.getOutputUnitDIE()) {
> -    const auto &Values = OutputDIE->getValues();
> -    auto Stmt =
> -        std::find_if(Values.begin(), Values.end(), [](const DIEValue
> &Value) {
> -          return Value.getAttribute() == dwarf::DW_AT_stmt_list;
> -        });
> -    assert(Stmt < Values.end() && "Didn't find DW_AT_stmt_list in cloned
> DIE!");
> -    OutputDIE->setValue(Stmt - Values.begin(),
> +    auto Stmt = std::find_if(OutputDIE->begin_values(),
> OutputDIE->end_values(),
> +                             [](const DIEValue &Value) {
> +      return Value.getAttribute() == dwarf::DW_AT_stmt_list;
> +    });
> +    assert(Stmt != OutputDIE->end_values() &&
> +           "Didn't find DW_AT_stmt_list in cloned DIE!");
> +    OutputDIE->setValue(Stmt - OutputDIE->begin_values(),
>                          DIEValue(Stmt->getAttribute(), Stmt->getForm(),
>
> DIEInteger(Streamer->getLineSectionSize())));
>    }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150527/87d21849/attachment.html>


More information about the llvm-commits mailing list