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

[David Blaikie]

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

>
> On May 5, 2015, at 9:42 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> 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)
>
>
> I casually mentioned that in one of the emails in the other thread; the
> idea is that when emitting a clang module the backend computes the regular
> DWARF DWOId for it. When emitting an external type reference, the frontend
> is using libDebugInfo to extract the DWOId from the module and emits a
> skeleton CU with pcm filename and DWOId.
>

That seems a bit heavy... I thought we discussed with Richard Smith that
the module has an actual identifier we could use here?

> While I'm all for incremental development, a little context of the path
> you've got in mind may be helpful.
>
>
> Right, the examples in cfe-dev were always DWARF and never showed the IR:
> A skeleton CU emitted by the frontend would be represented as a
>   !0 = DICompileUnit(file: MDFile(“/path/to”, “module.pcm”), dwoId:
> ABCD1234)
> and is emitted as a compile_unit with an AT_dwo_name and an AT_dwo_id by
> the backend.
>

OK.


>
> -- adrian
>
>
> - 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/310f7f7c/attachment.html>