[PATCH] D144007: [DwarfDebug] Move emission of globals from beginModule() to endModule() (6/7)

Mon Mar 13 11:04:49 PDT 2023

dblaikie added a comment.

In D144007#4188854 <https://reviews.llvm.org/D144007#4188854>, @krisb wrote:

> In D144007#4185548 <https://reviews.llvm.org/D144007#4185548>, @dblaikie wrote:
>
>> I guess this one's a bit clearer than the type one about why changing the order is important for getting local entities correctly scoped - local static is a global variable for the debug info and IR, so we need to emit the scopes before we'll know where to put the globals, etc.
>
> The motivation for this patch is mostly the same as for types:
>
> - unify/simplify emission of local and non-local entities getting everything done in one place (in our case `DwarfDebug::endModule()`). Keeping 'non-local' globals emission in `DwarfDebug::beginModule()` (while 'local' globals should be emitted not earlier than `DwarfDebug::endModule()`) would make the implementation much more complicated as both 'local' and 'global' globals emission depends on GVMap.
> - make sure IR/debug metadata would not be changed after DWARF entities get created.

Fair enough - some tension between independent changes being independent-ish (so they can be reverted/root caused/etc separately if needed) and keeping things consistent. I'm not insisting on any reordering/separation here if you feel this is the best way to go.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144007