[PATCH] D55281: debuginfo: Use symbol difference for CU length to simplify assembly reading/editing

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D55281#1333472 <https://reviews.llvm.org/D55281#1333472>, @dblaikie wrote:

> In D55281#1333470 <https://reviews.llvm.org/D55281#1333470>, @ABataev wrote:
>
> > In D55281#1333464 <https://reviews.llvm.org/D55281#1333464>, @dblaikie wrote:
> >
> > > In D55281#1333451 <https://reviews.llvm.org/D55281#1333451>, @ABataev wrote:
> > >
> > > > In D55281#1333445 <https://reviews.llvm.org/D55281#1333445>, @dblaikie wrote:
> > > >
> > > > > Hey Alexey - thanks for taking a look!
> > > > >
> > > > > In D55281#1333365 <https://reviews.llvm.org/D55281#1333365>, @ABataev wrote:
> > > > >
> > > > > > NVPTX does not support labels in the debug info sections. All these new labels must be created/emitted only if `DwarfDebug::useSectionsAsReferences()` is `false`.
> > > > >
> > > > >
> > > > > Is that the right name for the flag? If you reckon so - though I'm not sure it means a lot to me, personally. (
> > > > >
> > > > > Also, a few places in lib/CodeGen/AsmPrinter/* use isNVPTX directly, not UseSectionsAsReferences - are they correct? (eg: UseRangeSection, UseInlineStrings, UseLocSection, some test for physical registers in updateSubprogramScopeDIE) What's the difference between these two things?
> > > >
> > > >
> > > > `useSectionsAsReferences()` does not allow to define inner symbols in the debug sections and force using the section names itself as references in other debug info sections (if it is supported). You need to use this one. Other flags are also required for the NVPTX debug info, but they control different format features.
> > >
> > >
> > > Fair enough & I'll work on that (want to reproduce the internal failure first to then test this approach works there too)
> > >
> > > But I'd still like to understand the difference between these choices - given Google encountered an internal failure where testing NVPTX was insufficient but it looked like it was still related to LLVM's assembly being passed to ptxas - wouldn't these other conditions be incorrect in that case? (ie: assuming there's some non-NVPTX target that uses ptxas, and anything using ptxas needs to disable all these features (ranges, inline strings, etc, etc) - I would've thought these cases testing NVPTX directly would be bugs? Or are there different/varied reasons these things are disabled in some situations but not others?)
> >
> >
> > I need to make those tests more strict. Currently, they are using `CHECK` in all places, though we need `CHECK-NEXT` to catch the situation you ran into. It seems to me, you emitted the label somewhere in the debug info sections and the checks for NVPTX debug info missed this label.
>
>
> Sure - sorry, I'm still trying to understand this better.
>
> Targeting NVPTX itself enables the "UseSectionsAsReferences" as well as disabling UseRangeSection, UseInlineStrings, UseLocSection, etc...
>
> I made made the change only on !NVPTX, but that seemed to be insufficient - some internal test failed anyway (so it was !NVPTX and still using ptxas, I guess?)
>
> What I'm trying to understand is, if such a thing (ptxas but not NVPTX) - how does it function? Since it would not do the other NVPTX things like disabling UseRangeSection, UseInlineStrings, and UesLocSection.
>
> Does that make sense as a question? Is there an answer?


Not all of your changes are guarded by `!NVPTX`. E.g. `Asm->OutStreamer->EmitLabel(BeginLabel);` is not, but it emits the label inside of the debug info section. ptxas does not support labels at all! It uses oversimplified DWARF2, where the references can be emitted only as `section_name +- offset`.
All new `EmitLabel()`s must be guarded by `!UseSectionsAsReferences` condition.
Also, you can manually run the tests for the NVPTX debug info and you will definitely find all that new emitted labels in the output. You need to ensure, that your changes do not add new labels in the output when `UseSectionsAsReferences` is set to `true`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55281