[PATCH] D69839: [DebugInfo] Fix for stopping emission of debug_macinfo section in normal case and -fno-debug-macro switch enabled case.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D69839#1734416 <https://reviews.llvm.org/D69839#1734416>, @SouraVX wrote:

> In D69839#1734120 <https://reviews.llvm.org/D69839#1734120>, @dblaikie wrote:
>
> > I think the right fix for this would be to move the section change inside the if inside the loop (only changing section when we're about to emit macros - it's cheap to change the section to the section you're already in, so there's not much to be gained by checking early/separately)
>
>
> Thanks for reviewing this.
>  The Intention here is stop the emission{even switching of the section}, if the CUNode macro list is empty, just like first and foremost check as we prepare / enter for emission CUMap.empty().


Yep, I'm with you there - I /think/ all these cases can be simplified, though, by taking a slightly different approach than the one already started/that your patch is continuing (& also fixing a more significant bug in the emission of macro lists)

> Although that's a loop{CUNode check for macro list} we can't avoid. 
>  BTW is their I'm missing something, I mean in your approach. If we do this check{CUNode macro list} before / after switching section ?? does this affect{unintentionally  leave some void} anything ?

Here's what I'm suggesting:

  /// Emit macros into a debug macinfo section.
  void DwarfDebug::emitDebugMacinfo() {
    for (const auto &P : CUMap) {
      auto &TheCU = *P.second;
      auto *SkCU = TheCU.getSkeleton();
      DwarfCompileUnit &U = SkCU ? *SkCU : TheCU;
      auto *CUNode = cast<DICompileUnit>(P.first);
      DIMacroNodeArray Macros = CUNode->getMacros();
      if (Macros.empty())
        continue;
      Asm->OutStreamer->SwitchSection(
        Asm->getObjFileLowering().getDwarfMacinfoSection());
      Asm->OutStreamer->EmitLabel(U.getMacroLabelBegin());
      handleMacroNodes(Macros, U);
      Asm->OutStreamer->AddComment("End Of Macro List Mark");
      Asm->emitInt8(0);
    }
  }

Though this should be split into at least two patches - one to fix the "End Of Macro List Marker" to be correct (by moving it into the loop/next to the "handleMacroNodes", so it ends each list separately - including testing this correction)
and then another patch (or a few) to remove the redundant checks and move the section switching into the loop so it's only done when needed.

(it's possible I've oversimplified the code above - the original code has a "if debug directives only" bail out early in the loop - but I /think/ that should naturally bail out at the "macros empty" case and not be a problem, but I could be wrong)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69839