[DWARFLinkerParallel] Add support for -odr mode. #68721
Conversation
avl-llvm
requested review from
JDevlieghere,
adrian-prantl,
clayborg,
fredriss and
dwblaikie
|
@llvm/pr-subscribers-debuginfo Author: None (avl-llvm) ChangesThis patch is extracted from D96035, it adds support for the type deduplication mode. With this patch DWARFLinkerParallel handles --odr option. It also processes clang modules. run-time performance and memory requirements for clang binary --num-threads 16 : run-time performance and memory requirements for clang binary --num-threads 1 : Note: Sometimes DWARFLinkerParallel may produce non-deterministic results. Note: The dependency tracking algorithm handles DW_TAG_imported_module and Patch is 469.13 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/68721.diff 77 Files Affected:
diff --git a/llvm/lib/DWARFLinkerParallel/AcceleratorRecordsSaver.cpp b/llvm/lib/DWARFLinkerParallel/AcceleratorRecordsSaver.cpp
new file mode 100644
index 000000000000000..5ec25cfe5fd26e1
--- /dev/null
+++ b/llvm/lib/DWARFLinkerParallel/AcceleratorRecordsSaver.cpp
@@ -0,0 +1,295 @@
+//=== AcceleratorRecordsSaver.cpp -----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "AcceleratorRecordsSaver.h"
+#include "Utils.h"
+#include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
+#include "llvm/Support/DJB.h"
+
+namespace llvm {
+namespace dwarflinker_parallel {
+
+static uint32_t hashFullyQualifiedName(CompileUnit &InputCU, DWARFDie &InputDIE,
+ int ChildRecurseDepth = 0) {
+ const char *Name = nullptr;
+ CompileUnit *CU = &InputCU;
+ std::optional<DWARFFormValue> RefVal;
+
+ if (Error Err = finiteLoop([&]() -> Expected<bool> {
+ if (const char *CurrentName = InputDIE.getName(DINameKind::ShortName))
+ Name = CurrentName;
+
+ if (!(RefVal = InputDIE.find(dwarf::DW_AT_specification)) &&
+ !(RefVal = InputDIE.find(dwarf::DW_AT_abstract_origin)))
+ return false;
+
+ if (!RefVal->isFormClass(DWARFFormValue::FC_Reference))
+ return false;
+
+ std::optional<UnitEntryPairTy> RefDie = CU->resolveDIEReference(
+ *RefVal, ResolveInterCUReferencesMode::Resolve);
+ if (!RefDie)
+ return false;
+
+ if (!RefDie->DieEntry)
+ return false;
+
+ CU = RefDie->CU;
+ InputDIE = RefDie->CU->getDIE(RefDie->DieEntry);
+ return true;
+ })) {
+ consumeError(std::move(Err));
+ }
+
+ if (!Name && InputDIE.getTag() == dwarf::DW_TAG_namespace)
+ Name = "(anonymous namespace)";
+
+ DWARFDie ParentDie = InputDIE.getParent();
+ if (!ParentDie.isValid() || ParentDie.getTag() == dwarf::DW_TAG_compile_unit)
+ return djbHash(Name ? Name : "", djbHash(ChildRecurseDepth ? "" : "::"));
+
+ return djbHash(
+ (Name ? Name : ""),
+ djbHash((Name ? "::" : ""),
+ hashFullyQualifiedName(*CU, ParentDie, ++ChildRecurseDepth)));
+}
+
+void AcceleratorRecordsSaver::save(const DWARFDebugInfoEntry *InputDieEntry,
+ DIE *OutDIE, AttributesInfo &AttrInfo,
+ TypeEntry *TypeEntry) {
+ if (GlobalData.getOptions().AccelTables.empty())
+ return;
+
+ DWARFDie InputDIE = InUnit.getDIE(InputDieEntry);
+
+ // Look for short name recursively if short name is not known yet.
+ if (AttrInfo.Name == nullptr)
+ if (const char *ShortName = InputDIE.getShortName())
+ AttrInfo.Name = GlobalData.getStringPool().insert(ShortName).first;
+
+ switch (InputDieEntry->getTag()) {
+ case dwarf::DW_TAG_array_type:
+ case dwarf::DW_TAG_class_type:
+ case dwarf::DW_TAG_enumeration_type:
+ case dwarf::DW_TAG_pointer_type:
+ case dwarf::DW_TAG_reference_type:
+ case dwarf::DW_TAG_string_type:
+ case dwarf::DW_TAG_structure_type:
+ case dwarf::DW_TAG_subroutine_type:
+ case dwarf::DW_TAG_typedef:
+ case dwarf::DW_TAG_union_type:
+ case dwarf::DW_TAG_ptr_to_member_type:
+ case dwarf::DW_TAG_set_type:
+ case dwarf::DW_TAG_subrange_type:
+ case dwarf::DW_TAG_base_type:
+ case dwarf::DW_TAG_const_type:
+ case dwarf::DW_TAG_constant:
+ case dwarf::DW_TAG_file_type:
+ case dwarf::DW_TAG_namelist:
+ case dwarf::DW_TAG_packed_type:
+ case dwarf::DW_TAG_volatile_type:
+ case dwarf::DW_TAG_restrict_type:
+ case dwarf::DW_TAG_atomic_type:
+ case dwarf::DW_TAG_interface_type:
+ case dwarf::DW_TAG_unspecified_type:
+ case dwarf::DW_TAG_shared_type:
+ case dwarf::DW_TAG_immutable_type:
+ case dwarf::DW_TAG_rvalue_reference_type: {
+ if (!AttrInfo.IsDeclaration && AttrInfo.Name != nullptr &&
+ !AttrInfo.Name->getKey().empty()) {
+ uint32_t Hash = hashFullyQualifiedName(InUnit, InputDIE);
+
+ uint64_t RuntimeLang =
+ dwarf::toUnsigned(InputDIE.find(dwarf::DW_AT_APPLE_runtime_class))
+ .value_or(0);
+
+ bool ObjCClassIsImplementation =
+ (RuntimeLang == dwarf::DW_LANG_ObjC ||
+ RuntimeLang == dwarf::DW_LANG_ObjC_plus_plus) &&
+ dwarf::toUnsigned(
+ InputDIE.find(dwarf::DW_AT_APPLE_objc_complete_type))
+ .value_or(0);
+
+ saveTypeRecord(AttrInfo.Name, OutDIE, InputDieEntry->getTag(), Hash,
+ ObjCClassIsImplementation, TypeEntry);
+ }
+ } break;
+ case dwarf::DW_TAG_namespace: {
+ if (AttrInfo.Name == nullptr)
+ AttrInfo.Name =
+ GlobalData.getStringPool().insert("(anonymous namespace)").first;
+
+ saveNamespaceRecord(AttrInfo.Name, OutDIE, InputDieEntry->getTag(),
+ TypeEntry);
+ } break;
+ case dwarf::DW_TAG_imported_declaration: {
+ if (AttrInfo.Name != nullptr)
+ saveNamespaceRecord(AttrInfo.Name, OutDIE, InputDieEntry->getTag(),
+ TypeEntry);
+ } break;
+ case dwarf::DW_TAG_compile_unit:
+ case dwarf::DW_TAG_lexical_block: {
+ // Nothing to do.
+ } break;
+ default:
+ if (TypeEntry)
+ // Do not store this kind of accelerator entries for type entries.
+ return;
+
+ if (AttrInfo.HasLiveAddress || AttrInfo.HasRanges) {
+ if (AttrInfo.Name)
+ saveNameRecord(AttrInfo.Name, OutDIE, InputDieEntry->getTag(),
+ InputDieEntry->getTag() ==
+ dwarf::DW_TAG_inlined_subroutine);
+
+ // Look for mangled name recursively if mangled name is not known yet.
+ if (!AttrInfo.MangledName)
+ if (const char *LinkageName = InputDIE.getLinkageName())
+ AttrInfo.MangledName =
+ GlobalData.getStringPool().insert(LinkageName).first;
+
+ if (AttrInfo.MangledName && AttrInfo.MangledName != AttrInfo.Name)
+ saveNameRecord(AttrInfo.MangledName, OutDIE, InputDieEntry->getTag(),
+ InputDieEntry->getTag() ==
+ dwarf::DW_TAG_inlined_subroutine);
+
+ // Strip template parameters from the short name.
+ if (AttrInfo.Name && AttrInfo.MangledName != AttrInfo.Name &&
+ (InputDieEntry->getTag() != dwarf::DW_TAG_inlined_subroutine)) {
+ if (std::optional<StringRef> Name =
+ StripTemplateParameters(AttrInfo.Name->getKey())) {
+ StringEntry *NameWithoutTemplateParams =
+ GlobalData.getStringPool().insert(*Name).first;
+
+ saveNameRecord(NameWithoutTemplateParams, OutDIE,
+ InputDieEntry->getTag(), true);
+ }
+ }
+
+ if (AttrInfo.Name)
+ saveObjC(InputDieEntry, OutDIE, AttrInfo);
+ }
+ break;
+ }
+}
+
+void AcceleratorRecordsSaver::saveObjC(const DWARFDebugInfoEntry *InputDieEntry,
+ DIE *OutDIE, AttributesInfo &AttrInfo) {
+ std::optional<ObjCSelectorNames> Names =
+ getObjCNamesIfSelector(AttrInfo.Name->getKey());
+ if (!Names)
+ return;
+
+ StringEntry *Selector =
+ GlobalData.getStringPool().insert(Names->Selector).first;
+ saveNameRecord(Selector, OutDIE, InputDieEntry->getTag(), true);
+ StringEntry *ClassName =
+ GlobalData.getStringPool().insert(Names->ClassName).first;
+ saveObjCNameRecord(ClassName, OutDIE, InputDieEntry->getTag());
+ if (Names->ClassNameNoCategory) {
+ StringEntry *ClassNameNoCategory =
+ GlobalData.getStringPool().insert(*Names->ClassNameNoCategory).first;
+ saveObjCNameRecord(ClassNameNoCategory, OutDIE, InputDieEntry->getTag());
+ }
+ if (Names->MethodNameNoCategory) {
+ StringEntry *MethodNameNoCategory =
+ GlobalData.getStringPool().insert(*Names->MethodNameNoCategory).first;
+ saveNameRecord(MethodNameNoCategory, OutDIE, InputDieEntry->getTag(), true);
+ }
+}
+
+void AcceleratorRecordsSaver::saveNameRecord(StringEntry *Name, DIE *OutDIE,
+ dwarf::Tag Tag,
+ bool AvoidForPubSections) {
+ DwarfUnit::AccelInfo Info;
+
+ Info.Type = DwarfUnit::AccelType::Name;
+ Info.String = Name;
+ Info.OutOffset = OutDIE->getOffset();
+ Info.Tag = Tag;
+ Info.AvoidForPubSections = AvoidForPubSections;
+
+ OutUnit.getAsCompileUnit()->saveAcceleratorInfo(Info);
+}
+void AcceleratorRecordsSaver::saveNamespaceRecord(StringEntry *Name,
+ DIE *OutDIE, dwarf::Tag Tag,
+ TypeEntry *TypeEntry) {
+ if (OutUnit.isCompileUnit()) {
+ assert(TypeEntry == nullptr);
+ DwarfUnit::AccelInfo Info;
+
+ Info.Type = DwarfUnit::AccelType::Namespace;
+ Info.String = Name;
+ Info.OutOffset = OutDIE->getOffset();
+ Info.Tag = Tag;
+
+ OutUnit.getAsCompileUnit()->saveAcceleratorInfo(Info);
+ return;
+ }
+
+ assert(TypeEntry != nullptr);
+ TypeUnit::TypeUnitAccelInfo Info;
+ Info.Type = DwarfUnit::AccelType::Namespace;
+ Info.String = Name;
+ Info.OutOffset = 0xbaddef;
+ Info.Tag = Tag;
+ Info.OutDIE = OutDIE;
+ Info.TypeEntryBodyPtr = TypeEntry->getValue().load();
+
+ OutUnit.getAsTypeUnit()->saveAcceleratorInfo(Info);
+}
+
+void AcceleratorRecordsSaver::saveObjCNameRecord(StringEntry *Name, DIE *OutDIE,
+ dwarf::Tag Tag) {
+ DwarfUnit::AccelInfo Info;
+
+ Info.Type = DwarfUnit::AccelType::ObjC;
+ Info.String = Name;
+ Info.OutOffset = OutDIE->getOffset();
+ Info.Tag = Tag;
+ Info.AvoidForPubSections = true;
+
+ OutUnit.getAsCompileUnit()->saveAcceleratorInfo(Info);
+}
+
+void AcceleratorRecordsSaver::saveTypeRecord(StringEntry *Name, DIE *OutDIE,
+ dwarf::Tag Tag,
+ uint32_t QualifiedNameHash,
+ bool ObjcClassImplementation,
+ TypeEntry *TypeEntry) {
+ if (OutUnit.isCompileUnit()) {
+ assert(TypeEntry == nullptr);
+ DwarfUnit::AccelInfo Info;
+
+ Info.Type = DwarfUnit::AccelType::Type;
+ Info.String = Name;
+ Info.OutOffset = OutDIE->getOffset();
+ Info.Tag = Tag;
+ Info.QualifiedNameHash = QualifiedNameHash;
+ Info.ObjcClassImplementation = ObjcClassImplementation;
+
+ OutUnit.getAsCompileUnit()->saveAcceleratorInfo(Info);
+ return;
+ }
+
+ assert(TypeEntry != nullptr);
+ TypeUnit::TypeUnitAccelInfo Info;
+
+ Info.Type = DwarfUnit::AccelType::Type;
+ Info.String = Name;
+ Info.OutOffset = 0xbaddef;
+ Info.Tag = Tag;
+ Info.QualifiedNameHash = QualifiedNameHash;
+ Info.ObjcClassImplementation = ObjcClassImplementation;
+ Info.OutDIE = OutDIE;
+ Info.TypeEntryBodyPtr = TypeEntry->getValue().load();
+ OutUnit.getAsTypeUnit()->saveAcceleratorInfo(Info);
+}
+
+} // end of namespace dwarflinker_parallel
+} // namespace llvm
diff --git a/llvm/lib/DWARFLinkerParallel/AcceleratorRecordsSaver.h b/llvm/lib/DWARFLinkerParallel/AcceleratorRecordsSaver.h
new file mode 100644
index 000000000000000..5e7f4d0c3166fd1
--- /dev/null
+++ b/llvm/lib/DWARFLinkerParallel/AcceleratorRecordsSaver.h
@@ -0,0 +1,70 @@
+//===- AcceleratorRecordsSaver.h --------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIB_DWARFLINKERPARALLEL_ACCELERATORRECORDSSAVER_H
+#define LLVM_LIB_DWARFLINKERPARALLEL_ACCELERATORRECORDSSAVER_H
+
+#include "DIEAttributeCloner.h"
+#include "DWARFLinkerCompileUnit.h"
+#include "DWARFLinkerGlobalData.h"
+#include "DWARFLinkerTypeUnit.h"
+
+namespace llvm {
+namespace dwarflinker_parallel {
+
+/// This class helps to store information for accelerator entries.
+/// It prepares accelerator info for the certain DIE and store it inside
+/// OutUnit.
+class AcceleratorRecordsSaver {
+public:
+ AcceleratorRecordsSaver(LinkingGlobalData &GlobalData, CompileUnit &InUnit,
+ CompileUnit *OutUnit)
+ : AcceleratorRecordsSaver(GlobalData, InUnit,
+ CompileUnit::OutputUnitVariantPtr(OutUnit)) {}
+
+ AcceleratorRecordsSaver(LinkingGlobalData &GlobalData, CompileUnit &InUnit,
+ TypeUnit *OutUnit)
+ : AcceleratorRecordsSaver(GlobalData, InUnit,
+ CompileUnit::OutputUnitVariantPtr(OutUnit)) {}
+
+ /// Save accelerator info for the specified \p OutDIE inside OutUnit.
+ /// Side effects: set attributes in \p AttrInfo.
+ void save(const DWARFDebugInfoEntry *InputDieEntry, DIE *OutDIE,
+ AttributesInfo &AttrInfo, TypeEntry *TypeEntry);
+
+protected:
+ AcceleratorRecordsSaver(LinkingGlobalData &GlobalData, CompileUnit &InUnit,
+ CompileUnit::OutputUnitVariantPtr OutUnit)
+ : GlobalData(GlobalData), InUnit(InUnit), OutUnit(OutUnit) {}
+
+ void saveObjC(const DWARFDebugInfoEntry *InputDieEntry, DIE *OutDIE,
+ AttributesInfo &AttrInfo);
+
+ void saveNameRecord(StringEntry *Name, DIE *OutDIE, dwarf::Tag Tag,
+ bool AvoidForPubSections);
+ void saveNamespaceRecord(StringEntry *Name, DIE *OutDIE, dwarf::Tag Tag,
+ TypeEntry *TypeEntry);
+ void saveObjCNameRecord(StringEntry *Name, DIE *OutDIE, dwarf::Tag Tag);
+ void saveTypeRecord(StringEntry *Name, DIE *OutDIE, dwarf::Tag Tag,
+ uint32_t QualifiedNameHash, bool ObjcClassImplementation,
+ TypeEntry *TypeEntry);
+
+ /// Global linking data.
+ LinkingGlobalData &GlobalData;
+
+ /// Comiple unit corresponding to input DWARF.
+ CompileUnit &InUnit;
+
+ /// Compile unit or Artificial type unit corresponding to the output DWARF.
+ CompileUnit::OutputUnitVariantPtr OutUnit;
+};
+
+} // end of namespace dwarflinker_parallel
+} // end namespace llvm
+
+#endif // LLVM_LIB_DWARFLINKERPARALLEL_ACCELERATORRECORDSSAVER_H
diff --git a/llvm/lib/DWARFLinkerParallel/ArrayList.h b/llvm/lib/DWARFLinkerParallel/ArrayList.h
index 58d550982c8dfc0..def83f91bc6f319 100644
--- a/llvm/lib/DWARFLinkerParallel/ArrayList.h
+++ b/llvm/lib/DWARFLinkerParallel/ArrayList.h
@@ -21,6 +21,9 @@ namespace dwarflinker_parallel {
/// Method add() can be called asynchronously.
template <typename T, size_t ItemsGroupSize = 512> class ArrayList {
public:
+ ArrayList(parallel::PerThreadBumpPtrAllocator *Allocator)
+ : Allocator(Allocator) {}
+
/// Add specified \p Item to the list.
T &add(const T &Item) {
assert(Allocator);
@@ -73,8 +76,27 @@ template <typename T, size_t ItemsGroupSize = 512> class ArrayList {
LastGroup = nullptr;
}
- void setAllocator(parallel::PerThreadBumpPtrAllocator *Allocator) {
- this->Allocator = Allocator;
+ void sort(function_ref<bool(const T &LHS, const T &RHS)> Comparator) {
+ SmallVector<T> SortedItems;
+ forEach([&](T &Item) { SortedItems.push_back(Item); });
+
+ if (SortedItems.size()) {
+ std::sort(SortedItems.begin(), SortedItems.end(), Comparator);
+
+ size_t SortedItemIdx = 0;
+ forEach([&](T &Item) { Item = SortedItems[SortedItemIdx++]; });
+ assert(SortedItemIdx == SortedItems.size());
+ }
+ }
+
+ size_t size() {
+ size_t Result = 0;
+
+ for (ItemsGroup *CurGroup = GroupsHead; CurGroup != nullptr;
+ CurGroup = CurGroup->Next)
+ Result += CurGroup->getItemsCount();
+
+ return Result;
}
protected:
diff --git a/llvm/lib/DWARFLinkerParallel/CMakeLists.txt b/llvm/lib/DWARFLinkerParallel/CMakeLists.txt
index d321ecf8d5ce847..b0f0b3910e586ab 100644
--- a/llvm/lib/DWARFLinkerParallel/CMakeLists.txt
+++ b/llvm/lib/DWARFLinkerParallel/CMakeLists.txt
@@ -1,14 +1,17 @@
add_llvm_component_library(LLVMDWARFLinkerParallel
+ AcceleratorRecordsSaver.cpp
DependencyTracker.cpp
DIEAttributeCloner.cpp
DWARFEmitterImpl.cpp
DWARFFile.cpp
DWARFLinker.cpp
DWARFLinkerCompileUnit.cpp
+ DWARFLinkerTypeUnit.cpp
DWARFLinkerImpl.cpp
DWARFLinkerUnit.cpp
OutputSections.cpp
StringPool.cpp
+ SyntheticTypeNameBuilder.cpp
ADDITIONAL_HEADER_DIRS
${LLVM_MAIN_INCLUDE_DIR}/llvm/DWARFLinkerParallel
diff --git a/llvm/lib/DWARFLinkerParallel/DIEAttributeCloner.cpp b/llvm/lib/DWARFLinkerParallel/DIEAttributeCloner.cpp
index d05fd8d61b85743..81fc57f7cabbb79 100644
--- a/llvm/lib/DWARFLinkerParallel/DIEAttributeCloner.cpp
+++ b/llvm/lib/DWARFLinkerParallel/DIEAttributeCloner.cpp
@@ -13,18 +13,16 @@ namespace llvm {
namespace dwarflinker_parallel {
void DIEAttributeCloner::clone() {
- DWARFUnit &U = CU.getOrigUnit();
-
// Extract and clone every attribute.
- DWARFDataExtractor Data = U.getDebugInfoExtractor();
+ DWARFDataExtractor Data = InUnit.getOrigUnit().getDebugInfoExtractor();
uint64_t Offset = InputDieEntry->getOffset();
// Point to the next DIE (generally there is always at least a NULL
// entry after the current one). If this is a lone
// DW_TAG_compile_unit without any children, point to the next unit.
- uint64_t NextOffset = (InputDIEIdx + 1 < U.getNumDIEs())
- ? U.getDIEAtIndex(InputDIEIdx + 1).getOffset()
- : U.getNextUnitOffset();
+ uint64_t NextOffset = (InputDIEIdx + 1 < InUnit.getOrigUnit().getNumDIEs())
+ ? InUnit.getDIEAtIndex(InputDIEIdx + 1).getOffset()
+ : InUnit.getOrigUnit().getNextUnitOffset();
// We could copy the data only if we need to apply a relocation to it. After
// testing, it seems there is no performance downside to doing the copy
@@ -34,8 +32,8 @@ void DIEAttributeCloner::clone() {
DWARFDataExtractor(DIECopy, Data.isLittleEndian(), Data.getAddressSize());
// Modify the copy with relocated addresses.
- CU.getContaingFile().Addresses->applyValidRelocs(DIECopy, Offset,
- Data.isLittleEndian());
+ InUnit.getContaingFile().Addresses->applyValidRelocs(DIECopy, Offset,
+ Data.isLittleEndian());
// Reset the Offset to 0 as we will be working on the local copy of
// the data.
@@ -45,17 +43,18 @@ void DIEAttributeCloner::clone() {
Offset += getULEB128Size(Abbrev->getCode());
// Set current output offset.
- AttrOutOffset = OutDIE->getOffset();
+ AttrOutOffset = OutUnit.isCompileUnit() ? OutDIE->getOffset() : 0;
for (const auto &AttrSpec : Abbrev->attributes()) {
// Check whether current attribute should be skipped.
if (shouldSkipAttribute(AttrSpec)) {
DWARFFormValue::skipValue(AttrSpec.Form, Data, &Offset,
- U.getFormParams());
+ InUnit.getFormParams());
continue;
}
DWARFFormValue Val = AttrSpec.getFormValue();
- Val.extractValue(Data, &Offset, U.getFormParams(), &U);
+ Val.extractValue(Data, &Offset, InUnit.getFormParams(),
+ &InUnit.getOrigUnit());
// Clone current attribute.
switch (AttrSpec.Form) {
@@ -107,10 +106,10 @@ void DIEAttributeCloner::clone() {
AttrOutOffset += cloneAddressAttr(Val, AttrSpec);
break;
default:
- CU.warn("unsupported attribute form " +
- dwarf::FormEncodingString(AttrSpec.Form) +
- " in DieAttributeCloner::clone(). Dropping.",
- InputDieEntry);
+ InUnit.warn("unsupported attribute form " +
+ dwarf::FormEncodingString(AttrSpec.Form) +
+ " in DieAttributeCloner::clone(). Dropping.",
+ InputDieEntry);
}
}
@@ -118,19 +117,20 @@ void DIEAttributeCloner::clone() {
// Check if original compile unit already has DW_AT_str_offsets_base
// attribute.
if (InputDieEntry->getTag() == dwarf::DW_TAG_compile_unit &&
- CU.getVersion() >= 5 && !AttrInfo.HasStringOffsetBaseAttr) {
+ InUnit.getVersion() >= 5 && !AttrInfo.HasStringOffsetBaseAttr) {
DebugInfoOutput...
[truncated]
|
|
Why is the debug info section so much smaller when using the parallel linker? Is it really that much more efficient than the Apple one, and if so, where are the gains coming from? Are we eliminating types that the dsymutil isn't, like base types? I'm a little skeptical, but I can also imagine the new approach is a lot better because it doesn't need to keep things like forward declarations until it sees a definition, etc.
I know you're aware but I'll emphasize again that that's a non-starter for us. It sounds like you have patches in the works so that's great! |
yes, the size of debug info is smaller because the parallel linker does ODR deduplication differently:
llvm-xray built with llvm linker: llvm-xray built with apple linker:
yes, I intend to have deterministic output. I think it would be better to achieve this goal with separate patches as the current patch is already huge. The problem that leads to non-deterministic output is caused by inconsistent DWARF(**). It contains different definitions for the same type. So the definition that would be put in the result depends on which definition was handled earlier(which is non-deterministic). I think the proper way to fix that problem would be to fix Clang's DWARF generation to produce consistent results. (*) Example of types merging: Lets assume we have following types: Above partial definitions would be united into single one: (**) Example of inconsistent DWARF:
compile unit "llvm/tools/opt/opt.cpp" has following definition: while compile unit "llvm/lib/Target/X86/X86LowerAMXIntrinsics.cpp" has a bit different:
compile unit "llvm/lib/Bitcode/Reader/BitcodeReader.cpp" has following definition: while compile unit "llvm/lib/IR/SSAContext.cpp" has a bit different: |
|
DWARFLinker would still need to be deterministic, even on old/bad/different input - though maybe Apple & other folks would be open to it having an error on "bad" input rather than having to figure out a way to have a deterministic successful output. That said - I'm always here for a conversation about type consistency in DWARF... so let's take a look:
|
+1
As long as the input DWARF is valid, we need to be able to link it. Maybe a slow path would be okay if we can guarantee our toolchain doesn't trigger it (i.e. clang & Swift). |
that could be done with the cost of slower execution.
Speaking of original example, Do I correctly understand that "DW_AT_const_value (0)" should be in both type definition copies(0x001c4090 and 0x010f623d) in currently generated DWARF?
|
yes, more slower but deterministic handling is in my future plans. And we can use slower version with unknown sources and faster version with DWARF generated by clang&Swift (if inconsistent cases would be resolved). |
I'm not actually sure. I'm not sure if we can always compute the value reliably in every context (some parts of templates we're not allowed to instantiate only in certain contexts). If we couldn't do it in all cases, maybe we could not put the value on the declaration in all cases - and use a definition DIE to attach the value to (& not put a DW_AT_location on that definition, because it might not have storage in translation units, etc). |
I see. Thank you for explanations. |
|
friendly ping. |
…eclarations Since `f07cf5aaf96a70123530239d07c1e655dc91b98d` we emit a definition for any static data member with a constant initializer. That definition will either have a location or constant attached to it. So having the constant on the declaration is redundant. It also helps with the type merging inconsistencies encountered in llvm#68721
|
friendly ping. |
|
@JDevlieghere @adrian-prantl are there anything which should be done with this patch before integration? |
|
Did we come up with a solution for determinism that doesn't depend on the generator? My understanding was that we're okay with taking a penalty in terms of performance, but does the code reflect this, or will the current version of this patch still generate non-deterministic results with other producers or older version of clang that don't have Michael's changes? |
This version of patch will produce non-deterministic results if input have non-consistent DWARF(these are old-producers and current clang(even with Michael's patch)). I proposed to implement full-deterministic version as a follow-up patch. Current patch is already huge and it would be good to reduce it's size. I suggest following order of patches:
What do you think? |
|
Thanks! |
This patch is extracted from D96035, it adds support for the type deduplication mode. With this patch DWARFLinkerParallel handles --odr option. It also processes clang modules.
avl-llvm
force-pushed
the
avl-llvm/add-odr-to-dwarflinkerparallel
branch
from
bef2dd8 to
66b03b2
Compare
|
This change broke building on 32-bit platforms, see #73267. |
…mber declarations (#73626) In #71780 we started emitting definitions for all static data-members with constant initialisers, even if they were constants (i.e., didn't have a location). We also dropped the DW_AT_const_value from the declaration to [help resolve inconsistencies during type merging in the DWARFParallelLinker](#68721). However, for static data members that do have locations, we wouldn't emit a DW_AT_const_value on it, assuming that the consumer knows how to read the value using the location. This broke some consumers that really wanted to find a DW_AT_const_value. Ultimately we want to attach a DW_AT_const_value to definitions that have a location too. But to fix consumers broken by said change, this patch adds the constant back onto the declaration. This is what we used to do prior to #71780
This patch is extracted from D96035, it adds support for the type deduplication mode. With this patch DWARFLinkerParallel handles --odr option. It also processes clang modules. (cherry picked from commit b61ac4a)
This patch is extracted from D96035, it adds support for the type deduplication mode. With this patch DWARFLinkerParallel handles --odr option. It also processes clang modules.
run-time performance and memory requirements for clang binary --num-threads 16 :
run-time performance and memory requirements for clang binary --num-threads 1 :
Note: Sometimes DWARFLinkerParallel may produce non-deterministic results.
The reason for that is ambiguous input DWARF. That problem is assumed
to be solved with separate patches(most probably not for DWARFLinkerParallel
but for generated DWARF).
Note: The dependency tracking algorithm handles DW_TAG_imported_module and
DW_TAG_imported_declaration differently than current DWARFLinker. Current
DWARFLinker keeps all content referenced by DW_AT_import attribute despite
the fact whether it references live code or not. This patch keeps only DIEs
referencing live addresses(and all their dependencies)