[PATCH] D78851: Debug Info Support for Basic Block Sections

[David Blaikie via Phabricator via llvm-commits]

dblaikie added subscribers: JDevlieghere, probinson.
dblaikie added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:595-606
+    do {
+      if (MBB->sameSection(EndMBB) || MBB->isEndSection()) {
+        auto MBBSectionRange = Asm->MBBSectionRanges[MBB->getSectionID()];
+        List.push_back(
+            {MBB->sameSection(BeginMBB) ? BeginLabel
+                                        : MBBSectionRange.BeginLabel,
+             MBB->sameSection(EndMBB) ? EndLabel : MBBSectionRange.EndLabel});
----------------
Hmm, the existence of this code makes me think that this means LLVM debug info emission depends on block order. If some optimization did interesting things to block order it could break/change the semantics of the DWARF emitted.

That's perhaps a big enough bug that it's out of scope to deal with here (@aprantl @JDevlieghere @probinson - just FYI to keep in mind), but hopefully a general solution to this problem would remove the need for this code here. (& then no need for MBBSectionRanges to be a map, probably).

Maybe include a FIXME above this code to explain that?


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp:2122-2123
+    if (&MBB == &MF->front() || MBB.isBeginSection())
+      TheCU.addRange({Asm->MBBSectionRanges[MBB.getSectionID()].BeginLabel,
+                      Asm->MBBSectionRanges[MBB.getSectionID()].EndLabel});
+    if (!MF->hasBBSections())
----------------
Could we iterate MBBSectionRanges (it looks like it's built per-function?) instead of iterating BBs and looking up MBBSectionRanges? Looks like /maybe/ this code could be:

```
for (const auto &R : Asm->MBBSectionRanges)
  TheCU.addRange(R.second.BeginLabel, R.second.EndLabel);
```

(similarly for the code in updateSubprogramScopeDIE)


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

https://reviews.llvm.org/D78851