This patch introduce DIMacro and DIMacroFile debug info metadata in the LLVM IR to support macros.
Note: DIBuilder API for creating macro entries and emitting macro in dwarf sections will be committed in separate patches.
Paths
| Differential D14687
Macro support in LLVM IR ClosedPublic Authored by aaboud on Nov 15 2015, 5:39 AM.
Details Summary This patch introduce DIMacro and DIMacroFile debug info metadata in the LLVM IR to support macros. Note: DIBuilder API for creating macro entries and emitting macro in dwarf sections will be committed in separate patches.
Diff Detail
Event Timelineaaboud updated this object. Comment Actions This seems to be missing any changes to LangRef. Do we really need to add a new field to DICompileUnit? Why would the compile unit need to reference all the macros? I don't understand why you need MacroNode separate from DIMacro. DIMacro seems to be the only subclass, so why split it? Also, is MacroNode meant for something other than debug info? If it's for debug info, it should have a DI-prefix. It should also inherit from DINode. Comment Actions
If I understood you correctly, you are referring to IR documentation that I forgot to update according to these changes, right?
If not in the DICompileUnit then where do you suggest the macro tree be referred from? its own named metadata?
At first I wanted to use DINode as the subclass for both DIMacro and DIMacroFile, however, DINode assumes a DW_TAG_* enumerator while the macro is using different enumerator DW_MACINFO_*. For the same reason, the MacroNode cannot inherit from DINode. Comment Actions
[AA] OK I will do that when I upload a new patch.
[AA] According to my understanding of macros, the compiler should pass them to the debug info as is. That is why they look the same all the way from the preprocessor till they are emitted in the object file. It would be great if somebody, who know better than me the macro debug info definition, can give his opinion on this point.
[AA] I thought that MDNode is the base class, why should we have another base class? Comment Actions This new patch contains the following modifications:
Note: I did not document "DIMacroNode" in the LangRef as it is not part of the LLVM IR textual representation, similar to "DINode" that is not mentioned either in the LangRef document. Comment Actions Thanks for the comments.
Comment Actions Hi Alexey, Comment Actions Sorry for the delay, was away all last week and haven't made it back Comment Actions
Me? What changes do you refer to? Comment Actions
Sorry, I got confused :)
I will then do the right order in LLVM textual IR, i.e. "macros" before "dwoId", but will keep the other order in the bitcode to be able to be backward compatible. aaboud edited edge metadata. Comment ActionsFollowed Duncan comment and moved "macros" field in DICompileUnit to be before "dwoId" field in all places (including API and textual LLVM IR) except bitcode, where "dwoId" is emitted before "macros" for backward compatibility. Comment Actions The IR changes LGTM. Please wait for an LGTM from Adrian or David or Eric Comment Actions
Thanks Duncan. Thanks, Comment Actions The only concern I've got with this is whether we shouldn't future-proof this by using the DWARF5-style DW_MACRO_.* constants internally, since the old ones aren't even defined in the DWARF5 standard document any more and thus might be harder to find documentation for. Context: In DWARF5 ( http://dwarfstd.org/ShowIssue.php?issue=110722.1 ) the DW_MACINFO_.* constants will be renamed to DW_MACRO_*. This will be fun to implement in dwarfdump because the same numeric value has different names depending on the DWARF version. Comment Actions
Thanks Adrian for the comment. Do you think that this patch might not fit DWARF5? Comment Actions
Older versions of the DWARF spec will remain available; dwarfstd.org has them back to 1.1. I'm not concerned about the constant names becoming unavailable.
Yeah... reading the data from the object file will be unambiguous (different section name) but pretty printing it might get kind of tedious.
Conveying the macro data from the frontend through the IR should work the same way for either, I'd guess. DWARF 5 will have differences in detail of emitting the data into the object file and that will probably have to be done separately anyway. Comment Actions Thanks Paul for the comment. Do you still think I need to change anything with this patch? Thanks,
aprantl edited edge metadata. Comment ActionsSince the numeric values are identical there are no binary compatibility issues and the constants in the assembler could be renamed later and auto-upgraded without too much effort. thanks! This revision is now accepted and ready to land.Dec 9 2015, 8:47 AM Closed by commit rL255245: Macro debug info support in LLVM IR (authored by aaboud). · Explain WhyDec 10 2015, 4:59 AM This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 40233 include/llvm/Bitcode/LLVMBitCodes.h
include/llvm/IR/DebugInfoMetadata.h
include/llvm/IR/Metadata.h
include/llvm/IR/Metadata.def
include/llvm/Support/Dwarf.h
lib/AsmParser/LLLexer.cpp
lib/AsmParser/LLParser.cpp
lib/AsmParser/LLToken.h
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
lib/IR/Verifier.cpp
lib/Support/Dwarf.cpp
test/Assembler/debug-info.ll
test/Assembler/dicompileunit.ll
unittests/IR/MetadataTest.cpp
|
I tried that approach at first. However, it will not work, as it will break the backward compatibility.
There are some LIT tests that have 'dwoId' as record number 14, if I change the order these bitcode files will fail to load using the new bitcode reader.
Do you think that I should not care about backward compatibility and change the order back such that 'macros' appear before 'dwoId'?