[Lldb-commits] [PATCH] D96236: [lldb] DWZ 1/9: Pass main DWARFUnit * along DWARFDIEs

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D96236#2705715 <https://reviews.llvm.org/D96236#2705715>, @dblaikie wrote:

> 

(similar idea)

In D96236#2705920 <https://reviews.llvm.org/D96236#2705920>, @clayborg wrote:

> 



> Is there really not way to ask the contained "U" DWARFUnit member for the main unit?

There is no such way because one DWARFUnit can be used by multiple (in practice by two - DW_AT_language C plus C++) main units. You did ask it in D73206#1837138 <https://reviews.llvm.org/D73206#1837138>.

> Can we add a new "DWARFUnit *MU;" to the DWARFDie class?

Been there, done that. It is the option 1 from text below. It cannot be applied (it was against the DWZ patch that time), just for an idea how it did work: https://people.redhat.com/jkratoch/dwz-DWARFUnitPair-2019-10-30.patch

   class DWARFBaseDIE {
  -  DWARFUnit *m_cu;
  +  DWARFUnitPair m_cu;
     DWARFDebugInfoEntry *m_die;
   };
  +class DWARFUnitPair {
  ...
  +  DWARFUnit *m_cu;
  +  // For non-DWZ setups it is either equal to 'm_cu' or nullptr if 'm_cu' is a DWARFTypeUnit.
  +  DWARFCompileUnit *m_main_cu;
  +};

It was discussed in D73206 <https://reviews.llvm.org/D73206> where was originally a different initial comment:

-------------------------------------------------------------------------------------------------------------------------------------

[...] proposing multiple possibilities how to satisfy DWZ's need for `DWARFDIE::m_main_cu`, could you choose one?

1. `DWARFDIE` 16B->24B does not matter as `DWARFDIE` is not stored anywhere - maybe one could use this patch in such case.
2. PointerUnion for DWARFDIE::m_cu so that DWARFDIE remains 16B without DWZ.

  class DWARFBaseDIE {
    llvm::PointerUnion<DWARFUnit *, DWARFUnitPair *> m_cu;
    DWARFDebugInfoEntry *m_die;
  };
  class DWARFUnitPair {`
    DWARFUnit *m_cu;
    DWARFCompileUnit *m_main_cu;
  };

3. (removed as I do not find nowadays enough to store just `DW_LANG_*` instead of the main unit pointer.)

-------------------------------------------------------------------------------------------------------------------------------------

The option 2 is fine as the double dereference performance impact (if any) affects only LLDB run on DWZ systems. And still LLDB with this little performance hit is much faster than the only existing debugger GDB for DWZ systems.

@labath did disagree in D73206#1836018 <https://reviews.llvm.org/D73206#1836018> regarding the merge with LLVM DWARF. I cannot much talk about the merge as I do not know much the LLVM counterpart and so I haven't yet made any plan in my mind how to merge it.

Thanks for the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96236