[PATCH] D101421: [DebugInfo] Enable CodeView DebugInfo for basic block sections

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:2949
+  // non-primary basic block sections are not contiguous with primary basic
+  // block sectoin, don't emit relocation for non-prmiary (and non-contiguous)
+  // basic block sections.
----------------
TaoPan wrote:
> tmsriram wrote:
> > Typo, "section" and "primary".  Similar comment rnk at .  The condition below only checks whether it is not the entry block but the comment says it is all non-primary basic blocks.
> Thanks for pointing out the confuse comment! How about "The codeview line table format requires functions are contiguous, if function has BB sections, all MBBs have their own section, all sections are not contiguous, emit codeview relocation only for entry block MBB in this case."?
Please fix the typo before committing.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:2952
+  if (MI->getParent()->getParent()->hasBBSections() &&
+      !MI->getParent()->isEntryBlock())
+    return;
----------------
TaoPan wrote:
> rnk wrote:
> > Instead of just the entry block, can this be `MI->getParent()->getSectionID() == MBBSectionId::Default`? That would cover all blocks in the default section, right?
> I think MBBSectionId can not be used for distinguishing basic blocks in the default section or not. The enum values of MBBSectionId are Default = 0, Exception, Cold. Section ID of all MBBs are MBBSectionId::Default in BB Sections case.
> I tried C function like below:
> int bazb(...) {
>   ...           // Unique section for function bazb, the MBB is entry block: .text,"xr",one_only,"?bazb@@YAHH at Z"
>   if (...) {
>     ...         // BB section 1: .text,"xr",associative,"?bazb@@YAHH at Z.__part.1"
>   } else {
>     ...         // BB section 2: .text,"xr",associative,"?bazb@@YAHH at Z.__part.2"
>   }
>   ...           // BB section 3: .text,"xr",associative,"?bazb@@YAHH at Z.__part.3"
>   if (...) {
>     ...         // BB section 4: .text,"xr",associative,"?bazb@@YAHH at Z.__part.4"
>   } else {
>     ...         // BB section 5: .text,"xr",associative,"?bazb@@YAHH at Z.__part.5"
>   }
>   ...           // BB section 6: .text,"xr",associative,"?bazb@@YAHH at Z.__part.6"
> }
> I think if function has BB sections, all basic blocks have their own section, all non-EntryBlock sections are relocatable BB Sections. In other words, entry block is unique block in the default section.
I see. I guess that's me being confused between intra-function hot cold splitting and BB sections. The condition is correct, then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101421