[PATCH] Add a DWOId field to DICompileUnit (so DWARF skeleton CUs can be expression in IR).

[David Blaikie]

On Tue, May 5, 2015 at 8:35 AM, Adrian Prantl <aprantl at apple.com> wrote:

> David, I’m not entirely sure if you are ok with the naming of the field as
> is.. is this good to commit from your side?
>

It's a bit generic/misleading given that for the common case of plain
Fission the DWO ID is generated based on the DWARF in the backend, so it
doesn't make sense to carry it as part of the metadata because it cannot be
known then.

What's the particular part of the module debug info this will be used for?
Your comment says it's specifically for the skeleton side (so we'll take
the module ID in the frontend and emit a skeleton CU into the metadata)?
What about the concrete side for the module's debug info? (it'll need the
same dwo ID)

While I'm all for incremental development, a little context of the path
you've got in mind may be helpful.

- David


>
> -- adrian
>
> > On May 4, 2015, at 9:04 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >
> >
> >> On 2015 May 4, at 20:17, Adrian Prantl <aprantl at apple.com> wrote:
> >>
> >> Now with autoupgrade testcase.
> >>
> >>
> >> http://reviews.llvm.org/D9488
> >>
> >> Files:
> >> include/llvm/IR/DIBuilder.h
> >> include/llvm/IR/DebugInfoMetadata.h
> >> lib/AsmParser/LLParser.cpp
> >> lib/Bitcode/Reader/BitcodeReader.cpp
> >> lib/Bitcode/Writer/BitcodeWriter.cpp
> >> lib/IR/AsmWriter.cpp
> >> lib/IR/DIBuilder.cpp
> >> lib/IR/DebugInfoMetadata.cpp
> >> lib/IR/LLVMContextImpl.h
> >> test/Assembler/mdcompileunit.ll
> >> test/Bitcode/DICompileUnit-upgrade.test
> >> test/Bitcode/Inputs/DICompileUnit-no-DWOId.bc
> >> unittests/IR/MetadataTest.cpp
> >>
> >> EMAIL PREFERENCES
> >> http://reviews.llvm.org/settings/panel/emailpreferences/
> >> <D9488.24927.patch>
> >
> > LGTM with some changes to the autoupgrade test.
> >
> >> Index: test/Bitcode/DICompileUnit-upgrade.test
> >> ===================================================================
> >> --- /dev/null
> >> +++ test/Bitcode/DICompileUnit-upgrade.test
> >
> > Can you be more descriptive?  Perhaps dicompileunit-no-dwoid.ll.
> > (Feel free to use the camel-case filename; I personally avoid
> > capitals but clearly there's no harm.)
> >
> >> @@ -0,0 +1,6 @@
> >> +RUN: llvm-dis %p/Inputs/DICompileUnit-no-DWOId.bc -o - | FileCheck %s
> >
> > %p was undocumented last I checked.  I recommend the documented %S
> > when this sort of functionality is necessary.
> >
> > But I wouldn't even use the `Inputs` folder here.  The usual bitcode
> > upgrade tests say:
> >
> >    ; RUN: llvm-dis < %s.bc | FileCheck %s
> >    ; RUN: verify-uselistorder < %s.bc
> >
> > and just drop a .ll.bc file next to a .ll test in `test/Bitcode`.
> >
> > Moreover, the test file should be the `.ll` file that was used to
> > generate the bitcode.  Please add a comment that says what revision of
> > LLVM was used to generate the bitcode, something like:
> >
> >    ; Bitcode generated from llvm-as @ r123456.
> >
> >> +The input uses the older form without a dwoId field.
> >> +This should default to 0,
> >> +which is not displayed at all in the textual representation.
> >> +CHECK: !DICompileUnit
> >> +CHECK-NOT: dwoId:
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150505/f1af3c0f/attachment.html>