Skip to content

Commit 27000a9

Browse files
authored
determinism fix: hash results may differ between runs (#1431)
Hash results may differ between runs, as they can depend on pointers. In remove-duplicate-functions, that shouldn't matter, except that we only considered the first item in each hash group vs the others (to avoid O(N^2)), which is fine except for hash collisions (collisions mean 2 groups are merged into one, and considering just the first item vs the rest we miss out on the other duplicates in that single group). And hash collisions do occur (rarely) in practice. Instead, consider all comparisons in each hash group, which should be fine unless we have large amounts of hash collisions.
1 parent 85ae8cc commit 27000a9

File tree

1 file changed

+16
-17
lines changed

1 file changed

+16
-17
lines changed

src/passes/DuplicateFunctionElimination.cpp

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -102,23 +102,22 @@ struct DuplicateFunctionElimination : public Pass {
102102
std::set<Name> duplicates;
103103
for (auto& pair : hashGroups) {
104104
auto& group = pair.second;
105-
if (group.size() == 1) continue;
106-
// pick a base for each group, and try to replace everyone else to it. TODO: multiple bases per hash group, for collisions
107-
#if 0
108-
// for comparison purposes, pick in a deterministic way based on the names
109-
Function* base = nullptr;
110-
for (auto* func : group) {
111-
if (!base || strcmp(func->name.str, base->name.str) < 0) {
112-
base = func;
113-
}
114-
}
115-
#else
116-
Function* base = group[0];
117-
#endif
118-
for (auto* func : group) {
119-
if (func != base && equal(func, base)) {
120-
replacements[func->name] = base->name;
121-
duplicates.insert(func->name);
105+
Index size = group.size();
106+
if (size == 1) continue;
107+
// The groups should be fairly small, and even if a group is large we should
108+
// have almost all of them identical, so we should not hit actual O(N^2)
109+
// here unless the hash is quite poor.
110+
for (Index i = 0; i < size - 1; i++) {
111+
auto* first = group[i];
112+
if (duplicates.count(first->name)) continue;
113+
for (Index j = i + 1; j < size; j++) {
114+
auto* second = group[j];
115+
if (duplicates.count(second->name)) continue;
116+
if (equal(first, second)) {
117+
// great, we can replace the second with the first!
118+
replacements[second->name] = first->name;
119+
duplicates.insert(second->name);
120+
}
122121
}
123122
}
124123
}

0 commit comments

Comments
 (0)