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

Adrian Prantl aprantl at apple.com
Tue May 5 08:35:35 PDT 2015


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?

-- 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:
> 





More information about the llvm-commits mailing list