[llvm] r266549 - IR: Use an explicit map for debug info type uniquing
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Sat Apr 16 20:58:24 PDT 2016
Author: dexonsmith
Date: Sat Apr 16 22:58:21 2016
New Revision: 266549
URL: http://llvm.org/viewvc/llvm-project?rev=266549&view=rev
Log:
IR: Use an explicit map for debug info type uniquing
Rather than relying on the structural equivalence of DICompositeType to
merge type definitions, use an explicit map on the LLVMContext that
LLParser and BitcodeReader consult when constructing new nodes.
Each non-forward-declaration DICompositeType with a non-empty
'identifier:' field is stored/loaded from the type map, and the first
definiton will "win".
This map is opt-in: clients that expect ODR types from different modules
to be merged must call LLVMContext::ensureDITypeMap.
- Clients that just happen to load more than one Module in the same
LLVMContext won't magically merge types.
- Clients (like LTO) that want to continue to merge types based on ODR
identifiers should opt-in immediately.
I have updated LTOCodeGenerator.cpp, the two "linking" spots in
gold-plugin.cpp, and llvm-link (unless -disable-debug-info-type-map) to
set this.
With this in place, it will be straightforward to remove the DITypeRef
concept (i.e., referencing types by their 'identifier:' string rather
than pointing at them directly).
Added:
llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll
llvm/trunk/test/Linker/dicompositetype-unique.ll
llvm/trunk/unittests/IR/LLVMContextTest.cpp
Modified:
llvm/trunk/docs/LangRef.rst
llvm/trunk/include/llvm/IR/LLVMContext.h
llvm/trunk/lib/AsmParser/LLParser.cpp
llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/trunk/lib/IR/DebugInfoMetadata.cpp
llvm/trunk/lib/IR/LLVMContext.cpp
llvm/trunk/lib/IR/LLVMContextImpl.h
llvm/trunk/lib/LTO/LTOCodeGenerator.cpp
llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
llvm/trunk/tools/gold/gold-plugin.cpp
llvm/trunk/tools/llvm-link/llvm-link.cpp
llvm/trunk/unittests/IR/CMakeLists.txt
Modified: llvm/trunk/docs/LangRef.rst
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/LangRef.rst?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/docs/LangRef.rst (original)
+++ llvm/trunk/docs/LangRef.rst Sat Apr 16 22:58:21 2016
@@ -4006,6 +4006,11 @@ identifier used for type merging between
can refer to composite types indirectly via a :ref:`metadata string
<metadata-string>` that matches their identifier.
+For a given ``identifier:``, there should only be a single composite type that
+does not have ``flags: DIFlagFwdDecl`` set. LLVM tools that link modules
+together will unique such definitions at parse time via the ``identifier:``
+field, even if the nodes are ``distinct``.
+
.. code-block:: llvm
!0 = !DIEnumerator(name: "SixKind", value: 7)
Modified: llvm/trunk/include/llvm/IR/LLVMContext.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/LLVMContext.h?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/LLVMContext.h (original)
+++ llvm/trunk/include/llvm/IR/LLVMContext.h Sat Apr 16 22:58:21 2016
@@ -25,6 +25,8 @@ class StringRef;
class Twine;
class Instruction;
class Module;
+class MDString;
+class DIType;
class SMDiagnostic;
class DiagnosticInfo;
template <typename T> class SmallVectorImpl;
@@ -113,6 +115,23 @@ public:
/// especially in release mode.
void setDiscardValueNames(bool Discard);
+ /// Whether there is a string map for uniquing debug info types with
+ /// identifiers across the context. Off by default.
+ bool hasDITypeMap() const;
+ void ensureDITypeMap();
+ void destroyDITypeMap();
+
+ /// Get or insert the DIType mapped to the given string.
+ ///
+ /// Returns the address of the current \a DIType pointer mapped to \c S,
+ /// inserting a mapping to \c nullptr if \c S was not previously mapped.
+ /// This method has no effect (and returns \c nullptr instead of a valid
+ /// address) if \a hasDITypeMap() is \c false.
+ ///
+ /// \post If \a hasDITypeMap(), \c S will have a (possibly null) mapping.
+ /// \note The returned address is only valid until the next call.
+ DIType **getOrInsertDITypeMapping(const MDString &S);
+
typedef void (*InlineAsmDiagHandlerTy)(const SMDiagnostic&, void *Context,
unsigned LocCookie);
Modified: llvm/trunk/lib/AsmParser/LLParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/AsmParser/LLParser.cpp?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/lib/AsmParser/LLParser.cpp (original)
+++ llvm/trunk/lib/AsmParser/LLParser.cpp Sat Apr 16 22:58:21 2016
@@ -3839,11 +3839,25 @@ bool LLParser::ParseDICompositeType(MDNo
PARSE_MD_FIELDS();
#undef VISIT_MD_FIELDS
+ // If this isn't a forward declaration and it has a UUID, check for it in the
+ // type map in the context.
+ DIType **MappedT = nullptr;
+ if (!(flags.Val & DINode::FlagFwdDecl) && identifier.Val &&
+ (MappedT = Context.getOrInsertDITypeMapping(*identifier.Val)) &&
+ *MappedT) {
+ Result = *MappedT;
+ return false;
+ }
+
+ // Create a new node, and save it in the context if it belongs in the type
+ // map.
Result = GET_OR_DISTINCT(
DICompositeType,
(Context, tag.Val, name.Val, file.Val, line.Val, scope.Val, baseType.Val,
size.Val, align.Val, offset.Val, flags.Val, elements.Val,
runtimeLang.Val, vtableHolder.Val, templateParams.Val, identifier.Val));
+ if (MappedT)
+ *MappedT = cast<DIType>(Result);
return false;
}
Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Sat Apr 16 22:58:21 2016
@@ -2188,16 +2188,29 @@ std::error_code BitcodeReader::parseMeta
if (Record.size() != 16)
return error("Invalid record");
- MetadataList.assignValue(
- GET_OR_DISTINCT(DICompositeType, Record[0],
- (Context, Record[1], getMDString(Record[2]),
- getMDOrNull(Record[3]), Record[4],
- getMDOrNull(Record[5]), getMDOrNull(Record[6]),
- Record[7], Record[8], Record[9], Record[10],
- getMDOrNull(Record[11]), Record[12],
- getMDOrNull(Record[13]), getMDOrNull(Record[14]),
- getMDString(Record[15]))),
- NextMetadataNo++);
+ // If we have a UUID and this is not a forward declaration, lookup the
+ // mapping.
+ unsigned Flags = Record[10];
+ auto *Identifier = getMDString(Record[15]);
+ DIType **MappedT = nullptr;
+ if (!(Flags & DINode::FlagFwdDecl) && Identifier)
+ MappedT = Context.getOrInsertDITypeMapping(*Identifier);
+
+ // Use the mapped type node, or create a new one if necessary.
+ DIType *CT = MappedT ? *MappedT : nullptr;
+ if (!CT) {
+ CT = GET_OR_DISTINCT(
+ DICompositeType, Record[0],
+ (Context, Record[1], getMDString(Record[2]), getMDOrNull(Record[3]),
+ Record[4], getMDOrNull(Record[5]), getMDOrNull(Record[6]),
+ Record[7], Record[8], Record[9], Flags, getMDOrNull(Record[11]),
+ Record[12], getMDOrNull(Record[13]), getMDOrNull(Record[14]),
+ Identifier));
+ if (MappedT)
+ *MappedT = CT;
+ }
+
+ MetadataList.assignValue(CT, NextMetadataNo++);
break;
}
case bitc::METADATA_SUBROUTINE_TYPE: {
Modified: llvm/trunk/lib/IR/DebugInfoMetadata.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/DebugInfoMetadata.cpp?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/lib/IR/DebugInfoMetadata.cpp (original)
+++ llvm/trunk/lib/IR/DebugInfoMetadata.cpp Sat Apr 16 22:58:21 2016
@@ -254,6 +254,7 @@ DICompositeType *DICompositeType::getImp
Metadata *TemplateParams, MDString *Identifier, StorageType Storage,
bool ShouldCreate) {
assert(isCanonical(Name) && "Expected canonical MDString");
+
DEFINE_GETIMPL_LOOKUP(
DICompositeType, (Tag, Name, File, Line, Scope, BaseType, SizeInBits,
AlignInBits, OffsetInBits, Flags, Elements, RuntimeLang,
Modified: llvm/trunk/lib/IR/LLVMContext.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContext.cpp?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/lib/IR/LLVMContext.cpp (original)
+++ llvm/trunk/lib/IR/LLVMContext.cpp Sat Apr 16 22:58:21 2016
@@ -311,6 +311,23 @@ bool LLVMContext::shouldDiscardValueName
return pImpl->DiscardValueNames;
}
+bool LLVMContext::hasDITypeMap() const { return !!pImpl->DITypeMap; }
+
+void LLVMContext::ensureDITypeMap() {
+ if (pImpl->DITypeMap)
+ return;
+
+ pImpl->DITypeMap = llvm::make_unique<DenseMap<const MDString *, DIType *>>();
+}
+
+void LLVMContext::destroyDITypeMap() { pImpl->DITypeMap.reset(); }
+
+DIType **LLVMContext::getOrInsertDITypeMapping(const MDString &S) {
+ if (!hasDITypeMap())
+ return nullptr;
+ return &(*pImpl->DITypeMap)[&S];
+}
+
void LLVMContext::setDiscardValueNames(bool Discard) {
pImpl->DiscardValueNames = Discard;
}
Modified: llvm/trunk/lib/IR/LLVMContextImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/LLVMContextImpl.h?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/lib/IR/LLVMContextImpl.h (original)
+++ llvm/trunk/lib/IR/LLVMContextImpl.h Sat Apr 16 22:58:21 2016
@@ -1022,6 +1022,9 @@ public:
DenseSet<CLASS *, CLASS##Info> CLASS##s;
#include "llvm/IR/Metadata.def"
+ // Optional map for looking up composite types by identifier.
+ std::unique_ptr<DenseMap<const MDString *, DIType *>> DITypeMap;
+
// MDNodes may be uniqued or not uniqued. When they're not uniqued, they
// aren't in the MDNodeSet, but they're still shared between objects, so no
// one object can destroy them. This set allows us to at least destroy them
Modified: llvm/trunk/lib/LTO/LTOCodeGenerator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOCodeGenerator.cpp?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/LTOCodeGenerator.cpp (original)
+++ llvm/trunk/lib/LTO/LTOCodeGenerator.cpp Sat Apr 16 22:58:21 2016
@@ -84,6 +84,7 @@ LTOCodeGenerator::LTOCodeGenerator(LLVMC
: Context(Context), MergedModule(new Module("ld-temp.o", Context)),
TheLinker(new Linker(*MergedModule)) {
Context.setDiscardValueNames(LTODiscardValueNames);
+ Context.ensureDITypeMap();
initializeLTOPasses();
}
Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Sat Apr 16 22:58:21 2016
@@ -15,6 +15,7 @@
#include "llvm/Transforms/Utils/ValueMapper.h"
#include "llvm/IR/CallSite.h"
#include "llvm/IR/Constants.h"
+#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/GlobalAlias.h"
#include "llvm/IR/GlobalVariable.h"
Added: llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll?rev=266549&view=auto
==============================================================================
--- llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll (added)
+++ llvm/trunk/test/Linker/Inputs/dicompositetype-unique.ll Sat Apr 16 22:58:21 2016
@@ -0,0 +1,4 @@
+!named = !{!0, !1}
+
+!0 = !DIFile(filename: "abc", directory: "/path/to")
+!1 = !DICompositeType(tag: DW_TAG_class_type, name: "T2", identifier: "T", file: !0)
Added: llvm/trunk/test/Linker/dicompositetype-unique.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/dicompositetype-unique.ll?rev=266549&view=auto
==============================================================================
--- llvm/trunk/test/Linker/dicompositetype-unique.ll (added)
+++ llvm/trunk/test/Linker/dicompositetype-unique.ll Sat Apr 16 22:58:21 2016
@@ -0,0 +1,42 @@
+; RUN: llvm-link -S -o - %s %S/Inputs/dicompositetype-unique.ll \
+; RUN: | FileCheck %s
+; RUN: llvm-link -S -o - %S/Inputs/dicompositetype-unique.ll %s \
+; RUN: | FileCheck %s -check-prefix REVERSE
+; RUN: llvm-link -disable-debug-info-type-map -S -o - %s %S/Inputs/dicompositetype-unique.ll \
+; RUN: | FileCheck %s -check-prefix NOMAP
+
+; Check that the bitcode reader handles this too.
+; RUN: llvm-as -o %t1.bc <%s
+; RUN: llvm-as -o %t2.bc <%S/Inputs/dicompositetype-unique.ll
+; RUN: llvm-link -S -o - %t1.bc %t2.bc | FileCheck %s
+; RUN: llvm-link -S -o - %t2.bc %t1.bc | FileCheck %s -check-prefix REVERSE
+; RUN: llvm-link -disable-debug-info-type-map -S -o - %t1.bc %t2.bc \
+; RUN: | FileCheck %s -check-prefix NOMAP
+
+; Check that the type map will unique two DICompositeTypes.
+
+; CHECK: !named = !{!0, !1, !0, !1}
+; REVERSE: !named = !{!0, !1, !0, !1}
+; NOMAP: !named = !{!0, !1, !0, !2}
+!named = !{!0, !1}
+
+; Check both directions.
+; CHECK: !1 = !DICompositeType(
+; CHECK-SAME: name: "T1"
+; CHECK-SAME: identifier: "T"
+; CHECK-NOT: identifier: "T"
+; REVERSE: !1 = !DICompositeType(
+; REVERSE-SAME: name: "T2"
+; REVERSE-SAME: identifier: "T"
+; REVERSE-NOT: identifier: "T"
+
+; These types are different, so we should get both copies when there is no map.
+; NOMAP: !1 = !DICompositeType(
+; NOMAP-SAME: name: "T1"
+; NOMAP-SAME: identifier: "T"
+; NOMAP: !2 = !DICompositeType(
+; NOMAP-SAME: name: "T2"
+; NOMAP-SAME: identifier: "T"
+; NOMAP-NOT: identifier: "T"
+!0 = !DIFile(filename: "abc", directory: "/path/to")
+!1 = !DICompositeType(tag: DW_TAG_class_type, name: "T1", identifier: "T", file: !0)
Modified: llvm/trunk/tools/gold/gold-plugin.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/gold/gold-plugin.cpp?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/tools/gold/gold-plugin.cpp (original)
+++ llvm/trunk/tools/gold/gold-plugin.cpp Sat Apr 16 22:58:21 2016
@@ -1110,6 +1110,7 @@ static void thinLTOBackendTask(claimed_f
raw_fd_ostream *OS, unsigned TaskID) {
// Need to use a separate context for each task
LLVMContext Context;
+ Context.ensureDITypeMap(); // Merge debug info types.
Context.setDiagnosticHandler(diagnosticHandlerForContext, nullptr, true);
std::unique_ptr<llvm::Module> NewModule(new llvm::Module(File.name, Context));
@@ -1231,6 +1232,7 @@ static ld_plugin_status allSymbolsReadHo
}
LLVMContext Context;
+ Context.ensureDITypeMap(); // Merge debug info types.
Context.setDiagnosticHandler(diagnosticHandlerForContext, nullptr, true);
std::unique_ptr<Module> Combined(new Module("ld-temp.o", Context));
Modified: llvm/trunk/tools/llvm-link/llvm-link.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-link/llvm-link.cpp?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-link/llvm-link.cpp (original)
+++ llvm/trunk/tools/llvm-link/llvm-link.cpp Sat Apr 16 22:58:21 2016
@@ -71,6 +71,10 @@ static cl::opt<bool>
Internalize("internalize", cl::desc("Internalize linked symbols"));
static cl::opt<bool>
+ DisableDITypeMap("disable-debug-info-type-map",
+ cl::desc("Don't use a uniquing type map for debug info"));
+
+static cl::opt<bool>
OnlyNeeded("only-needed", cl::desc("Link only needed symbols"));
static cl::opt<bool>
@@ -337,6 +341,9 @@ int main(int argc, char **argv) {
llvm_shutdown_obj Y; // Call llvm_shutdown() on exit.
cl::ParseCommandLineOptions(argc, argv, "llvm linker\n");
+ if (!DisableDITypeMap)
+ Context.ensureDITypeMap();
+
auto Composite = make_unique<Module>("llvm-link", Context);
Linker L(*Composite);
Modified: llvm/trunk/unittests/IR/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/CMakeLists.txt?rev=266549&r1=266548&r2=266549&view=diff
==============================================================================
--- llvm/trunk/unittests/IR/CMakeLists.txt (original)
+++ llvm/trunk/unittests/IR/CMakeLists.txt Sat Apr 16 22:58:21 2016
@@ -16,6 +16,7 @@ set(IRSources
IRBuilderTest.cpp
InstructionsTest.cpp
IntrinsicsTest.cpp
+ LLVMContextTest.cpp
LegacyPassManagerTest.cpp
MDBuilderTest.cpp
MetadataTest.cpp
Added: llvm/trunk/unittests/IR/LLVMContextTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/LLVMContextTest.cpp?rev=266549&view=auto
==============================================================================
--- llvm/trunk/unittests/IR/LLVMContextTest.cpp (added)
+++ llvm/trunk/unittests/IR/LLVMContextTest.cpp Sat Apr 16 22:58:21 2016
@@ -0,0 +1,57 @@
+//===- LLVMContextTest.cpp - LLVMContext unit tests -----------------------===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/DebugInfoMetadata.h"
+#include "gtest/gtest.h"
+using namespace llvm;
+
+namespace {
+
+TEST(LLVMContextTest, ensureDITypeMap) {
+ LLVMContext Context;
+ EXPECT_FALSE(Context.hasDITypeMap());
+ Context.ensureDITypeMap();
+ EXPECT_TRUE(Context.hasDITypeMap());
+ Context.destroyDITypeMap();
+ EXPECT_FALSE(Context.hasDITypeMap());
+}
+
+TEST(LLVMContextTest, getOrInsertDITypeMapping) {
+ LLVMContext Context;
+ const MDString &S = *MDString::get(Context, "string");
+
+ // Without a type map, this should return null.
+ EXPECT_FALSE(Context.getOrInsertDITypeMapping(S));
+
+ // Get the mapping.
+ Context.ensureDITypeMap();
+ DIType **Mapping = Context.getOrInsertDITypeMapping(S);
+ ASSERT_TRUE(Mapping);
+
+ // Create some type and add it to the mapping.
+ auto &BT =
+ *DIBasicType::get(Context, dwarf::DW_TAG_unspecified_type, S.getString());
+ *Mapping = &BT;
+
+ // Check that we get it back.
+ Mapping = Context.getOrInsertDITypeMapping(S);
+ ASSERT_TRUE(Mapping);
+ EXPECT_EQ(&BT, *Mapping);
+
+ // Check that it's discarded with the type map.
+ Context.destroyDITypeMap();
+ EXPECT_FALSE(Context.getOrInsertDITypeMapping(S));
+
+ // And it shouldn't magically reappear...
+ Context.ensureDITypeMap();
+ EXPECT_FALSE(*Context.getOrInsertDITypeMapping(S));
+}
+
+} // end namespace
More information about the llvm-commits
mailing list