[PATCH] D30377: [DebugInfo] [DWARFv5] Support for DW_AT_calling_convention for types
Adrian Prantl via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 27 13:31:41 PST 2017
aprantl added a comment.
Thanks for working on this!
- There also needs to be a bitcode upgrade test (using a .bc file generated by current LLVM trunk)
- There also needs to be an IR llvm-as | llvm-dis | llvm-as | ... roundtrip test
- This needs to be added to the metadata unit tests.
================
Comment at: include/llvm/IR/DIBuilder.h:312
/// \param TemplateParms Template type parameters.
/// \param UniqueIdentifier A unique identifier for the class.
DICompositeType *createClassType(
----------------
Please document the new argument.
================
Comment at: include/llvm/IR/DIBuilder.h:319
+ StringRef UniqueIdentifier = "",
+ dwarf::CallingConvention ArgABI = dwarf::DW_CC_normal);
----------------
Maybe call this CallingConvention?
================
Comment at: include/llvm/IR/DIBuilder.h:331
/// \param RunTimeLang Optional parameter, Objective-C runtime version.
/// \param UniqueIdentifier A unique identifier for the struct.
DICompositeType *createStructType(
----------------
comment
================
Comment at: include/llvm/IR/DIBuilder.h:349
/// \param RunTimeLang Optional parameter, Objective-C runtime version.
/// \param UniqueIdentifier A unique identifier for the union.
DICompositeType *createUnionType(DIScope *Scope, StringRef Name,
----------------
comment
================
Comment at: lib/Bitcode/Reader/MetadataLoader.cpp:1130
case bitc::METADATA_COMPOSITE_TYPE: {
- if (Record.size() != 16)
+ if (Record.size() < 16)
return error("Invalid record");
----------------
Since there are only 2 bits necessary to encode the CC, we should just use some bits in Record[0] for this and save some space.
================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:1500
Record.push_back(VE.getMetadataOrNullID(N->getRawIdentifier()));
+ Record.push_back(N->getArgABI());
----------------
See my comment in MetadataLoader.cpp
================
Comment at: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:973
+ // Add DW_AT_calling_convention attr.
+ if (DD->getDwarfVersion() >= 5) {
+ auto ArgABI = CTy->getArgABI();
----------------
Technically true, but I think there is no harm in emitting this attribute also in DWARF 4 or earlier. We usually only do this if the new attribute cannot be safely ignored by an older consumer.
https://reviews.llvm.org/D30377
More information about the llvm-commits
mailing list