[PATCH] D107874: [DWARF] Find offset of attribute.

[Alexander Yermolovich via Phabricator via llvm-commits]

ayermolo added a comment.

In D107874#2942339 <https://reviews.llvm.org/D107874#2942339>, @dblaikie wrote:

> Broadly I'm a bit hesitant about the whole direction of adding various features to libDebugInfoDWARF for writing DWARF - since it hasn't been used for that so far, it's a fairly significant change in direction/API, even if the particular features needed so far are small.
>
> How does llvm-dsymutil handle this? Does it use libDebugInfoDWARF for modifying DWARF, or does it do something else? (& broadly: Why are BOLT's needs categorically different from llvm-dsymutil's needs that motivate different API design here?)

I looked in to dsymutil in llvm tools. Looks like it relies on DWARFLinker. Looking through that closest I found is

1. patchRangesForUnit This uses DWARFLInker data structures and DWARFDataExtractor to iterate over all the ranges, patches them and writes them out.
2. cloneAllCompileUnits --> cloneDIE This goes through DIEs in systematic way keeping track of offset as it clones a die.

I believe BOLT access pattern is more generic where we access context specific values in DIE in various parts of code. Both using getAttrFieldOffsetForUnit, and DWARDie::find explicilty.
Some of the examples.
getAttrFieldOffsetForUnit
https://github.com/facebookincubator/BOLT/blob/37e51a3d450f371b811bf4ee5c0575d5e7c97e9c/bolt/src/DWARFRewriter.cpp#L704
DWARFDie::find
https://github.com/facebookincubator/BOLT/blob/37e51a3d450f371b811bf4ee5c0575d5e7c97e9c/bolt/src/DWARFRewriter.cpp#L170
https://github.com/facebookincubator/BOLT/blob/37e51a3d450f371b811bf4ee5c0575d5e7c97e9c/bolt/src/DWARFRewriter.cpp#L197



================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFAbbreviationDeclaration.cpp:155-184
   Optional<uint32_t> MatchAttrIndex = findAttributeIndex(Attr);
   if (!MatchAttrIndex)
     return None;
 
   auto DebugInfoData = U.getDebugInfoExtractor();
 
   // Add the byte size of ULEB that for the abbrev Code so we can start
----------------
dblaikie wrote:
> Might it make sense to decompose this function into two (the original getAttributeValue could remain - but it'd call two other functions to do the work - one that computes the offset+AttributeSpec, and another that uses that to extract the value.
> 
> Then the BOLT use case could use the first function to get the offset if it wants it, without modifying the existing API to have an extra/optional out parameter that's a bit strange for its use case? 
Maybe break it up in to two APIs. one not taking offset (under the hood invoking) one that invokes one which takes it as none optional parameter?
There are cases in BOLT where we want both the value and offset.
https://github.com/facebookincubator/BOLT/blob/37e51a3d450f371b811bf4ee5c0575d5e7c97e9c/bolt/src/DWARFRewriter.cpp#L446
Somewhat similar to extractValue in DWARFFormValue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107874/new/

https://reviews.llvm.org/D107874