[llvm-branch-commits] [llvm] f5f7ff8 - [dsymutil][DWARFLinker][NFC] Refactor usages of UniquingStringPool.
Alexey Lapshin via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sun Jan 3 03:56:33 PST 2021
Author: Alexey Lapshin
Date: 2021-01-03T14:44:51+03:00
New Revision: f5f7ff8d0faacf4813e38081e551b9ab6cdc76ae
URL: https://github.com/llvm/llvm-project/commit/f5f7ff8d0faacf4813e38081e551b9ab6cdc76ae
DIFF: https://github.com/llvm/llvm-project/commit/f5f7ff8d0faacf4813e38081e551b9ab6cdc76ae.diff
LOG: [dsymutil][DWARFLinker][NFC] Refactor usages of UniquingStringPool.
That refactoring is helpful since it reduces data inter-dependencies.
Which is good for current implementation and even more good for
fully multi-thread implementation. The idea of the refactoring
is to delete UniquingStringPool from the global DWARFLinker level.
It is used to unique type names while ODR deduplication is done.
Thus we move UniquingStringPool into the DeclContextTree which
matched to UniquingStringPool usage scope.
golden-dsymutil/dsymutil 23787992
clang MD5: 7d9873ff94f0246b6ab1ec3e8d0f3f06
build-Release/bin/dsymutil 23921272
clang MD5: 7d9873ff94f0246b6ab1ec3e8d0f3f06
Differential Revision: https://reviews.llvm.org/D93460
Added:
Modified:
llvm/include/llvm/DWARFLinker/DWARFLinker.h
llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h
llvm/lib/DWARFLinker/DWARFLinker.cpp
llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
index edf74168d5b3..97faad6b6180 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h
@@ -467,7 +467,6 @@ class DWARFLinker {
bool registerModuleReference(DWARFDie CUDie, const DWARFUnit &Unit,
const DWARFFile &File,
OffsetsStringPool &OffsetsStringPool,
- UniquingStringPool &UniquingStringPoolStringPool,
DeclContextTree &ODRContexts,
uint64_t ModulesEndOffset, unsigned &UnitID,
bool IsLittleEndian, unsigned Indent = 0,
@@ -480,7 +479,6 @@ class DWARFLinker {
StringRef ModuleName, uint64_t DwoId,
const DWARFFile &File,
OffsetsStringPool &OffsetsStringPool,
- UniquingStringPool &UniquingStringPool,
DeclContextTree &ODRContexts, uint64_t ModulesEndOffset,
unsigned &UnitID, bool IsLittleEndian,
unsigned Indent = 0, bool Quiet = false);
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h b/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
index 740b1293f618..549a0ce4e4b7 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h
@@ -237,21 +237,6 @@ class CompileUnit {
const std::vector<AccelInfo> &getNamespaces() const { return Namespaces; }
const std::vector<AccelInfo> &getObjC() const { return ObjC; }
- /// Get the full path for file \a FileNum in the line table
- StringRef getResolvedPath(unsigned FileNum) {
- if (FileNum >= ResolvedPaths.size())
- return StringRef();
- return ResolvedPaths[FileNum];
- }
-
- /// Set the fully resolved path for the line-table's file \a FileNum
- /// to \a Path.
- void setResolvedPath(unsigned FileNum, StringRef Path) {
- if (ResolvedPaths.size() <= FileNum)
- ResolvedPaths.resize(FileNum + 1);
- ResolvedPaths[FileNum] = Path;
- }
-
MCSymbol *getLabelBegin() { return LabelBegin; }
void setLabelBegin(MCSymbol *S) { LabelBegin = S; }
@@ -310,12 +295,6 @@ class CompileUnit {
std::vector<AccelInfo> ObjC;
/// @}
- /// Cached resolved paths from the line table.
- /// Note, the StringRefs here point in to the intern (uniquing) string pool.
- /// This means that a StringRef returned here doesn't need to then be uniqued
- /// for the purposes of getting a unique address for each string.
- std::vector<StringRef> ResolvedPaths;
-
/// Is this unit subject to the ODR rule?
bool HasODR;
diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h b/llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h
index e59e15f00a7e..d2274488e85f 100644
--- a/llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h
+++ b/llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h
@@ -15,6 +15,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/CodeGen/NonRelocatableStringpool.h"
#include "llvm/DWARFLinker/DWARFLinkerCompileUnit.h"
+#include "llvm/DebugInfo/DWARF/DWARFDebugLine.h"
#include "llvm/DebugInfo/DWARF/DWARFDie.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
@@ -31,16 +32,18 @@ class CachedPathResolver {
public:
/// Resolve a path by calling realpath and cache its result. The returned
/// StringRef is interned in the given \p StringPool.
- StringRef resolve(std::string Path, NonRelocatableStringpool &StringPool) {
+ StringRef resolve(const std::string &Path,
+ NonRelocatableStringpool &StringPool) {
StringRef FileName = sys::path::filename(Path);
- SmallString<256> ParentPath = sys::path::parent_path(Path);
+ StringRef ParentPath = sys::path::parent_path(Path);
// If the ParentPath has not yet been resolved, resolve and cache it for
// future look-ups.
if (!ResolvedPaths.count(ParentPath)) {
SmallString<256> RealPath;
sys::fs::real_path(ParentPath, RealPath);
- ResolvedPaths.insert({ParentPath, StringRef(RealPath).str()});
+ ResolvedPaths.insert(
+ {ParentPath, std::string(RealPath.c_str(), RealPath.size())});
}
// Join the file name again with the resolved path.
@@ -95,7 +98,6 @@ class DeclContext {
void setDefinedInClangModule(bool Val) { DefinedInClangModule = Val; }
uint16_t getTag() const { return Tag; }
- StringRef getName() const { return Name; }
private:
friend DeclMapInfo;
@@ -129,10 +131,10 @@ class DeclContextTree {
///
/// FIXME: The invalid bit along the return value is to emulate some
/// dsymutil-classic functionality.
- PointerIntPair<DeclContext *, 1>
- getChildDeclContext(DeclContext &Context, const DWARFDie &DIE,
- CompileUnit &Unit, UniquingStringPool &StringPool,
- bool InClangModule);
+ PointerIntPair<DeclContext *, 1> getChildDeclContext(DeclContext &Context,
+ const DWARFDie &DIE,
+ CompileUnit &Unit,
+ bool InClangModule);
DeclContext &getRoot() { return Root; }
@@ -141,8 +143,19 @@ class DeclContextTree {
DeclContext Root;
DeclContext::Map Contexts;
- /// Cache resolved paths from the line table.
+ /// Cached resolved paths from the line table.
+ /// The key is <UniqueUnitID, FileIdx>.
+ using ResolvedPathsMap = DenseMap<std::pair<unsigned, unsigned>, StringRef>;
+ ResolvedPathsMap ResolvedPaths;
+
+ /// Helper that resolves and caches fragments of file paths.
CachedPathResolver PathResolver;
+
+ /// String pool keeping real path bodies.
+ NonRelocatableStringpool StringPool;
+
+ StringRef getResolvedPath(CompileUnit &CU, unsigned FileNum,
+ const DWARFDebugLine::LineTable &LineTable);
};
/// Info type for the DenseMap storing the DeclContext pointers.
diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp
index b48f6ea0e50f..bcda34066f42 100644
--- a/llvm/lib/DWARFLinker/DWARFLinker.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp
@@ -313,9 +313,8 @@ static void updateChildPruning(const DWARFDie &Die, CompileUnit &CU,
/// (i.e., forward declarations that are children of a DW_TAG_module).
static bool analyzeContextInfo(
const DWARFDie &DIE, unsigned ParentIdx, CompileUnit &CU,
- DeclContext *CurrentDeclContext, UniquingStringPool &StringPool,
- DeclContextTree &Contexts, uint64_t ModulesEndOffset,
- swiftInterfacesMap *ParseableSwiftInterfaces,
+ DeclContext *CurrentDeclContext, DeclContextTree &Contexts,
+ uint64_t ModulesEndOffset, swiftInterfacesMap *ParseableSwiftInterfaces,
std::function<void(const Twine &, const DWARFDie &)> ReportWarning,
bool InImportedModule = false) {
// LIFO work list.
@@ -366,7 +365,7 @@ static bool analyzeContextInfo(
if (CU.hasODR() || InClangModule) {
if (Current.Context) {
auto PtrInvalidPair = Contexts.getChildDeclContext(
- *Current.Context, Current.Die, CU, StringPool, InClangModule);
+ *Current.Context, Current.Die, CU, InClangModule);
Current.Context = PtrInvalidPair.getPointer();
Info.Ctxt =
PtrInvalidPair.getInt() ? nullptr : PtrInvalidPair.getPointer();
@@ -1977,11 +1976,13 @@ static std::string remapPath(StringRef Path,
return p.str().str();
}
-bool DWARFLinker::registerModuleReference(
- DWARFDie CUDie, const DWARFUnit &Unit, const DWARFFile &File,
- OffsetsStringPool &StringPool, UniquingStringPool &UniquingStringPool,
- DeclContextTree &ODRContexts, uint64_t ModulesEndOffset, unsigned &UnitID,
- bool IsLittleEndian, unsigned Indent, bool Quiet) {
+bool DWARFLinker::registerModuleReference(DWARFDie CUDie, const DWARFUnit &Unit,
+ const DWARFFile &File,
+ OffsetsStringPool &StringPool,
+ DeclContextTree &ODRContexts,
+ uint64_t ModulesEndOffset,
+ unsigned &UnitID, bool IsLittleEndian,
+ unsigned Indent, bool Quiet) {
std::string PCMfile = dwarf::toString(
CUDie.find({dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}), "");
if (PCMfile.empty())
@@ -2025,10 +2026,9 @@ bool DWARFLinker::registerModuleReference(
// shouldn't run into an infinite loop, so mark it as processed now.
ClangModules.insert({PCMfile, DwoId});
- if (Error E =
- loadClangModule(CUDie, PCMfile, Name, DwoId, File, StringPool,
- UniquingStringPool, ODRContexts, ModulesEndOffset,
- UnitID, IsLittleEndian, Indent + 2, Quiet)) {
+ if (Error E = loadClangModule(CUDie, PCMfile, Name, DwoId, File, StringPool,
+ ODRContexts, ModulesEndOffset, UnitID,
+ IsLittleEndian, Indent + 2, Quiet)) {
consumeError(std::move(E));
return false;
}
@@ -2038,9 +2038,8 @@ bool DWARFLinker::registerModuleReference(
Error DWARFLinker::loadClangModule(
DWARFDie CUDie, StringRef Filename, StringRef ModuleName, uint64_t DwoId,
const DWARFFile &File, OffsetsStringPool &StringPool,
- UniquingStringPool &UniquingStringPool, DeclContextTree &ODRContexts,
- uint64_t ModulesEndOffset, unsigned &UnitID, bool IsLittleEndian,
- unsigned Indent, bool Quiet) {
+ DeclContextTree &ODRContexts, uint64_t ModulesEndOffset, unsigned &UnitID,
+ bool IsLittleEndian, unsigned Indent, bool Quiet) {
/// Using a SmallString<0> because loadClangModule() is recursive.
SmallString<0> Path(Options.PrependPath);
if (sys::path::is_relative(Filename))
@@ -2064,9 +2063,9 @@ Error DWARFLinker::loadClangModule(
auto CUDie = CU->getUnitDIE(false);
if (!CUDie)
continue;
- if (!registerModuleReference(
- CUDie, *CU, File, StringPool, UniquingStringPool, ODRContexts,
- ModulesEndOffset, UnitID, IsLittleEndian, Indent, Quiet)) {
+ if (!registerModuleReference(CUDie, *CU, File, StringPool, ODRContexts,
+ ModulesEndOffset, UnitID, IsLittleEndian,
+ Indent, Quiet)) {
if (Unit) {
std::string Err =
(Filename +
@@ -2094,9 +2093,8 @@ Error DWARFLinker::loadClangModule(
Unit = std::make_unique<CompileUnit>(*CU, UnitID++, !Options.NoODR,
ModuleName);
Unit->setHasInterestingContent();
- analyzeContextInfo(CUDie, 0, *Unit, &ODRContexts.getRoot(),
- UniquingStringPool, ODRContexts, ModulesEndOffset,
- Options.ParseableSwiftInterfaces,
+ analyzeContextInfo(CUDie, 0, *Unit, &ODRContexts.getRoot(), ODRContexts,
+ ModulesEndOffset, Options.ParseableSwiftInterfaces,
[&](const Twine &Warning, const DWARFDie &DIE) {
reportWarning(Warning, File, &DIE);
});
@@ -2310,10 +2308,6 @@ bool DWARFLinker::link() {
// parallel loop.
unsigned NumObjects = ObjectContexts.size();
- // This Dwarf string pool which is only used for uniquing. This one should
- // never be used for offsets as its not thread-safe or predictable.
- UniquingStringPool UniquingStringPool(nullptr, true);
-
// This Dwarf string pool which is used for emission. It must be used
// serially as the order of calling getStringOffset matters for
// reproducibility.
@@ -2383,7 +2377,7 @@ bool DWARFLinker::link() {
}
if (CUDie && !LLVM_UNLIKELY(Options.Update))
registerModuleReference(CUDie, *CU, OptContext.File, OffsetsStringPool,
- UniquingStringPool, ODRContexts, 0, UnitID,
+ ODRContexts, 0, UnitID,
OptContext.File.Dwarf->isLittleEndian());
}
}
@@ -2425,8 +2419,8 @@ bool DWARFLinker::link() {
auto CUDie = CU->getUnitDIE(false);
if (!CUDie || LLVM_UNLIKELY(Options.Update) ||
!registerModuleReference(CUDie, *CU, Context.File, OffsetsStringPool,
- UniquingStringPool, ODRContexts,
- ModulesEndOffset, UnitID, Quiet)) {
+ ODRContexts, ModulesEndOffset, UnitID,
+ Quiet)) {
Context.CompileUnits.push_back(std::make_unique<CompileUnit>(
*CU, UnitID++, !Options.NoODR && !Options.Update, ""));
}
@@ -2438,9 +2432,8 @@ bool DWARFLinker::link() {
if (!CUDie)
continue;
analyzeContextInfo(CurrentUnit->getOrigUnit().getUnitDIE(), 0,
- *CurrentUnit, &ODRContexts.getRoot(),
- UniquingStringPool, ODRContexts, ModulesEndOffset,
- Options.ParseableSwiftInterfaces,
+ *CurrentUnit, &ODRContexts.getRoot(), ODRContexts,
+ ModulesEndOffset, Options.ParseableSwiftInterfaces,
[&](const Twine &Warning, const DWARFDie &DIE) {
reportWarning(Warning, Context.File, &DIE);
});
diff --git a/llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp b/llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp
index c9a5da6676b3..d9b3c4235b4d 100644
--- a/llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp
+++ b/llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp
@@ -40,9 +40,9 @@ bool DeclContext::setLastSeenDIE(CompileUnit &U, const DWARFDie &Die) {
return true;
}
-PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
- DeclContext &Context, const DWARFDie &DIE, CompileUnit &U,
- UniquingStringPool &StringPool, bool InClangModule) {
+PointerIntPair<DeclContext *, 1>
+DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE,
+ CompileUnit &U, bool InClangModule) {
unsigned Tag = DIE.getTag();
// FIXME: dsymutil-classic compat: We should bail out here if we
@@ -80,27 +80,20 @@ PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
break;
}
- const char *Name = DIE.getLinkageName();
- const char *ShortName = DIE.getShortName();
-
- if (!Name)
- Name = ShortName;
-
StringRef NameRef;
- StringRef ShortNameRef;
StringRef FileRef;
- if (Name)
- NameRef = StringPool.internString(Name);
- else if (Tag == dwarf::DW_TAG_namespace)
+ if (const char *LinkageName = DIE.getLinkageName())
+ NameRef = StringPool.internString(LinkageName);
+ else if (const char *ShortName = DIE.getShortName())
+ NameRef = StringPool.internString(ShortName);
+
+ bool IsAnonymousNamespace = NameRef.empty() && Tag == dwarf::DW_TAG_namespace;
+ if (IsAnonymousNamespace) {
// FIXME: For dsymutil-classic compatibility. I think uniquing within
// anonymous namespaces is wrong. There is no ODR guarantee there.
- NameRef = StringPool.internString("(anonymous namespace)");
-
- if (ShortName && ShortName != Name)
- ShortNameRef = StringPool.internString(ShortName);
- else
- ShortNameRef = NameRef;
+ NameRef = "(anonymous namespace)";
+ }
if (Tag != dwarf::DW_TAG_class_type && Tag != dwarf::DW_TAG_structure_type &&
Tag != dwarf::DW_TAG_union_type &&
@@ -121,7 +114,7 @@ PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
// module-defined types do not have a file and line.
ByteSize = dwarf::toUnsigned(DIE.find(dwarf::DW_AT_byte_size),
std::numeric_limits<uint64_t>::max());
- if (Tag != dwarf::DW_TAG_namespace || !Name) {
+ if (Tag != dwarf::DW_TAG_namespace || IsAnonymousNamespace) {
if (unsigned FileNum =
dwarf::toUnsigned(DIE.find(dwarf::DW_AT_decl_file), 0)) {
if (const auto *LT = U.getOrigUnit().getContext().getLineTableForUnit(
@@ -129,29 +122,14 @@ PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
// FIXME: dsymutil-classic compatibility. I'd rather not
// unique anything in anonymous namespaces, but if we do, then
// verify that the file and line correspond.
- if (!Name && Tag == dwarf::DW_TAG_namespace)
+ if (IsAnonymousNamespace)
FileNum = 1;
if (LT->hasFileAtIndex(FileNum)) {
Line = dwarf::toUnsigned(DIE.find(dwarf::DW_AT_decl_line), 0);
// Cache the resolved paths based on the index in the line table,
- // because calling realpath is expansive.
- StringRef ResolvedPath = U.getResolvedPath(FileNum);
- if (!ResolvedPath.empty()) {
- FileRef = ResolvedPath;
- } else {
- std::string File;
- bool FoundFileName = LT->getFileNameByIndex(
- FileNum, U.getOrigUnit().getCompilationDir(),
- DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath,
- File);
- (void)FoundFileName;
- assert(FoundFileName && "Must get file name from line table");
- // Second level of caching, this time based on the file's parent
- // path.
- FileRef = PathResolver.resolve(File, StringPool);
- U.setResolvedPath(FileNum, FileRef);
- }
+ // because calling realpath is expensive.
+ FileRef = getResolvedPath(U, FileNum, *LT);
}
}
}
@@ -175,7 +153,7 @@ PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
// FIXME: dsymutil-classic compatibility: when we don't have a name,
// use the filename.
- if (Tag == dwarf::DW_TAG_namespace && NameRef == "(anonymous namespace)")
+ if (IsAnonymousNamespace)
Hash = hash_combine(Hash, FileRef);
// Now look if this context already exists.
@@ -210,4 +188,28 @@ PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
return PointerIntPair<DeclContext *, 1>(*ContextIter);
}
+StringRef
+DeclContextTree::getResolvedPath(CompileUnit &CU, unsigned FileNum,
+ const DWARFDebugLine::LineTable &LineTable) {
+ std::pair<unsigned, unsigned> Key = {CU.getUniqueID(), FileNum};
+
+ ResolvedPathsMap::const_iterator It = ResolvedPaths.find(Key);
+ if (It == ResolvedPaths.end()) {
+ std::string FileName;
+ bool FoundFileName = LineTable.getFileNameByIndex(
+ FileNum, CU.getOrigUnit().getCompilationDir(),
+ DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, FileName);
+ (void)FoundFileName;
+ assert(FoundFileName && "Must get file name from line table");
+
+ // Second level of caching, this time based on the file's parent
+ // path.
+ StringRef ResolvedPath = PathResolver.resolve(FileName, StringPool);
+
+ It = ResolvedPaths.insert(std::make_pair(Key, ResolvedPath)).first;
+ }
+
+ return It->second;
+}
+
} // namespace llvm
More information about the llvm-branch-commits
mailing list