[PATCH] D70350: [DWARF] Allow cross-CU references of subprogram definitions

[Vedant Kumar via Phabricator via llvm-commits]

vsk marked an inline comment as done.
vsk added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp:191
 bool DwarfUnit::isShareableAcrossCUs(const DINode *D) const {
   // When the MDNode can be part of the type system, the DIE can be shared
   // across CUs.
----------------
vsk wrote:
> aprantl wrote:
> > Technically DISubprogram that are function definitions aren't part of the type system (as opposed to function declarations). Maybe just the comment needs to be reworded, but I wonder if this could have some unintended consequences when `llvm-link`ing two llvm::Modules that both define different functions with the same name.
> > 
> > What decision is `isShareableAcrossCUs()` used for?
> `isShareableAcrossCUs` is used to figure out whether to whether a DIE can be retrieved from the DwarfFile backing a DwarfUnit, it looks like.
> 
> If there were two modules which both defined functions with the same name, I'd expect the definitions to either be merged, or for at least one to be TU-local. If we were to treat all definitions as shareable, we might expose the TU-local one (not sure what/if any harm that would cause -- maybe messed up call site tags? -- but we should probably just sidestep the issue).
My comment here is wrong, we actually do want to share all definitions (explanation + test case provided in the last patch update).


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

https://reviews.llvm.org/D70350