Adding llvm DWARFDie APIs to LLDB DWARFDIE (original) (raw)

September 23, 2024, 10:29pm 1

[DWARF] Generalize DWARFTypePrinter to a template class by ZequanWu · Pull Request #109459 · llvm/llvm-project · GitHub generalizes llvm’s DWARFTypePrinter class to a template class with Dwarf DIE type as type parameter so it can be used for both llvm tools and lldb. The problem is that llvm’s DWARFDie has different APIs than lldb’s DWARFDIE. In order for lldb to use the template class, we needs to add those APIs used in the template class. However, lldb has different naming conversion for functions than llvm.

So, the questions is: Is it okay to add llvm’s DWARFDie’s APIs to lldb’s DWARFDIE despite it’s introducing llvm’s naming conversion?

In longer term, should we migrate lldb’s DWARFDIE APIs to be compatible to llvm’s DWARFDie?

Note:

  1. [DWARF] Generalize DWARFTypePrinter to a template class by ZequanWu · Pull Request #109459 · llvm/llvm-project · GitHub is a split of lldb simplified template names rebuild without clang ast by dwblaikie · Pull Request #90008 · llvm/llvm-project · GitHub which simplifies how fully qualified names are constructed in lldb. It only walks through the DIE trees to construct the names from simplified template names without resolving any reference types nor creating Clang AST, as lldb’s current approach, which is a big overhead.
  2. This post is not about unifying lldb DWARF DIE types with llvm DWARF DIE types, which has a much larger scope. Maybe this work could be the first step toward unifying these two types in the future.

@dblaikie @Michael137 @labath

jingham September 23, 2024, 10:52pm 2

IMO so long as the class is in LLDB and especially if there’s a mirror class on the LLVM side, being able to see that you’re working on one side or the other by naming convention is useful. But I don’t actively work on the lldb DWARF parser, so I’ll defer to folks who are more engaged on that side of lldb.

CC @bulbazord who was working on unifying some of the LLVM and LLDB DWARF bits a while back

dblaikie September 24, 2024, 6:46pm 4

I don’t have strong feelings or investment here, but perhaps some options to consider:

I think it’ll be tricky to have both naming conventions in the one class, especially if they’re required (if we add some LLVM API names and remove the equivalent lldb ones). So I /think/ the better spots here are either:

or

That’s my rough guess, at least? And that probably looks like doing the former rather than the latter is the right balance at the moment, but perhaps other folks feel differently.

Are there other points on the spectrum folks think are valid intermediate landing points? (even renaming everything is still an intermediate point - to the ultimate goal of LLDB’s DWARFDie/etc API sinking down into/merging with LLVM’s fully)

labath September 27, 2024, 11:38am 5

Do you have a list of functions that would be touched by this?

If the list is something like “all the DIE tree traversal methods and ~nothing else”, then I think we could just remove the LLDB functions in favour of the llvm names (I don’t think we need getParent and GetParent). If the list is more ad-hoc, then we may need to do something like what David suggested.

ZequanWu October 7, 2024, 9:40pm 6

It’s more than just DIE tree traversal methods. The list is very ad-hoc.

I’m in favor of this approach:

ZequanWu October 18, 2024, 5:57pm 7

I created the PR for using DWARFTypePrinter in lldb to compute the type name:

Here’s the list of APIs added in lldb in order to use DWARFTypePrinter: