[PATCH] D114705: [DwarfDebug] Move emission of global vars, types and imports to endModule()

[Kristina Bessonova via Phabricator via llvm-commits]

krisb added a comment.

@dblaikie

> Could you point to/provide inline comments on any tests that demonstrate this difference?
> Are all the other tests just shifting things around/not changing the content?

Actually, all the changes in tests in this patch just reflect the changed order of debug entities, non of them reproduced the issue (1).

Perhaps I stated the motivation in the wrong order. The main goal of the patch is to make reviewing D113741 <https://reviews.llvm.org/D113741> easier, and all changes in the tests just reflect the new order of debug info; thus the tests from this PR could be removed from D113741 <https://reviews.llvm.org/D113741>.

Concerning (1) from the description is not about an actual observable bug, but rather about a design flaw that could cause a bug in the future. To implement D113741 <https://reviews.llvm.org/D113741> we could in theory collect debug info data in `DwarfDebug::beginModule`, but in this case, no one can guarantee that the collected info will be valid till `endFunctionImpl()` or `endModule()`. NVPTX indeed changes globals in a module, but it's hard to pinpoint a concrete test case that would show the problem, and it is not covered by the current test suite.
The patch eliminates (module) passes between stages of debug info emission, thus in my opinion, the design becomes more reliable. It's much better to guarantee no changes in IR rather than argue that "reasonable" changes in IR could not affect DI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114705