[llvm] 15035c1 - [ORC] Fix and rename SymbolLookupSet::removeDuplicates (#216574)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Aug 16 16:06:19 PDT 2026
Author: Lang Hames
Date: 2026-08-17T09:06:13+10:00
New Revision: 15035c1d7932f7143f22a365435c6cede49738e0
URL: https://github.com/llvm/llvm-project/commit/15035c1d7932f7143f22a365435c6cede49738e0
DIFF: https://github.com/llvm/llvm-project/commit/15035c1d7932f7143f22a365435c6cede49738e0.diff
LOG: [ORC] Fix and rename SymbolLookupSet::removeDuplicates (#216574)
removeDuplicates uniqued on the (name, flags) pair, so a name appearing
with both RequiredSymbol and WeaklyReferencedSymbol survived twice --
leaving a set that containsDuplicates, which compares names only, still
reported as duplicated.
Replace it with mergeEntries, which merges by name. Where entries
disagree on flags the strongest requirement wins, so a missing
definition still fails the lookup. Renamed because the entries being
merged need not be exact duplicates.
removeDuplicates had no in-tree callers, so no existing behaviour
changes. Also adds a unit test for mergeEntries -- the first direct
coverage for SymbolLookupSet.
Added:
llvm/unittests/ExecutionEngine/Orc/SymbolLookupSetTest.cpp
Modified:
llvm/include/llvm/ExecutionEngine/Orc/SymbolLookupSet.h
llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt
llvm/utils/gn/secondary/llvm/unittests/ExecutionEngine/Orc/BUILD.gn
Removed:
################################################################################
diff --git a/llvm/include/llvm/ExecutionEngine/Orc/SymbolLookupSet.h b/llvm/include/llvm/ExecutionEngine/Orc/SymbolLookupSet.h
index bad4cd6052b8d..a3adf21986aea 100644
--- a/llvm/include/llvm/ExecutionEngine/Orc/SymbolLookupSet.h
+++ b/llvm/include/llvm/ExecutionEngine/Orc/SymbolLookupSet.h
@@ -225,12 +225,33 @@ class SymbolLookupSet {
});
}
- /// Remove any duplicate elements. If a SymbolLookupSet is not duplicate-free
- /// by construction, this method can be used to turn it into a proper set.
- void removeDuplicates() {
+ /// Merge entries that share a name, so that each name appears exactly once.
+ /// If a SymbolLookupSet is not duplicate-free by construction, this method
+ /// can be used to turn it into a proper set.
+ ///
+ /// Entries sharing a name need not agree on their flags. Where they
diff er
+ /// the strongest requirement wins: if any entry required the symbol then the
+ /// merged entry requires it too, so that a missing definition still fails the
+ /// lookup.
+ void mergeEntries() {
+ if (Symbols.size() < 2)
+ return;
sortByAddress();
- auto LastI = llvm::unique(Symbols);
- Symbols.erase(LastI, Symbols.end());
+
+ auto Out = Symbols.begin();
+ for (auto In = Out + 1; In != Symbols.end(); ++In) {
+ if (In->first == Out->first) {
+ // Same name: keep the stronger requirement.
+ if (In->second == SymbolLookupFlags::RequiredSymbol)
+ Out->second = SymbolLookupFlags::RequiredSymbol;
+ } else {
+ // New name: compact it down next to the previous survivor.
+ ++Out;
+ if (Out != In)
+ *Out = std::move(*In);
+ }
+ }
+ Symbols.erase(std::next(Out), Symbols.end());
}
#ifndef NDEBUG
diff --git a/llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt b/llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt
index 8af05a8b32e8f..ac648b3f420bf 100644
--- a/llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt
+++ b/llvm/unittests/ExecutionEngine/Orc/CMakeLists.txt
@@ -49,6 +49,7 @@ add_llvm_unittest(OrcJITTests
SimpleExecutorMemoryManagerTest.cpp
SimplePackedSerializationTest.cpp
SPSProxySpecTest.cpp
+ SymbolLookupSetTest.cpp
SymbolStringPoolTest.cpp
TaskDispatchTest.cpp
ThreadSafeModuleTest.cpp
diff --git a/llvm/unittests/ExecutionEngine/Orc/SymbolLookupSetTest.cpp b/llvm/unittests/ExecutionEngine/Orc/SymbolLookupSetTest.cpp
new file mode 100644
index 0000000000000..95c3884e0b8e8
--- /dev/null
+++ b/llvm/unittests/ExecutionEngine/Orc/SymbolLookupSetTest.cpp
@@ -0,0 +1,133 @@
+//===- SymbolLookupSetTest.cpp - Test SymbolLookupSet --------------------===//
+//
+// 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/ExecutionEngine/Orc/SymbolLookupSet.h"
+
+#include "llvm/ExecutionEngine/Orc/SymbolStringPool.h"
+
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::orc;
+
+namespace {
+
+class SymbolLookupSetTest : public testing::Test {
+protected:
+ static constexpr SymbolLookupFlags Required =
+ SymbolLookupFlags::RequiredSymbol;
+ static constexpr SymbolLookupFlags Weak =
+ SymbolLookupFlags::WeaklyReferencedSymbol;
+
+ std::shared_ptr<SymbolStringPool> SSP = std::make_shared<SymbolStringPool>();
+
+ SymbolStringPtr intern(StringRef S) { return SSP->intern(S); }
+
+ /// Collect a lookup set into a name-sorted vector of (name, flags).
+ ///
+ /// A vector rather than a map so that a name surviving more than once is
+ /// visible, and name-sorted because mergeEntries sorts by pointer value,
+ /// leaving an order that depends on allocation and must not be asserted on.
+ using Entries = std::vector<std::pair<std::string, SymbolLookupFlags>>;
+
+ static Entries contents(const SymbolLookupSet &LS) {
+ Entries Result;
+ for (const auto &[Name, Flags] : LS)
+ Result.emplace_back(std::string(*Name), Flags);
+ llvm::sort(Result);
+ return Result;
+ }
+};
+
+} // namespace
+
+// A set that is already duplicate-free is left alone, flags included.
+TEST_F(SymbolLookupSetTest, MergeEntriesNoDuplicates) {
+ SymbolLookupSet LS;
+ LS.add(intern("foo"), Required);
+ LS.add(intern("bar"), Weak);
+
+ LS.mergeEntries();
+
+ EXPECT_EQ(contents(LS), (Entries{{"bar", Weak}, {"foo", Required}}));
+}
+
+// Duplicates that agree on flags collapse to a single entry.
+TEST_F(SymbolLookupSetTest, MergeEntriesSameFlags) {
+ SymbolLookupSet LS;
+ LS.add(intern("foo"), Required);
+ LS.add(intern("foo"), Required);
+ LS.add(intern("bar"), Weak);
+ LS.add(intern("bar"), Weak);
+
+ LS.mergeEntries();
+
+ EXPECT_EQ(contents(LS), (Entries{{"bar", Weak}, {"foo", Required}}));
+}
+
+// A name requested both ways merges to RequiredSymbol: if any requester needs
+// the symbol then a missing definition must fail the lookup.
+//
+// Both insertion orders are checked because mergeEntries sorts by pointer
+// value, so which of the two entries is seen first is not under our control.
+TEST_F(SymbolLookupSetTest, MergeEntriesRequiredWinsWeakFirst) {
+ SymbolLookupSet LS;
+ LS.add(intern("foo"), Weak);
+ LS.add(intern("foo"), Required);
+
+ LS.mergeEntries();
+
+ EXPECT_EQ(contents(LS), (Entries{{"foo", Required}}));
+}
+
+TEST_F(SymbolLookupSetTest, MergeEntriesRequiredWinsRequiredFirst) {
+ SymbolLookupSet LS;
+ LS.add(intern("foo"), Required);
+ LS.add(intern("foo"), Weak);
+
+ LS.mergeEntries();
+
+ EXPECT_EQ(contents(LS), (Entries{{"foo", Required}}));
+}
+
+// Merging must not invent a requirement: all-weak duplicates stay weak.
+TEST_F(SymbolLookupSetTest, MergeEntriesAllWeakStaysWeak) {
+ SymbolLookupSet LS;
+ LS.add(intern("foo"), Weak);
+ LS.add(intern("foo"), Weak);
+ LS.add(intern("foo"), Weak);
+
+ LS.mergeEntries();
+
+ EXPECT_EQ(contents(LS), (Entries{{"foo", Weak}}));
+}
+
+// Several distinct names, each duplicated a
diff erent number of times and with
+// mixed flags, all merge in one pass.
+TEST_F(SymbolLookupSetTest, MergeEntriesMultipleRuns) {
+ SymbolLookupSet LS;
+ LS.add(intern("foo"), Weak);
+ LS.add(intern("bar"), Required);
+ LS.add(intern("foo"), Weak);
+ LS.add(intern("baz"), Weak);
+ LS.add(intern("bar"), Weak);
+ LS.add(intern("foo"), Required);
+ LS.add(intern("qux"), Required);
+
+ LS.mergeEntries();
+
+ EXPECT_EQ(contents(LS), (Entries{{"bar", Required},
+ {"baz", Weak},
+ {"foo", Required},
+ {"qux", Required}}));
+}
diff --git a/llvm/utils/gn/secondary/llvm/unittests/ExecutionEngine/Orc/BUILD.gn b/llvm/utils/gn/secondary/llvm/unittests/ExecutionEngine/Orc/BUILD.gn
index dfbf74b3cd1a9..5659ddfea45e0 100644
--- a/llvm/utils/gn/secondary/llvm/unittests/ExecutionEngine/Orc/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/unittests/ExecutionEngine/Orc/BUILD.gn
@@ -48,6 +48,7 @@ unittest("OrcJITTests") {
"SharedMemoryMapperTest.cpp",
"SimpleExecutorMemoryManagerTest.cpp",
"SimplePackedSerializationTest.cpp",
+ "SymbolLookupSetTest.cpp",
"SymbolStringPoolTest.cpp",
"TaskDispatchTest.cpp",
"ThreadSafeModuleTest.cpp",
More information about the llvm-commits
mailing list