[llvm] [AccelTable] Order colliding hashes by name (PR #220109)
Jonas Devlieghere via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 1 08:56:14 PDT 2026
https://github.com/JDevlieghere updated https://github.com/llvm/llvm-project/pull/220109
>From 0b63b1f5096e9f2a3965a7f35d015b0df77fd828 Mon Sep 17 00:00:00 2001
From: Jonas Devlieghere <jonas at devlieghere.com>
Date: Tue, 1 Sep 2026 08:55:56 -0700
Subject: [PATCH] [AccelTable] Order colliding hashes by name
Sorting the buckets only looked at the hash value, so names whose hashes
collide came out in the order they were added. The parallel DWARF linker
gathers accelerator records from all cloning threads into a lock-free
list. Since the order would vary between runs, it would result in
different bytes, while still being semantically equivalent.
We can break ties by name to provide a total order. Without equal
entries, there is nothing for a stable sort to preserve so we can drop
llvm::stable_sort and use llvm::sort again.
The temporary symbols were created while walking Entries, which is in
insertion order too, so the assembly for a sorted bucket list came out
scrambled. Create them after the sort instead.
This covers the bucket contents only. The per-name Values sort still
orders on DWARF5AccelTableData::order(), which is the unit-relative DIE
offset alone, and .debug_pubnames for the artificial type unit still
replays the same unconstrained list. Both need their own fix.
Assisted-by: Claude
---
llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp | 24 +++---
.../Generic/accel-table-hash-collisions.ll | 13 ++--
.../Generic/debug-names-hash-collisions.ll | 36 ++++++---
llvm/unittests/CodeGen/AccelTableTest.cpp | 78 +++++++++++++++++++
llvm/unittests/CodeGen/CMakeLists.txt | 1 +
5 files changed, 126 insertions(+), 26 deletions(-)
create mode 100644 llvm/unittests/CodeGen/AccelTableTest.cpp
diff --git a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp
index a5f55707ad284..83171fd8785fd 100644
--- a/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/AccelTable.cpp
@@ -60,18 +60,24 @@ void AccelTableBase::finalize(AsmPrinter *Asm, StringRef Prefix) {
// Compute bucket contents and final ordering.
Buckets.resize(BucketCount);
- for (auto &E : Entries) {
- uint32_t Bucket = E.second.HashValue % BucketCount;
- Buckets[Bucket].push_back(&E.second);
- E.second.Sym = Asm->createTempSymbol(Prefix);
- }
+ for (auto &E : Entries)
+ Buckets[E.second.HashValue % BucketCount].push_back(&E.second);
// Sort the contents of the buckets by hash value so that hash collisions end
- // up together. Stable sort makes testing easier and doesn't cost much more.
- for (auto &Bucket : Buckets)
- llvm::stable_sort(Bucket, [](HashData *LHS, HashData *RHS) {
- return LHS->HashValue < RHS->HashValue;
+ // up together. Entries is keyed by name, so breaking ties by name yields a
+ // total order that does not depend on the order names were added in.
+ for (HashList &Bucket : Buckets)
+ llvm::sort(Bucket, [](const HashData *LHS, const HashData *RHS) {
+ if (LHS->HashValue != RHS->HashValue)
+ return LHS->HashValue < RHS->HashValue;
+ return LHS->Name.getString() < RHS->Name.getString();
});
+
+ // Create the labels in bucket order so that their numbering matches the
+ // order they are emitted in.
+ for (HashList &Bucket : Buckets)
+ for (HashData *Hash : Bucket)
+ Hash->Sym = Asm->createTempSymbol(Prefix);
}
namespace {
diff --git a/llvm/test/DebugInfo/Generic/accel-table-hash-collisions.ll b/llvm/test/DebugInfo/Generic/accel-table-hash-collisions.ll
index 7ed21f7623759..e4cc73a6dbe41 100644
--- a/llvm/test/DebugInfo/Generic/accel-table-hash-collisions.ll
+++ b/llvm/test/DebugInfo/Generic/accel-table-hash-collisions.ll
@@ -26,30 +26,31 @@
; CHECK: Bucket count: 6
; CHECK: Hashes count: 6
-; Check that all the names are present in the output
+; Check that all the names are present in the output. Names that share a hash
+; are ordered by name.
; CHECK: Hash 0x597841
-; CHECK: String: 0x{{[0-9a-f]*}} "k1"
; CHECK: String: 0x{{[0-9a-f]*}} "is"
+; CHECK: String: 0x{{[0-9a-f]*}} "k1"
; CHECK: Hash 0xa4b42a1e
-; CHECK: String: 0x{{[0-9a-f]*}} "_ZN5clang23DataRecursiveASTVisitorIN12_GLOBAL__N_124UnusedBackingIvarCheckerEE26TraverseCUDAKernelCallExprEPNS_18CUDAKernelCallExprE"
; CHECK: String: 0x{{[0-9a-f]*}} "_ZN4llvm16DenseMapIteratorIPNS_10MDLocationENS_6detail13DenseSetEmptyENS_10MDNodeInfoIS1_EENS3_12DenseSetPairIS2_EELb0EE23AdvancePastEmptyBucketsEv"
+; CHECK: String: 0x{{[0-9a-f]*}} "_ZN5clang23DataRecursiveASTVisitorIN12_GLOBAL__N_124UnusedBackingIvarCheckerEE26TraverseCUDAKernelCallExprEPNS_18CUDAKernelCallExprE"
; CHECK: Hash 0xeee7c0b2
-; CHECK: String: 0x{{[0-9a-f]*}} "_ZNK4llvm12LivePhysRegs5printERNS_11raw_ostreamE"
; CHECK: String: 0x{{[0-9a-f]*}} "_ZN4llvm15ScalarEvolution14getSignedRangeEPKNS_4SCEVE"
+; CHECK: String: 0x{{[0-9a-f]*}} "_ZNK4llvm12LivePhysRegs5printERNS_11raw_ostreamE"
; CHECK: Hash 0xea48ac5f
; CHECK: String: 0x{{[0-9a-f]*}} "ForceTopDown"
; CHECK: String: 0x{{[0-9a-f]*}} "_ZNSt3__116allocator_traitsINS_9allocatorINS_11__tree_nodeINS_12__value_typeIPN4llvm10BasicBlockEPNS4_10RegionNodeEEEPvEEEEE11__constructIS9_JNS_4pairIS6_S8_EEEEEvNS_17integral_constantIbLb1EEERSC_PT_DpOT0_"
; CHECK: Hash 0x6b22f71f
-; CHECK: String: 0x{{[0-9a-f]*}} "_ZNK5clang12OverrideAttr5cloneERNS_10ASTContextE"
; CHECK: String: 0x{{[0-9a-f]*}} "_ZN4llvm22MachineModuleInfoMachOD2Ev"
+; CHECK: String: 0x{{[0-9a-f]*}} "_ZNK5clang12OverrideAttr5cloneERNS_10ASTContextE"
; CHECK: Hash 0x8c248979
-; CHECK: String: 0x{{[0-9a-f]*}} "setStmt"
; CHECK: String: 0x{{[0-9a-f]*}} "_ZN4llvm5TwineC1Ei"
+; CHECK: String: 0x{{[0-9a-f]*}} "setStmt"
source_filename = "test/DebugInfo/Generic/accel-table-hash-collisions.ll"
diff --git a/llvm/test/DebugInfo/Generic/debug-names-hash-collisions.ll b/llvm/test/DebugInfo/Generic/debug-names-hash-collisions.ll
index ee35d75eb7814..bc39aa8af8b2a 100644
--- a/llvm/test/DebugInfo/Generic/debug-names-hash-collisions.ll
+++ b/llvm/test/DebugInfo/Generic/debug-names-hash-collisions.ll
@@ -2,6 +2,7 @@
; RUN: %llc_dwarf -accel-tables=Dwarf -filetype=obj -o %t < %s
; RUN: llvm-dwarfdump -debug-names %t | FileCheck %s
; RUN: llvm-dwarfdump -debug-names -verify %t | FileCheck --check-prefix=VERIFY %s
+; RUN: %llc_dwarf -accel-tables=Dwarf -filetype=asm -o - < %s | FileCheck --check-prefix=ASM %s
; Generated from the following C code using
; clang -S -emit-llvm -g col.c
@@ -26,38 +27,51 @@
; CHECK: Bucket count: 5
; CHECK: Name count: 10
-; Check that all the names are present in the output
+; Check that all the names are present in the output. Names that share a hash
+; are ordered by name.
; CHECK: Bucket 0
; CHECK: Hash: 0xF8CF70D
-; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBBlockaSERKS0_"
-; CHECK: Hash: 0xF8CF70D
; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBBlockC1ERKS0_"
-; CHECK: Hash: 0x135A482C
-; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBErroraSERKS0_"
+; CHECK: Hash: 0xF8CF70D
+; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBBlockaSERKS0_"
; CHECK: Hash: 0x135A482C
; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBErrorC1ERKS0_"
+; CHECK: Hash: 0x135A482C
+; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBErroraSERKS0_"
; CHECK-NOT: String:
; CHECK: Bucket 1
; CHECK-NEXT: EMPTY
; CHECK: Bucket 2
; CHECK: Hash: 0x2841B989
-; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZL11numCommutes"
-; CHECK: Hash: 0x2841B989
; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZL11NumCommutes"
-; CHECK: Hash: 0x3E190F5F
-; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZL9NumRemats"
+; CHECK: Hash: 0x2841B989
+; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZL11numCommutes"
; CHECK: Hash: 0x3E190F5F
; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZL9NumReMats"
+; CHECK: Hash: 0x3E190F5F
+; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZL9NumRemats"
; CHECK-NOT: String:
; CHECK: Bucket 3
; CHECK: Hash: 0x2642207F
-; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBValueaSERKS0_"
-; CHECK: Hash: 0x2642207F
; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBValueC1ERKS0_"
+; CHECK: Hash: 0x2642207F
+; CHECK-NEXT:String: 0x{{[0-9a-f]*}} "_ZN4lldb7SBValueaSERKS0_"
; CHECK-NOT: String:
; CHECK: Bucket 4
; CHECK-NEXT: EMPTY
+; Check that the labels are created in emission order.
+; ASM: names0:
+; ASM: names1:
+; ASM: names2:
+; ASM: names3:
+; ASM: names4:
+; ASM: names5:
+; ASM: names6:
+; ASM: names7:
+; ASM: names8:
+; ASM: names9:
+
; VERIFY: No errors.
@_ZN4lldb7SBBlockC1ERKS0_ = common dso_local global ptr null, align 8, !dbg !0
diff --git a/llvm/unittests/CodeGen/AccelTableTest.cpp b/llvm/unittests/CodeGen/AccelTableTest.cpp
new file mode 100644
index 0000000000000..e2ed275f2f773
--- /dev/null
+++ b/llvm/unittests/CodeGen/AccelTableTest.cpp
@@ -0,0 +1,78 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "llvm/CodeGen/AccelTable.h"
+#include "TestAsmPrinter.h"
+#include "llvm/CodeGen/DwarfStringPoolEntry.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+
+namespace {
+
+class AccelTableTest : public testing::Test {
+protected:
+ void SetUp() override {
+ auto ExpectedTestPrinter = TestAsmPrinter::create(
+ "x86_64-pc-linux", /*DwarfVersion=*/5, dwarf::DWARF32);
+ ASSERT_THAT_EXPECTED(ExpectedTestPrinter, Succeeded());
+ TestPrinter = std::move(*ExpectedTestPrinter);
+ if (!TestPrinter)
+ GTEST_SKIP();
+ }
+
+ /// Builds a table holding \p Names and returns each bucket's contents in the
+ /// order they would be emitted in.
+ std::vector<std::vector<StringRef>>
+ bucketOrder(ArrayRef<const DwarfStringPoolEntryWithExtString *> Names) {
+ DWARF5AccelTable Table;
+ for (const DwarfStringPoolEntryWithExtString *Name : Names)
+ Table.addName(DwarfStringPoolEntryRef(*Name), /*DieOffset=*/0x11,
+ /*DefiningParentOffset=*/std::nullopt,
+ /*DieTag=*/dwarf::DW_TAG_variable, /*UnitID=*/0,
+ /*IsTU=*/false);
+ Table.finalize(TestPrinter->getAP(), "names");
+
+ std::vector<std::vector<StringRef>> Order;
+ for (const AccelTableBase::HashList &Bucket : Table.getBuckets()) {
+ Order.emplace_back();
+ for (const AccelTableBase::HashData *Hash : Bucket)
+ Order.back().push_back(Hash->Name.getString());
+ }
+ return Order;
+ }
+
+ std::unique_ptr<TestAsmPrinter> TestPrinter;
+};
+
+TEST_F(AccelTableTest, CollidingNamesOrderedIndependentlyOfInsertion) {
+ // The DWARF v5 hash folds case, so these three names always collide.
+ DwarfStringPoolEntryWithExtString Lower = {{}, "fixups"};
+ DwarfStringPoolEntryWithExtString Mixed = {{}, "Fixups"};
+ DwarfStringPoolEntryWithExtString Upper = {{}, "FIXUPS"};
+ DwarfStringPoolEntryWithExtString Other = {{}, "gamma"};
+
+ const uint32_t Hash = DWARF5AccelTableData::hash(Lower.String);
+ ASSERT_EQ(Hash, DWARF5AccelTableData::hash(Mixed.String));
+ ASSERT_EQ(Hash, DWARF5AccelTableData::hash(Upper.String));
+ ASSERT_LT(DWARF5AccelTableData::hash(Other.String), Hash);
+
+ const std::vector<std::vector<StringRef>> Order =
+ bucketOrder({&Lower, &Mixed, &Upper, &Other});
+ EXPECT_EQ(bucketOrder({&Other, &Upper, &Mixed, &Lower}), Order);
+ EXPECT_EQ(bucketOrder({&Mixed, &Other, &Lower, &Upper}), Order);
+
+ // The hash value orders before the name, so "gamma" comes first even though
+ // it sorts after the names it shares a bucket with.
+ EXPECT_THAT(Order, testing::Contains(testing::ElementsAre(
+ "gamma", "FIXUPS", "Fixups", "fixups")));
+}
+
+} // end namespace
diff --git a/llvm/unittests/CodeGen/CMakeLists.txt b/llvm/unittests/CodeGen/CMakeLists.txt
index 96034ff950830..deaa3dbcfcc5b 100644
--- a/llvm/unittests/CodeGen/CMakeLists.txt
+++ b/llvm/unittests/CodeGen/CMakeLists.txt
@@ -20,6 +20,7 @@ set(LLVM_LINK_COMPONENTS
)
add_llvm_unittest(CodeGenTests
+ AccelTableTest.cpp
AllocationOrderTest.cpp
AMDGPUMetadataTest.cpp
AsmPrinterDwarfTest.cpp
More information about the llvm-commits
mailing list